rolldown / rolldown

Fast Rust bundler for JavaScript/TypeScript with Rollup-compatible API.
https://rolldown.rs
MIT License
8.88k stars 497 forks source link

[Bug]: `modules` property in `RenderedChunk` is not deterministic #2364

Open sapphi-red opened 1 month ago

sapphi-red commented 1 month ago

Reproduction link or steps

  1. Clone https://github.com/rolldown/vite/tree/d0518d3cff2439e864a98b6e51993ed18b254bcb
  2. pnpm i && pnpm build
  3. cd playground/css-codesplit
  4. run pnpm build multiple time
  5. see assets/main-h8NuCY-I.css or assets/main-A25c-rgU.css exists

The result of this line changes: https://github.com/rolldown/vite/blob/d0518d3cff2439e864a98b6e51993ed18b254bcb/packages/vite/src/node/plugins/css.ts#L600

What is expected?

The order of the keys in modules property in RenderedChunk is deterministic.

What is actually happening?

The order of the keys in modules property in RenderedChunk is not deterministic and changes some times even if the input is same.

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (24) x64 AMD Ryzen 9 5900X 12-Core Processor
    Memory: 14.70 GB / 31.92 GB
  Binaries:
    Node: 20.17.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.8.2 - C:\Program Files\nodejs\npm.CMD
    pnpm: 9.10.0 - C:\Program Files\nodejs\pnpm.CMD
    bun: 1.1.27 - ~\AppData\Local\Microsoft\WinGet\Links\bun.EXE
  Browsers:
    Edge: Chromium (127.0.2651.74)
    Internet Explorer: 11.0.22621.3527

Any additional comments?

In some case, it seems it's not deterministic in Rollup: https://github.com/rollup/rollup/issues/3682 Note that in this case modules have moduleSideEffects: 'no-treeshake' set.

IWANABETHATGUY commented 1 month ago

related to https://github.com/rolldown/rolldown/blob/00fa6ffd8e20cf567c5fba5b6741273a0d87830c/crates/rolldown_common/src/types/rollup_rendered_chunk.rs#L18-L18 FxHashmap did not guarantee order.

A little curious about why vite care about modules keys order?

sapphi-red commented 1 month ago

Vite traverses the modules and concatenate all CSS linked to those modules. If the order is not deterministic, the bundle output hash gets non-deterministic. Given that Vite will use rolldown's internal CSS handle mechanism, this line won't be a problem anymore. I think it's a nice-to-have and not urgent.

IWANABETHATGUY commented 1 month ago

Vite traverses the modules and concatenate all CSS linked to those modules. If the order is not deterministic, the bundle output hash gets non-deterministic. Given that Vite will use rolldown's internal CSS handle mechanism, this line won't be a problem anymore. I think it's a nice-to-have and not urgent.

Understood

hyf0 commented 2 weeks ago

modules would be sorted by its execution order. But this behavior is not always true, we might change it in the future for having better cache control.

I'll fix this when I have time.