Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Non-deterministic asset file name when multiple asset has same content with a different name #4909

Closed
sapphi-red opened this issue Mar 17, 2023 · 2 comments · Fixed by #4912
Closed

Non-deterministic asset file name when multiple asset has same content with a different name #4909

sapphi-red opened this issue Mar 17, 2023 · 2 comments · Fixed by #4912
Labels
c³ ⋅ PR welcome t¹ 🐞 bug

Comments

@sapphi-red
Copy link
Contributor

sapphi-red commented Mar 17, 2023 •
edited

Rollup Version

3.19.1

Operating System (or Browser)

stackblitz

Node Version (if applicable)

No response

Link To Reproduction

https://stackblitz.com/edit/rollup-repro-dju2ff?file=rollup.config.js

Expected Behaviour

When calling this.emitFile with a same content but with different filename, rollup dedupes that and only outputs a single asset.
But because rollup always uses the filename used in the first call, the asset file name is non-deterministic when this.emitFile is called in parallel.

For example, Vite outputs CSS files by this.emitFile and uses the chunk name as a file name of the CSS file. renderChunk is called in parallel and when there's multiple CSS files containing the same content, the build becomes non-deterministic.

I think it would be nice to fix this on rollup side as I guess this can happen by using rollup without Vite.

Actual Behaviour

Rollup outputs assets with deterministic file names.

One possible solution is to sort all the file names and choose the first one.

Related: vitejs/vite#11911, vitejs/vite#12256
@sapphi-red sapphi-red mentioned this issue Mar 17, 2023
Closed
7 tasks
@sun0day
Copy link
Contributor

sun0day commented Mar 17, 2023

I can give a try

sun0day pushed a commit to sun0day/rollup that referenced this issue Mar 19, 2023
fix: make file name deterministic in parallel emits (fix rollup#4909)
14dcbd7
@sun0day sun0day mentioned this issue Mar 19, 2023
Merged
9 tasks
lukastaegert pushed a commit that referenced this issue Mar 20, 2023
@sun0day
fix: make file name deterministic in parallel emits (fix #4909) (#4912)
3682f30 
* fix: make file name deterministic in parallel emits (fix #4909)

* refactor: use > determine emit file name

* test: emit file in a random order
@rollup-bot
Copy link
Collaborator

This issue has been resolved via #4912 as part of rollup@3.20.0. You can test it via npm install rollup.

@sapphi-red sapphi-red mentioned this issue Mar 23, 2023
Merged
9 tasks
@peterpeterparker peterpeterparker mentioned this issue May 25, 2023
Merged
github-merge-queue bot pushed a commit to dfinity/nns-dapp that referenced this issue May 25, 2023
@peterpeterparker
Bump dev dependencies (#2574)
a27edf5 
# Motivation

As far as tested, this does not solve the reproducibility but, bumping
the dev dependencies should introduce a non deterministic improvement
developed in Rollup.

References:

- Rollup: rollup/rollup#4909
- Vite: vitejs/vite#11911
- SvelteKit: sveltejs/kit#8291

# Notes

I bumped all dev dependencies except Playwright and the types of jest
and marked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c³ ⋅ PR welcome t¹ 🐞 bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: make file name deterministic in parallel emits (fix #4909) sun0day/rollup
4 participants
@lukastaegert @sapphi-red @rollup-bot @sun0day