Skip to content

Compiler render_ssr does not respect css option #3604

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

Closed
JohnGurin opened this issue Sep 21, 2019 · 9 comments
Closed

Compiler render_ssr does not respect css option #3604

JohnGurin opened this issue Sep 21, 2019 · 9 comments
Labels
awaiting submitter needs a reproduction, or clarification

Comments

@JohnGurin
Copy link

JohnGurin commented Sep 21, 2019

svelte 3.12.1
https://svelte.dev/examples#styling

const css = options.customElement ?
{ code: null, map: null } :
component.stylesheet.render(options.filename, true);

A possible fix:

const css = options.css === false || options.customElement ?
	{ code: null, map: null } :
	component.stylesheet.render(options.filename, true);
@Conduitry
Copy link
Member

The documentation for the css option reads:

If true, styles will be included in the JavaScript class and injected at runtime. It's recommended that you set this to false and use the CSS that is statically generated, as it will result in smaller JavaScript bundles and better performance.

Perhaps this could be clarified to say that it only applies to the client-side code. But it has nothing to do with whether the .css value in the output from the compiler is present or not. You can use it or not, depending on whether you want it. The .css value is there in the output in both generate: 'browser' and generate: 'ssr' mode, whether or not you specify css: false, and this is intentional.

@JohnGurin
Copy link
Author

...But` it has nothing to do with whether the .css value in the output from the compiler is present or not...

It is not about a compiler's output. The compiler always emits both js and css.
It is about compiled code, which will be run on a server.

I don't see any reasons why

const css = {code: "...",  map: "..."} // <----- this
const Component = create_ssr_component(($$result, ...) => {
        $$result.css.add(css); // <---- and this
        return '...';
});

is in js.code compiler output when css:false

const App = require('./compiled-app')
const { html, css, head } = App.render({ answer: 42 }); // css should be null

@TehShrike
Copy link
Member

Playing around in the REPL, it looks like when you're generating SSR, the only effect that turning on customElement has is to prevent the CSS from being included in the JS output.

I'm not sure why the customElement toggle would have that effect for the SSR output?

@Conduitry Conduitry added the awaiting submitter needs a reproduction, or clarification label Sep 22, 2019
@Conduitry Conduitry reopened this Sep 22, 2019
@Rich-Harris
Copy link
Member

SSR doesn't really work with custom elements full-stop. (Part of the reason I never use them, and recommend that other people don't use them either.) If there's anything that needs to change there, it's that this...

svelte.compile(text, {
  customElement: true,
  generate: 'ssr'
});

...should result in an error being thrown.

To be clear, the css: true option is designed to only control whether the add_css function is created in a client-side component to inject styles with JavaScript, which means that the resulting module is portable (i.e. you can freely import it anywhere without worrying about having to wire up static CSS files). It's not designed to have any effect in SSR mode. Perhaps it should have been called injectCss or something instead.

Is the concern here that extra work is being done unnecessarily, or is the presence of a non-null rendered.css value actively harmful somehow?

@JohnGurin
Copy link
Author

JohnGurin commented Sep 22, 2019

If I build bundles for DOM and SSR from the same source, I extract CSS as an asset and inject it as a link in HTML.
Then I use SSR compiled app and I don't use CSS from app.render. SSR should be fast, what you say here is "let it be there for no reason"

Is the concern here that extra work is being done unnecessarily

Yep

@antony antony added the wontfix label Apr 9, 2020
@antony
Copy link
Member

antony commented Apr 9, 2020

Closing for now unless this is considered an issue that requires fixing.

@jimafisk
Copy link

My use case is the same as @JohnGurin:

I build bundles for DOM and SSR from the same source, I extract CSS as an asset and inject it as a link in HTML

For my project, I also unfortunately have to do some regex parsing of the SSR output before using it to create static HTML because I'm trying to do everything in v8. The const css = {code: "...", map: "..."} output makes this process harder and isn't being used at all.

Should this just be omitted in SSR by default? It's possible I'm reading @Rich-Harris's comment wrong:

It's not designed to have any effect in SSR mode.

At the very least it would be great if passing css: false worked when using generate: 'ssr'.

@NikolayMakhonin
Copy link

The documentation for the css option reads:

If true, styles will be included in the JavaScript class and injected at runtime. It's recommended that you set this to false and use the CSS that is statically generated, as it will result in smaller JavaScript bundles and better performance.

Perhaps this could be clarified to say that it only applies to the client-side code. But it has nothing to do with whether the .css value in the output from the compiler is present or not. You can use it or not, depending on whether you want it. The .css value is there in the output in both generate: 'browser' and generate: 'ssr' mode, whether or not you specify css: false, and this is intentional.

@Conduitry Why can't you remove the css from the server bundle? I have a bundle size of 5 megabytes only due to css. I think if you do not include css in the server bundle, it will speed up the build and reduce the size of the js files on the server.
I am trying to speed up svelte development so that the project will rebuild as quickly as possible after changing the code.

@aral
Copy link

aral commented Feb 23, 2022

FWIW, I thought setting css: false would have the same effect as @JohnGurin (#3604 (comment))

Use case:

Makes trying to ascertain whether JS has changed between builds harder when implementing live reload vs hot CSS injection. (I can check if CSS has changed by comparing the CSS returned from the compiler but I can’t compare if the JS has changed between builds without removing the generated superfluous CSS which I don’t use anyway as I inject the CSS into a style tag in my template.)

Update: In case it helps someone else, I don’t know how robust this is but currently I’m transforming the JS with the following regular expressions to remove the effects of the CSS for the purpose of being able to isolate JS and CSS changes between builds:

output.js.code
  .replace(/const css = \{.*?};/s, '')
  .replace('$$result.css.add(css);', '')
  .replace(/svelte-.*?"\}/g, '')

(You also have to replace the new CSS hash with the old one if you’re using it to inject CSS on changes like I am.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification
Projects
None yet
Development

No branches or pull requests

8 participants