Tree-shaking removes CSS files in Sapper with Rollup 2

[habibrosyad]

• Rollup Version: 2.3.4
• Operating System (or Browser): MacOS
• Node Version (if applicable): 12
• Link to reproduction (IMPORTANT, read below): https://github.com/habibrosyad/sapper-rollup-css-issue

Expected Behavior

CSS files should be bundled into several files (e.g per routes):

[slug].8afac42c.css
client.6bfaa544.css
index.7be4ff0d.css
index.9bb43f04.css
main.237455804.css

Actual Behavior

All CSS bundled into single file:

main.2065939480.css

Is this behaviour is expected when upgrading from version 1 to 2? When we build our project using version 2 we experienced conflicting CSS rules as all of them bundled into a single file. Prior to the upgrade, CSS can be loaded as needed per route.

[lukastaegert]

I am sorry to say but Rollup core is not a CSS bundler, this functionality must be added via plugins, and at least I am not familiar with how Sapper works in detail. If this is triggered by a Rollup update, It sounds like Sapper might not have been updated properly to support Rollup 2, as there were of course some breaking changes. I would recommend posting the issue with Sapper instead.

[habibrosyad]

Actually I've been posting this issue in the sveltejs/sapper-template#233 first, and got a suggestion where it might be better to ask it here.

Thanks anyway...

[habibrosyad]

Will close this for now...

[benmccann]

I investigated this and found out that if you set treeshake: false then it works. @habibrosyad I think it might make sense to reopen this issue (and change the title to something like "Tree-shaking removes CSS files") because I wouldn't expect tree shaking to affect CSS files, but only JS files. @lukastaegert is that a fair assumption that I've made?

[habibrosyad]

Per finding and suggestion of @benmccann I've updated the title and reopen this issue for further discussion @lukastaegert.

[lukastaegert]

It might be possible that Sapper CSS relies on something like treeshaken files being listed as part of chunks in the generated output. This will not be fixed. This is a breaking change in Rollup 2 that will need some changes in Sapper. I can help here make things easier but someone WILL need to fix Sapper.

[benmccann]

I can make any changes necessary in Sapper, but I'm having some trouble understanding what's happening or supposed to happen here. rollup-plugin-svelte creates CSS files and then it seems like perhaps Rollup tree shakes them out. But what does tree shaking mean in the context of CSS and why would they be removed?

[lukastaegert]

Rollup does nothing with CSS files, it must all be handled in Sapper. But I suspect that Sapper relies likely on the modules property in https://rollupjs.org/guide/en/#generatebundle to determine to which chunk the CSS belongs. But this needs someone familiar with the Sapper code base to confirm, or at least someone who is willing to dig into it.

If Sapper handles emitting CSS by transforming imports of ".css" files into imports of empty JavaScript files and then tries to find out in which chunk in the bundle those empty files ended up, then that is what is no longer working. Tree-shaking will remove those modules entirely and they are not part of any chunk.

I am planning on adding a property to mark files as non-treeshakeable, but first someone should determine if this is actually the issue.

[benmccann]

If Sapper handles emitting CSS by transforming imports of ".css" files into imports of empty JavaScript files and then tries to find out in which chunk in the bundle those empty files ended up, then that is what is no longer working.

I'm fairly certain that's not what it's doing. The code is here:

https://github.com/sveltejs/sapper/blob/be8db302fa58d4186b6ea0e03021e8cfe6f98348/src/core/create_compilers/extract_css.ts#L143

In Rollup 1 and Rollup 2 with treeshake: false, chunk.modules contains files with .css extension (and the contents are actually CSS as well). There's no JavaScript involved anywhere in this issue. These CSS files would have been generated by rollup-plugin-svelte and Sapper is expecting them to be present. For some reason, in Rollup 2 when treeshake: true (the default) these files are no longer present in chunk.modules. Sapper just uses the output from Rollup directly and hasn't touched it at all before examining the chunk.modules here.

Rollup does nothing with CSS files

That seems to be mostly true, but not 100% true. I feel fairly confident that Rollup is removing them from chunk.modules when treeshaking. I wouldn't expect this to happen, but perhaps it's intended and Sapper should be implementing its logic another way?

@Rich-Harris wrote the implementation of this logic in Sapper. He seems pretty busy though, so I'm not sure if we'll be able to get him to chime in here. Perhaps you're better connected to him than I am.

[lukastaegert]

Well, believe what you want about what Rollup is doing, just assume that I am not guessing about how Rollup works being the one basically writing Rollup for the last years. Rollup internally can only import JavaScript, so what plugins do that want to handle e.g. an import 'foo.css' is that they tell Rollup that foo.css is actually JavaScript because extensions really do not matter. And because a JavaScript file needs some code, they just say that the module is an empty string, which is valid JavaScript all right. It just happens to have no side-effects, which is why Rollup thinks we can just ignore this import altogether.

As far as I can tell, #3663 will solve this issue from Rollup side in that it will allow you to disable treeshaking for your foo.css replacement module altogether, which again puts it back into generateBundle.

Looking at the repo you linked, the relevant code actually appears to be here: https://github.com/sveltejs/sapper/blob/be8db302fa58d4186b6ea0e03021e8cfe6f98348/src/core/create_compilers/RollupCompiler.ts#L41-L46

My guess is if you use the rollup PR I linked and replace that with

transform: (code: string, id: string) => {
  if (/\.css$/.test(id)) {
    this.css_files.push({ id, code });
    return {code: ``, moduleSideEffects: 'no-treeshake'};
  }
}

Everything should be working again nicely. Would be nice if you could verify that, though.

[habibrosyad]

@lukastaegert @benmccann I confirmed that the changes stated above indeed fix the problem. @lukastaegert any estimate when #3663 will be merged into master?

[benmccann]

Aha!! Thanks @lukastaegert for the investigation and digging through the Sapper code. I didn't mean to doubt you, but I looked at all the plugins defined in package.json and rollup.config.js and none of them were responsible. I didn't know there was another plugin being added in Sapper's code! Great catch!

I'll trust @habibrosyad's testing, but please let me know if there's anything else I can do to help. Thanks again so much for all your help!!

[lukastaegert]

No problems, I guess it IS kindof confusing to have some tool mess secretly with your bundler config. I will have another look at the docs and then publish this later today as a new minor version.

[lukastaegert]

Published in Rollup@2.21.0

[habibrosyad]

Thanks @lukastaegert 👍

[benmccann]

Yes, huge thank you!!