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.
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
Only set asset names when finalizing #4919
Conversation
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#fix-asset-deduplication or load it into the REPL: |
Codecov Report
@@ Coverage Diff @@
## master #4919 +/- ##
=======================================
Coverage 99.01% 99.01%
=======================================
Files 221 221
Lines 8029 8049 +20
Branches 2210 2214 +4
=======================================
+ Hits 7950 7970 +20
Misses 26 26
Partials 53 53
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Thanks! But I'm afraid this does not solve Vite's issue (#4909, vitejs/vite#11911). Vite needs to call I'm having a feeling that
has a nature of non-deterministic file names. I think I can mitigate the issue specifically for CSS files by calling |
src/utils/FileEmitter.ts
Outdated
} | ||
fileName = newFileName; | ||
fileNamesBySource.set(sourceHash, fileName); | ||
if (!fileName || assetFileName < fileName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fileName
sort strategy is a bit different from #4912. The previous code always uses the shortest fileName
while this one doesn't; I think it's better to keep the same strategy, otherwise this would be a breaking change.
This PR has been released as part of rollup@3.20.1. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
The logic to guarantee deterministic names for emitted assets with the same source added in #4912 was unfortunately breaking
this.getFileName
. The new logic now only fixes the name when doing the first round of finalizations beforerenderStart
, which should be good enough. Assets emitted at a later stage will still follow a "first assets determines the name" strategy. This guarantees, thatthis.getFileName
will always return a correct and reliable name that does not change.