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.

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

Possible breaking change in minor version, sudden rename of chunk filenames #3060

Closed
remcovaes opened this issue Aug 15, 2019 · 4 comments · Fixed by #3083
Closed

Possible breaking change in minor version, sudden rename of chunk filenames #3060

remcovaes opened this issue Aug 15, 2019 · 4 comments · Fixed by #3083

Comments

@remcovaes
Copy link

  • Rollup Version: 1.8.0 to 1.9.4
  • Operating System (or Browser): Windows (has only to do with the rollup cli)
  • Node Version: 12.3.0

How Do We Reproduce?

  1. https://github.com/remcovaes/rollup-breaking-change
  2. Run npm install
  3. Run npx rollup -c
  4. Observe files:
    • app-f863e8ca.js
    • chunk-1620c961.js
    • doc1-df1a6f13.js
    • doc2-e74ddcef.js
  5. Bump rollup to 1.19.4
  6. npm install
  7. npx rollup -c
  8. Observe output:
    • app-f863e8ca.js
    • exports-a-function-1620c961.js
    • doc1-df1a6f13.js
    • doc2-e74ddcef.js

The first doc1-df1a6f13.js (also cached for a long time) imports chunk-1620c961.js. The new doc1-df1a6f13.js import exports-a-function-1620c961.js. Because the first doc1 is cached for a long time, the second one is never loaded in the browser, due to server side caching.

Expected Behavior

When file name changes of imported files, i would expect that the hash in the filename changes.

Actual Behavior

There can be 2 files with different content (the imports differ) with the same hash, causing server to cache a file with an import to a non existent file.

@philipwalton
Copy link

I'm noticing this as well. It looks like there are two problems here:

  1. The doc1-df1a6f13.js hash has not changed even though the file it's importing has changed
  2. Looks like in v1.9.0 Rollup switched from naming all unnamed chunks "chunk" (as described in the documentation) to now attempting to use the filename.

Regarding (1), I believe this is a bug not a breaking change and it definitely needs to be fixed or reverted. If the chunk hashing is not taking into consideration the content as well as the filenames of all dependencies, that's going to cause bugs like the one described here -- not just in cases where Rollup changes the naming, but even in cases where users switching their naming scheme. I wrote about a similar issue in #2839.

Regarding (2), I believe this should be considered a breaking change (it broke my build code), because any build logic that was expecting the name of non-manually-generated chunks to always be chunk-XXXX is now going to fail.

In my particular case, I was creating an asset map JSON file of chunk names to full filenames (including the hash), so that in my templates I could create src URLs to modules based on their entry name.

The problem introduced by this name change is now I can have multiple chunks with the same name, and I don't have control over that naming at all -- which means dynamically generated chunks were overriding my entry chunks in my asset manifest.

Given that (2) broke my code and (1) is clearly problematic, would it make sense to revert to the old behavior until a better solution is found?

@lukastaegert
Copy link
Member

I definitely agree that not taking the pattern itself or the [name] into account when generating the hash is a severe bug.

Regarding (2), I believe this should be considered a breaking change (it broke my build code), because any build logic that was expecting the name of non-manually-generated chunks to always be chunk-XXXX is now going to fail.

As for the name changing, I agree that this may be inconvenient for you but as I see it, there was never a strict rule that auto-generated chunks have their [name] set to chunk. This rule was already broken for dynamic imports as well as some other edge cases where the auto-generated chunk contained code from a named entry point and of course manual chunks. If we treat this as a breaking change, we could also argue that any change to the generated code by Rollup is breaking because people could rely on the exact structure of the code. But I admit the text on the website is misleading as it only mentions manual chunks and should definitely be changed.

My suggestion if you rely on the name chunk being used is to use an explicit naming pattern, e.g. chunkFileNames: "chunk-[hash].js", or maybe even chunkFileNames: "chunk/[name]-[hash].js", which would retain the names but place all auto-generated chunks in a sub-folder.

As for the manifest file, not sure I completely understand your scenario but in the bundle object, you also have the information available if a file is an entry point, maybe this would help you?

@philipwalton
Copy link

But I admit the text on the website is misleading as it only mentions manual chunks and should definitely be changed.

This was the reason I considered it "breaking". It was documented to do X and then it stopped doing X and started doing Y from 1.8 to 1.9.

I do understand though that documentation isn't always correct (I've certainly written my share of incorrect docs), so I guess I don't necessarily consider all deviations from documentation breaking...

Anyway, in terms of this feature, one thing that would be helpful for my particular use case here is the ability to distinguish between chunks created via the manualChunks option and chunks created by Rollup due to dynamic import.

While I can solve my particular issue in a variety of ways, my preference would be to not have two files in the same directory that only differ in their hash (e.g. lodash-XXXX.js and lodash-YYYY.js) because to me that looks like one of them is an older version of the same file. For manual vs dynamic chunks, I'd prefer to name these as something like (lodash-XXXX.js and chunk-lodash-XXXX.js).

If @jakearchibald's original suggestion of using a function is possible, that would be my preference.

{
  chunkFileName: (chunkInfo: ChunkInfo) => string
}

I guess that would require adding some additional info to the ChunkInfo object (e.g. hash, etc.). It would also be nice if the ChunkInfo object exposed whether it was generated via the manualChunks options, but I think I might be able to infer that through other methods...

@lukastaegert
Copy link
Member

Fix for the original issue at #3083

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants