CSS bundling no longer working as it used to be with Rollup 2
habibrosyad opened this issue · 13 comments
Hi, I'm using https://github.com/sveltejs/sapper-template-rollup as the base of my project months ago. Today, I updated packages as well as the rollup configs according to the latest commit of the template (particularly updating rollup from v1 to v2). However, I noticed that when building the app, all my css bundled into a single file only (main.css) whereas previously there were several css files (I guess it's based on the routes). Currently this behaviour breaks some of the styling in my project because there are conflicts between CSS rules.
Is there away to mitigate this issue (apart from rolling back prior to the update)?
An example for the list of css files generated with the vanilla dev build of the template (prior to update):
[slug].8afac42c.css
client.6bfaa544.css
index.7be4ff0d.css
index.9bb43f04.css
main.237455804.css
After update:
main.2065939480.css
The main reason that I noticed seems to be because of upgrading rollup v1 to v2. May need to adjust the rollup config, but I have no idea what to change.
I can confirm this. When visiting http://localhost:3000/profile/@ikzsl5 in the realworld sample it included http://localhost:3000/client/chunk.aef098f1.css until sveltejs/realworld#55
I have no idea whether this change was intended or how to affect the behavior. You'll probably have better luck asking questions about on Rollup on https://gitter.im/rollup/rollup or https://stackoverflow.com/questions/tagged/rollupjs or filing issues in https://github.com/rollup/rollup/issues. Please keep us updated in this issue though with any feedback or suggestions you receive from the Rollup community
Ok, will do.
@benmccann per the response in rollup/rollup#3655, they suggest that Sapper might not have been updated properly to support Rollup 2, therefore the breaking changes. Should this issue be brought up to Sapper instead?
Hmm. So I investigated more and I'm not quite sure why visiting http://localhost:3000/profile/@ikzsl5 in the realworld sample would include http://localhost:3000/client/chunk.aef098f1.css with Rollup 1 because the only style tag is in _error.svelte, so it actually seems maybe the Rollup 1 behavior was broken in some way too.
I think perhaps it will be better to investigate using the hn.svelte.dev sample
With the hn.svelte.dev sample and downgrading the Rollup to version 1 I could get different CSS being loaded when you visit different routes:
Visiting http://localhost:3000/top/1
<link rel="stylesheet" href="client/main.1788086292.css">
<link rel="stylesheet" href="client/[page].b543e849.css">
<link rel="stylesheet" href="client/client.cdab43ac.css">Visiting http://localhost:3000/item/23703367
<link rel="stylesheet" href="client/main.1788086292.css">
<link rel="stylesheet" href="client/[id].465e0571.css">
<link rel="stylesheet" href="client/client.cdab43ac.css">I suppose main and client seems to be the common CSS shared between those routes. Might as well unnecessary styles being loaded as you suggest there (weird behaviour with Rollup 1). However, with Rollup 2 everything just become a single file. Therefore, I think the later is much worse than the first one.
In Rollup 2, chunk.modules no longer contains the .css files and so the CSS files never get written to disk.
I'm not sure whether there's anything Sapper can do about this. That comes straight from Rollup. It seems most likely to need to be addressed in either Rollup or in rollup-plugin-svelte. rollup-plugin-svelte doesn't seem to be behaving any differently as far as I can see, but there may be some new requirement placed upon it which it's not fulfilling
I pinned the Rollup version at 1.32.1 and it worked the old way and then pinned it to 2.0.0 and the behavior changed, so it was definitely related to the actual 2.0.0. Nothing in the release notes looks super obviously related - maybe the CSS files are now getting tree shaken or the return value of transform needs to change?
If I set treeshake: false then it works the way it did with Rollup 1. I'll leave a comment on the Rollup issue
Summing up based on the discussion in rollup/rollup#3655, another way to fix this without disabling treeshake all together would be using rollup/rollup#3663 (may need to wait for this to be released into master first) and update Sapper codes in https://github.com/sveltejs/sapper/blob/be8db302fa58d4186b6ea0e03021e8cfe6f98348/src/core/create_compilers/RollupCompiler.ts#L41-L46 into:
transform: (code: string, id: string) => {
if (/\.css$/.test(id)) {
this.css_files.push({ id, code });
return {code: ``, moduleSideEffects: 'no-treeshake'};
}
}I have confirmed this using https://github.com/sveltejs/sapper/releases/tag/v0.27.16 (as well latest master) with the above changes and rollup/rollup#3663
@benmccann Should I make a PR for this in Sapper (while waiting for the update in Rollup to be released)? I've checked that those changes do not affect anything when running sapper project with the current template.
Cool. Let's wait until rollup/rollup#3663 is merged to confirm that will be the API, but then yeah it will make sense to make that change here
I also tested and confirmed the fix. I sent a PR here: sveltejs/sapper#1306
Thanks @benmccann, also rollup/rollup#3663 has been released in rollup@2.21.0, so the only things remains is for sveltejs/sapper#1306 to get merged.
Closed via sveltejs/sapper#1306 – thank you @benmccann