Skip to content

Conversation

@dummdidumm
Copy link
Member

@dummdidumm dummdidumm commented Jul 4, 2024

There are various use cases where this continues to be necessary/nice to have:

  • rendering OG cards
  • rendering emails
  • basically anything where you use render manually and want to quickly stitch together the CSS without setting up an elaborate tooling chain

It's not necessary in the common case though, which is why it's behind a compiler flag.

The compiler option name is subject to bikeshedding. I wanted it to begin with css so that all the CSS-related options share the same prefix.
One alternative I thought of was to enhance the css somehow to incorporate:

  • more string enums, like injected-on-server
  • make it optionally a tuple, where the first entry is for the client and the second for the server, so you could write ['external', 'injected']

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

There are various use cases where this continues to be necessary/nice to have:
- rendering OG cards
- rendering emails
- basically anything where you use `render` manually and want to quickly stitch together the CSS without setting up an elaborate tooling chain
@changeset-bot
Copy link

changeset-bot bot commented Jul 4, 2024

🦋 Changeset detected

Latest commit: a334755

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dominikg
Copy link
Member

dominikg commented Jul 4, 2024

what about a css subkey in compilerOptions and then css.generate , css.hash etc? On the output type css would be optional or set to empty string by default?

@Rich-Harris
Copy link
Member

Recapping discussion elsewhere: this ought to be something we can configure via <svelte:options>, because in many use cases (such as an OG card) you only want specific components to include CSS in the bundle. For cases where you do want the entire app to include CSS in the bundle (we've encountered this in the past with AMP) or where you need to set the option for components outside your immediate control (e.g. your OG card uses a third party library), you can do so with app-wide compiler options, or by using e.g. dynamicCompileOptions with vite-plugin-svelte.

This does make it tricky to put everything in a single namespace though, because <svelte:options> can only deal with simple expressions (nothing that needs to execute), and so it would be weird to allow cssHash there. It's not a huge problem, but it's a design challenge and a breaking change that seems best avoided if we can.

Honestly, I prefer the enum approach. I have to wonder if we can just use the existing injected/external pair — what if injected caused <style> elements to be added to the head in SSR, then reused in the client? In other words this...

<svelte:options css="injected" />

<h1>hello!</h1>

<style>
  h1 {
    color: red;
  }
</style>

...becomes this...

import * as $ from "svelte/internal/server";

var $$css = ['bt9zrl', `
  h1.svelte-bt9zrl {
    color: red;
  }
`];

export default function App($$payload) {
  $$payload.css.add($$css);
  $$payload.out += `<h1 class="svelte-bt9zrl">hello!</h1>`;
}

...yielding output like this...

rendered = render(App);

/*
{
  head: '<style data-svelte="bt9zrl">h1 {...}</style>',
  body: '<!--[--><h1 class="svelte-bt9zrl">hello</h1><!--]-->'
}
*/

...and the CSR output looks like this...

import * as $ from "svelte/internal/client";

var $$css = ['bt9zrl', `
  h1.svelte-bt9zrl {
    color: red;
  }
`];

var root = $.template(`<h1 class="svelte-bt9zrl">hello!</h1>`);

export default function App($$anchor) {
  $.append_styles($css);

  var h1 = root();

  $.append($$anchor, h1);
}

...where append_styles adds the <style data-svelte="bt9zrl"> if it doesn't already exist?

Feels like that would capture everyone's use case and make everything a lot more turnkey (e.g. avoiding the FOUC that you currently face if you use injected).

@dominikg
Copy link
Member

dominikg commented Jul 9, 2024

where would append_styles append the styles for custom element output?

how would this work with css hmr during dev? (style node already exists on update) For externalized css vite helps nicely with it but here we'd be on our own

@Rich-Harris
Copy link
Member

where would append_styles append the styles for custom element output?

the shadow root, as it does today

how would this work with css hmr during dev?

we can update the HMR code to replace the <style> contents in-situ, if they've changed

@dominikg
Copy link
Member

dominikg commented Jul 9, 2024

sounds like a plan 🌈

can we cache the fact that style was added already so creating multiple instances doesn't end up doing extra work? And want happens when the last instance is destroyed, does that remove the style node too?

@Rich-Harris
Copy link
Member

can we cache the fact that style was added already so creating multiple instances doesn't end up doing extra work?

yes!

And want happens when the last instance is destroyed, does that remove the style node too?

we could. it would be a departure from how things work today but it's possible — we just wrap everything in an effect and remove on teardown (if a counter reaches 0). It would create a discrepancy between injected/external that might confuse people — we sometimes get issues like 'why are my :global styles still on the page after I navigate elsewhere?', and the answer is that Vite controls whether the <link> is present; we don't have a way to remove it. If we removed it in the injected case but not the external case it would likely confuse people.

@Rich-Harris
Copy link
Member

opened #12374

@Rich-Harris
Copy link
Member

merged #12374, closing

@Conduitry Conduitry deleted the server-css branch July 25, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants