sveltejs/sapper

treeshaking of css

Opened this issue · 8 comments

Describe the bug
I have a Component that injects some CSS in DEV mode. I don't want this style to be applied in my production build.
To do this I created a the InjectDevStyle.svelte-component with only following content:

<script>
  import "../styles/dev.css";
</script>

Then i use this component in my _layout.svelte file like this:

{#if DEVELOPMENT}
  <InjectDevStyle />
{/if}

where DEVELOPMENT is a variable that is statically set with the @rollup/plugin-replace plugin.

Until sapper 0.28.2 the style only appeared in dev mode.
But since sapper 0.28.3 the style is also being bundled into my production build.

To Reproduce

Please see this repo:
https://github.com/ivanhofer/sapper-style-bug

To simplify the example, I replaced DEVELOPMENT with false and added a dev.css with following content:

body {
  background-color: red;
}

Run npm run build and see that the content of dev.css is also inside the __sapper__/build-folder. When starting the application, the background-color will be red.

When going back one version of sapper with npm i sapper@0.28.2 and running the build process again, the dev.css style is missing like expected.

Expected behavior
I would expect sapper to treeshake the InjectDevStyle-component completely.

Information about your Sapper Installation:

System:
OS: Windows 10 10.0.19041
CPU: (12) x64 Intel(R) Core(TM) i7-9850H CPU @ 2.60GHz
Memory: 20.16 GB / 31.73 GB
Binaries:
Node: 13.13.0 - C:\Program Files\nodejs\node.EXE
Yarn: 1.22.4 - C:\Program Files\nodejs\yarn.CMD
npm: 6.14.4 - C:\Program Files\nodejs\npm.CMD
Browsers:
Chrome: 85.0.4183.102
Edge: Spartan (44.19041.423.0), ChromiumDev (87.0.634.0)
Internet Explorer: 11.0.19041.1
npmPackages:
rollup: ^2.26.11 => 2.26.11
sapper: ^0.28.3 => 0.28.3
svelte: ^3.25.0 => 3.25.0

Severity
This issue blocks me from an update to the latest version of sapper, since I need this additional CSS for debugging.

A workaround would be to comment out the InjectDevStyle-component every time I do a production deployment, but its error-prone and I'm sure I will forget it someday.

@ivanhofer if downgrade (i.e. "sapper": "0.28.2", in package.json) then I still see a red background. I tried with older versions besides that as well. Can you check the steps to reproduce?

@benmccann I just tested it a few times switching back and forth between version 0.28.2 and 0.28.3 and for me the background is always red in 0.28.3 and always white in 0.28.2. Do you also use a windows machine to build the application?

No, I'm on Linux. We just checked in a big update to the CSS functionality. Can you try against master?

Even with the newest changes in master, I still get a red background.

@ivanhofer I'm still getting a red background with 0.28.2 too, so I'm not sure this is a regression. In fact, in Sapper 0.28.0 and before most styles were included on the page twice as a result of another bug, so I'd be surprised if you could recreate this on those versions. Some things you might want to try:

  • git clone git@github.com:ivanhofer/sapper-style-bug.git in a new directory. Change the Sapper version to 0.28.2 in package.json. Do npm install and then npm run dev
  • Hit unregister several times in the Application tab of the chrome debugger to get rid of any service workers. They cache aggressively. Then hit Ctrl + F5 to reload
  • Delete the __sapper__ directory each time before running the dev server

@Rich-Harris this is something you mentioned wanting to support I believe. If you take a look at the layout from this reproduction it has:

{#if false}
  <InjectDevStyle />
{/if}

InjectDevStyle.svelte does import "../styles/dev.css"; which has body { background-color: red; }, so you end up with a red background on every page eventhough that component is not instantiated

Since this is in the layout, that style is ending up in client-[hash].css, which is included on every page. I'm not sure whether InjectDevStyle is being treeshaken or not or whether the tree shaking happens later in the lifecycle after the CSS plugin has already looked up the imports.

This does work if you change it to a dynamic import:

<script>
	import Nav from '../components/Nav.svelte';

	export let InjectDevStyle;
	if (false) {
		import('../components/InjectDevStyle.svelte').then(mod => {
			InjectDevStyle = mod.default;
		});
	}

	export let segment;
</script>

{#if false}
  <svelte:component this="{InjectDevStyle}" />
{/if}

I wonder if that might be good enough for most use cases

@benmccann thanks for your solution with the dynamic import 👍
With this workaround I am able to upgrade to the newest version of sapper.

Still I would prefer to have it like in version 0.28.2. The way it was before looks cleaner and aligns better with the svelte-way of doing things 😉. But it's not a high priority for me.