-
Notifications
You must be signed in to change notification settings - Fork 80
Splitting out Sapper's CSS handling #29
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
Conversation
6019007
to
1c17488
Compare
87d0022
to
658e27f
Compare
The main reason for the current sapper-internal song and dance — which I completely agree is egregiously complicated and prevents useful things like additional transformations — is that we're able to extract styles into I worry that a change that puts CSS in JS strings instead will harm performance, partly because of the double-loading effect you mention in the RFC, but also because increasing JS payloads is inherently bad (rather than being able to fetch a |
I think it's pretty complicated in practice. While this would increase the JS payload it also reduces the number of requests and improves compression. E.g. things like the CSS class names will be shared between the JS and CSS. Also, compression works better on larger files than smaller ones. I tried to research this a bit. It doesn't seem like there's a clear winner one way or the other. The benchmarks I found showed concatenation to be an improvement even with HTTP/2, but probably aren't representative of a typical Sapper app.
https://medium.com/@jacobtan/understanding-http-2-and-its-caveats-1e8200519c4c:
https://spinupwp.com/performance-best-practices-http2/:
|
Agree that it's essentially an empirical question. I think the research around what happens when you serve 100+ small modules over HTTP/2 probably isn't hugely relevant to our case, since we're really talking about n JS modules (where n is typically 1-3 for a given navigation) vs n JS modules plus 1 CSS file — I'd naively expect it to take less time to fetch two 5kb resources than one 10kb resource, though I could be wildly wrong about that. The point about compression is well made though. Maybe we should try this approach on whatever reasonably-sized Sapper apps we have to hand (e.g. svelte.dev) and try and figure out a concrete answer to the question. Might be harder to pin down the effects of having more JS to parse, though I guess these are probably small (my understanding is that JS engines are quite fast at parsing strings). Something else that occurred to me is whether or not coarse-grained CSS chunks (the status quo) have better performance characteristics than adding a |
I did some benchmarking, which I wrote up in my first blog post in three years, and it looks like it no longer matters whether you load CSS first of JS first. This doesn't answer all the other questions, but at least means that's probably not a blocker here. |
For However, I have noticed a bit of a flash of unstyled content (FOUC). In webpack this is typically solved by:
It turns out that our very own @tanhauhau wrote one of the top search results for css code splitting that explains how
|
conceptually when you have this in the source code: import('./Foo.svelte').then(Foo => {
//
}); it will turn into Promise.all([
import('./Foo.js'),
loadCss('./Foo.css')
]).then(([Foo]) => {
//
});
function loadCss(css) {
return new Promise((resolve, reject) => {
const link = document.createElement('link');
link.href = css;
link.onload = resolve;
link.onerror = reject;
document.head.appendChild(link);
});
} For a client-side SPA, the HtmlWebpackPlugin will inject the entry css into head and entry js into end of the body. <html>
<head>
<link rel="stylesheet" href="entry.css" />
<head>
<body>
<script src="entry.js"></script>
</body>
</html> because the css is blocking, you kinda guaranteed that the style is there when you evaluating the now things get tricky with server rendering,
so you have to inline the styles instead.
this way, the styles are available and evaluated before parsing the html body. but how do you know what are the css files needed for the page, that will be identified based on the routes / js chunks used to render the content in server.
before appending the |
Would static imports get put into promises or converted to dynamic imports so that they can be run in parallel with E.g. today in hn.svelte.dev the Rollup outputs
|
so we are sending so you can have import "./api.5c9dd840.css";
import "./index.37804c28.css";
import "./Home.26cab2fc.css";
import{S as t,i as s,s as r,c as a,a as e,m as n,t as p,b as o,d as c}from"./client.0422cf2a.js";
import"./api.5c9dd840.js";
import"./index.37804c28.js";
import{H as i}from"./Home.26cab2fc.js"; |
I haven't thought through all the implications of this, but: If this made it so that no styles at all were loaded if the user had javascript disabled or if it crashed, this change would be pretty likely to be unacceptable. Extracting to real CSS files isn't just to keep the JS files down in size, it's to make sure the site degrades somewhat gracefully without javascript. Would that be possible with this? |
@tanhauhau I'm wondering how it gets transformed by webpack. I think there's not really such a thing as @Conduitry good thing to check on. Yes, it would degrade gracefully. During SSR it renders a |
ok i think im confused for a bit. so is this going to be the initial entry file, or a lazy chunk? in webpack, things are quite different because there's a module loading mechanism in webpack. it will ensure dependencies are loaded before executing it. so if we can have a corresponding
thus Promise.all([
import('./[p].8fa69d4d.js'),
import('./[p].8fa69d4d.css.js').then(mod => mod.default()),
]).then(([Foo]) => {
//
}); if it is the entry chunk, then, the css should be already been inlined in the html from ssr. |
Not necessarily an entry chunk, but not dynamically imported either. Most imports in Sapper with Rollup are ES6 static imports. I just realized this question might not make sense in the Webpack context because it looks like Webpack doesn't have the ability to output ES6 like Rollup does. I'm having a pretty hard time figuring out how to chunk and load CSS in Rollup in a way that's analogous to Webpack as a result, and so perhaps that shouldn't be the goal. I'll have to sleep on it. I think I know enough about how Webpack works now though to better understand some ways this might be approached. Interested if folks have any ideas on what the optimal way to do this in Rollup would be. Thanks again for all the help! |
The Sapper CSS Rollup plugin was updated to implement the functionality suggested in the RFC and I've updated the RFC accordingly to document the design motivation for posterity. The functionality still lives within Sapper and we could probably still benefit from moving it out in order to close out this effort |
Closing this since we've moved on from Sapper to SvelteKit |
Rendered