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

rollup@19.1.0 outputs invalid CSS chunk names #536

Closed
tivac opened this issue Jan 14, 2019 · 3 comments

Comments

Projects
None yet
1 participant
@tivac
Copy link
Owner

commented Jan 14, 2019

Outputs files like:

chunk2.css
chunk23.css
chunk234.css

// Want to use source chunk name when code-splitting, otherwise match bundle name
let identifier = outputOptions.dir ? name : path.basename(entry, path.extname(entry));
// Replicate rollup chunk name de-duping logic here since that isn't exposed for us
while(out.has(identifier)) {
identifier = `${identifier}${++counter}`;
}

Should be something like:

let base = outputOptions.dir ? name : path.basename(entry, path.extname(entry));
let identifier = base;

console.log(identifier);

// Replicate rollup chunk name de-duping logic here since that isn't exposed for us
while(out.has(identifier)) {
    identifier = `${base}${++counter}`;
}

@tivac tivac self-assigned this Jan 14, 2019

@tivac tivac changed the title v19.1.0 outputs invalid CSS chunks rollup@19.1.0 outputs invalid CSS chunks Jan 14, 2019

@tivac tivac changed the title rollup@19.1.0 outputs invalid CSS chunks rollup@19.1.0 outputs invalid CSS chunk names Jan 14, 2019

@tivac

This comment has been minimized.

Copy link
Owner Author

commented Jan 14, 2019

Rollup already handles deduping asset names, so I should rework this to use an array instead of a Map and let rollup do its thing.

One catch, source maps will need to use this.getAssetFileName(id) to get the name of the file & do a .replace(".css", ".css.map"). Otherwise they end up as chunk.css.map, chunk.css2.map, etc. which then won't get correctly loaded.

@tivac

This comment has been minimized.

Copy link
Owner Author

commented Jan 14, 2019

One unfortunate wrinkle:

Passing rollup a name like "chunk.css" twice results in "chunk2.css", but passing something like "chunk.css.map" twice results in "chunk.css2.map".

-chunk2.css
+chunk.css2.map

Since the names don't match up now browsers won't auto-load the sourcemap, which isn't great. Working around this is gonna be tricky since rollup supports [hash] in name templates.

@tivac

This comment has been minimized.

Copy link
Owner Author

commented Jan 14, 2019

Thought about using the postcss { map : { annotation } } field to adjust the auto-injected sourcemap comment, but turns out that this.getAssetFileName() can't be called until the asset source is set. Leads to a nasty chicken-and-egg problem.

Maps will need to be written out using fs.writeFile(), there's no way to say "this thing was already hashed please ignore" via the rollup plugin api.

tivac added a commit that referenced this issue Jan 15, 2019

WIP: fix chunk naming
Still fighting through maps though.

Working to fix #536

tivac added a commit that referenced this issue Jan 15, 2019

@tivac tivac closed this in #537 Jan 15, 2019

tivac added a commit that referenced this issue Jan 15, 2019

fix: clean up rollup chunk naming & source maps (#537)
Fixes #536 

- No more `chunk2.css`, `chunk23.css`, etc
- Fixed `sourceMappingURL` annotations at the end of CSS files when source maps are enabled
- Source maps still have invalid file values if `[hash]` is used as part of the `assetFileNames` param, not really much that can be done about that atm.

BREAKING CHANGE:

Source maps are written directly to the filesystem now, instead of going through rollup's asset pipeline. This is due to some limitations inherent in how the asset pipeline works and may be changed back once those can be resolved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.