-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Description
Expected Behavior / Situation
Adding a minification plugin (which will change a chunk's output contents) should also change the chunk's file hash.
Actual Behavior / Situation
The chunk's hash is unchanged by minification plugins, which can result in lots of subtle bugs when deploying files to production.
For example, consider the following scenario:
- A user deploys a web app to production with buggy minification settings (e.g. property mangling that's inconsistent across chunks).
- The users discovers the mistake and redeploys, thinking they've fixed the problem.
- The bug will persist for any returning users since the chunk filenames won't have changed and the browser will load the old (cached) versions of those files.
Modification Proposal
I see hash determination was already discussed when code splitting was added to rollup, but I think there are some problems with that logic -- specifically this part:
Calculate a content hash for each chunk. This initial hash should only depend on the content of this chunk itself, not on its imports
If the imports are not included in the content hash calculation, then partial redeploys will break.
For example, consider a case where you have chunk A which depends on chunk B, which depends on chunk C. If the contents of C changes, then the hashes of all three chunks needs to change. The reason is because if the contents of C changes then it needs a new filename in order to cache-bust. But if C has a new filename then the contents of B must also change (e.g. the import path) in order to load the correct version of C. And if the contents of B change then it also needs a new filename, and so on and so forth all the way down to A.
As a result, I don't see any feasible way to correctly determine file hashes without considering the entire file, including its imports and their file paths.
My recommendation is Rollup should handle this itself after all plugins that can modify source code have run. If it processes all chunks in reverse topologically sorted order, then it should be able to properly calculate the hashes of all chunks based on their full, final contents (including import statements).
The main problem/issue I see with this approach is how to handle circular dependencies. However, Rollup already warns on circular dependencies, so the simplest solution seems to be just continue to warn but add some additional text that explains that content hashes are non-deterministic in bundles with circular dependencies.
Aside: hopefully this issue won't be as tricky to solve in the future once all browsers support import maps, because once they do all file hashes can just be added to the import map declaration rather than having to appear in the file itself.