Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Revisit inline <style> issue #1115

Open
Conduitry opened this issue Mar 12, 2020 · 6 comments
Open

Revisit inline <style> issue #1115

Conduitry opened this issue Mar 12, 2020 · 6 comments

Comments

@Conduitry
Copy link
Member

In an attempt to fix #1076, I recently merged #1098, but this seems to have had an unpleasant side effect of moving a whole bunch of styles from .css files to the inline <style> block. Some stuff needs to be rethought here.

I asked Rich what he recalls being the 'critical' CSS that was supposed to end up in an inline <style> tag, and he said

the intent was that any component styles that were depended upon by the current route would be considered critical, along with (IIRC) any CSS that couldn't be definitively associated with one specific route, e.g. import './global.css'
actually no, wait
i'm not sure that's right
gah, you know what? i can't remember. i feel like in most situations it would just create a link for a CSS chunk, and then it could skip that chunk whenever it was lazily requested. i can't remember exactly what would cause something to be promoted to inline styles

so there's definitely some confusion here.

@artemjackson
Copy link
Contributor

artemjackson commented Mar 16, 2020

@Conduitry I think that sapper does not deal with critical css. It just inlines page styles if there are no prebuilt css assets, e.g. Webpack app without css-plugins set up. At least it looks like this.

Ouch! I've just understood that sapper works different with Rollup and Webpack.

@Conduitry
Copy link
Member Author

The change in #1098 was reverted in 0.27.11 released yesterday, leaving us with the original question from #1076 of why there are the same styles in two CSS files.

@Sidd27
Copy link

Sidd27 commented Apr 15, 2020

@Conduitry What kind of clarification you need on this one as this is foremost thing which is stopping use to use sapper for production env

@Conduitry
Copy link
Member Author

What needs clarification is the thing I mentioned in the initial issue description. As I said, it's not at all clear how the inlined critical styles are supposed to work, and this is definitely an adjacent question, because the previous attempt at removing the duplicate CSS files instead moved way too many styles into the inline <style> tag.

@Sidd27
Copy link

Sidd27 commented Jun 28, 2020

@Conduitry for now can't we give developer a option to mention which all css he will need to be inlined just like a global option we can give inline option

@benmccann
Copy link
Member

benmccann commented Jul 25, 2020

#1269 may somewhat obviate the need for this. If you use a CDN, then the preload header will probably cause the CSS to be server pushed to you, which is probably better than inline because it can be cached. Though Luke also shared a Chrome issue tracker where they're discussing removing support for server push, so maybe it's only a temporary help 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants