fix: keep the module graph traversal order consistent #19686
Conversation
|
There's also #19012, but not much activity there anymore :( |
CodSpeed Performance ReportMerging #19686 will degrade performances by 95.45%Comparing Summary
Benchmarks breakdown
|
94b6215 to
3a68feb
Compare
aa6643b to
0b1fe37
Compare
f52f19d to
2bb2bc1
Compare
2bb2bc1 to
a99df5a
Compare
|
Hi @ahabhgk, may I ask if that Rspack issue has been resolved? I'd appreciate it if you could take a look at this code as well. cc @alexander-akait @hai-x @jantimon |
I believe this is the key to success 👍 - My first idea was to solve the order only inside In my solution the following example caused a lot of headaches: import {Foo} from "./foo-barrel";
import {Bar} from "./bar-barrel";
console.log(Bar);
console.log(Foo);and import {Foo} from "./barrel";
import {Bar} from "./barrel";
console.log(Bar);
console.log(Foo);The problem here is that Could you please double check that this not a problem in your solution anymore? |
|
Hi @jantimon, big thanks for your earlier work. I’ve confirmed that the case you mentioned shouldn’t be an issue. The core of my fix is to sort HarmonyImportSpecifierDependency by import order, so downstream logic like module graph traversal behaves as expected. In webpack, HarmonyImportSideEffectDependency keeps the original import order, but others like HarmonyImportSpecifierDependency are sorted by usage. This mismatch can cause ordering issues, once tree-shaking removes the HarmonyImportSideEffectDependency, loading CSS solely via HarmonyImportSpecifierDependency causes it to follow usage order instead of import order. To fix this, I propose sorting HarmonyImportSpecifierDependency by import order as well, aligning with how native JS engines handle modules without bundlers. |
|
No, Rspack haven't resolve this, actually most css order inconsistent problem we are facing is caused by splitChunks, we are working on that before and introduced a experimental plugin which is inspired by next.js, to at least let users can resolve it by their own. And since not many libraries will publish CSS Modules directly so we put this on a low priority. The code looks good to me, I would probably write it in a similar way if let me work on it 😃 |
What kind of change does this PR introduce?
Fixes #18961
Thanks to @jantimon for the detailed analysis of this issue. Instead of adopting the approach in #19012, I chose to fix the dependency order directly at the point where module updates are triggered, without relying on the internal implementation details of the graph.
Did you add tests for your changes?
Yes, The test cases come from
Does this PR introduce a breaking change?
No
What needs to be documented once your changes are merged?
No