Open ivanhofer opened 4 years ago
@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
__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.
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:Then i use this component in my
_layout.svelte
file like this: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
withfalse
and added adev.css
with following content:Run
npm run build
and see that the content ofdev.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, thedev.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.