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
Tree-shaking removes CSS files in Sapper with Rollup 2 #3655
Comments
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. |
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... |
Will close this for now... |
I investigated this and found out that if you set |
Per finding and suggestion of @benmccann I've updated the title and reopen this issue for further discussion @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. |
I can make any changes necessary in Sapper, but I'm having some trouble understanding what's happening or supposed to happen here. |
Rollup does nothing with CSS files, it must all be handled in Sapper. But I suspect that Sapper relies likely on the 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. |
I'm fairly certain that's not what it's doing. The code is here: In Rollup 1 and Rollup 2 with
That seems to be mostly true, but not 100% true. I feel fairly confident that Rollup is removing them from @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. |
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 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 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. |
@lukastaegert @benmccann I confirmed that the changes stated above indeed fix the problem. @lukastaegert any estimate when #3663 will be merged into master? |
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 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!! |
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. |
Published in Rollup@2.21.0 |
Thanks @lukastaegert 👍 |
Yes, huge thank you!! |
Expected Behavior
CSS files should be bundled into several files (e.g per routes):
Actual Behavior
All CSS bundled into single file:
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.
The text was updated successfully, but these errors were encountered: