search

sveltejs / sapper-template

Starter template for Sapper apps
703 stars 214 forks source link

Emit css in modern build only #273

Open saylerb opened 4 years ago

saylerb commented 4 years ago

Currently the template rollup config emits CSS twice if you are running build with the --legacy flag. This is resulting in duplicate css getting served to legacy browsers like IE.

This change changes the config to only emit css on the first pass.

antony commented 4 years ago

@saylerb is this the right fix? I don't fully understand what emitCss does as I have proven to myself in the last few days, but if you don't emit css, what happens to it? Does it just end up written to the browser as part of the javascript? Or does it literally leave out css?

The docs don't make it any clearer for me.

Conduitry commented 4 years ago

I've just tested this, and yeah, for the legacy build, this makes the CSS appear inline in the JS, which isn't what we want either.

saylerb commented 4 years ago

@antony I have to admit I don't fully understand the emitCSS. But It appeared to me with it set to true, additional css files were getting written to the client/legacy/ folder that matched that in the client/. I can do a bit more digging into it

@Conduitry I can take a second pass. I don't think I saw the additional inline css.

saylerb commented 4 years ago

I looked into this briefly, I think there may be a bug inject_scripts in how it is resolving the paths. If the CSS handling is being re-written, should I hold off on looking more deeply into this until 0.29 comes out?

benmccann commented 4 years ago

There are not going to be any major changes to the CSS handling, so if there are any bugs that are there now they would continue to be present