Skip to content
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

feat: add <svelte:html> element #14397

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from
Draft

feat: add <svelte:html> element #14397

wants to merge 22 commits into from

Conversation

dummdidumm
Copy link
Member

@dummdidumm dummdidumm commented Nov 21, 2024

closes #8663

Also closes #3105, because we're not adding that capability for <svelte:body>, because you can just add the class attribute to <svelte:html> instead, and with #14714 you get more expressive power for class.

Companion PRs:

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.
  • If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

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

Copy link

changeset-bot bot commented Nov 21, 2024

🦋 Changeset detected

Latest commit: 75e1dea

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

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

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

@Rich-Harris
Copy link
Member

preview: https://svelte-dev-git-preview-svelte-14397-svelte.vercel.app/

this is an automated message

Copy link
Contributor

Playground

pnpm add https://pkg.pr.new/svelte@14397

@Leonidaz
Copy link

Nice! works as expected.

one minor issue that I noticed in the playground, if you add an attribute and then erase it from the code, the attribute stays in the <html>, unless you set the attribute to undefined and then it's removed.

@dummdidumm
Copy link
Member Author

that sounds like something we need to specifically handle in the playground/during hmr

@dummdidumm
Copy link
Member Author

What should happen when someone has <svelte:html foo="bar" /> in a component, and that component is destroyed? Does foo="bar" stay as the attribute on the HTML tag, or is it removed?

@Rich-Harris
Copy link
Member

I think it should be removed, and if there's another component with <svelte:html foo="baz" /> that was previously overridden, the attribute should change to that instead. Which admittedly could be awkward to implement

@ollema
Copy link

ollema commented Nov 29, 2024

will it be possible to make this work with localStorage to prevent FOUC upon initial page load?

not sure how to describe the problem in the best way. but currently, if you want to configure the dark/light mode, you can achieve this with e.g. the mode-watcher package.

the mode-watcher inserts a stringified function into the head that sets the correct <html> class based on localStorage and/or prefers-color-scheme, basically something along these lines:

<svelte:head>
  {@html `<script>(` +
    setInitialMode.toString() +
    `)(` +
    args +
    `);</script>`}
</svelte:head>

where:

export function setInitialMode(defaultMode: Mode, themeColors?: ThemeColors) {
  const rootEl = document.documentElement;
  const mode = localStorage.getItem('mode-watcher-mode') || defaultMode;
  const light =
    mode === 'light' ||
    (mode === 'system' && window.matchMedia('(prefers-color-scheme: light)').matches);

  rootEl.classList[light ? 'remove' : 'add']('dark');
  rootEl.style.colorScheme = light ? 'light' : 'dark';

  localStorage.setItem('mode-watcher-mode', mode);
}

but this approach is a bit brittle and it would be great if it could be simplified!

it is also complicated if you want to use nonce, then you can not use the mode-watcher component out of the box

@Rich-Harris
Copy link
Member

No, it would still be necessary to inject a <script> into the head. Which... now that I think about it does make me question how valuable this feature really is, especially when we consider all the additional complexity it entails (figuring out how to revert to a previous attribute value when one instance is removed, combining classes from different components, adding new stuff to the render return argument, etc).

Maybe the status quo is actually fine?

@dummdidumm
Copy link
Member Author

I think it's still valuabe:

  • works without JS
  • no idea how browsers behave if you change the lang attribute midway through on the client
  • an API everyone knows, compared to coming up with script tag workarounds which may all look a bit different

The fact that there are tricky things to figure out isn't really an argument against it to me, if we don't answer these questions / figure them out then we're essentially saying "good luck user, you gotta figure it out" - we question of "how to handle duplicate attributes" isn't solved in user land (even harder there, probably).

@Leonidaz
Copy link

Leonidaz commented Nov 30, 2024

document.documentElement

I agree with @dummdidumm It would be very valuable.


will it be possible to make this work with localStorage to prevent FOUC upon initial page load?

not sure how to describe the problem in the best way. but currently, if you want to configure the dark/light mode, you can achieve this with e.g. the mode-watcher package.

the mode-watcher inserts a stringified function into the head that sets the correct <html> class based on localStorage and/or prefers-color-scheme, basically something along these lines:

<svelte:head>
  {@html `<script>(` +
    setInitialMode.toString() +
    `)(` +
    args +
    `);</script>`}
</svelte:head>

where:

export function setInitialMode(defaultMode: Mode, themeColors?: ThemeColors) {
  const rootEl = document.documentElement;
  const mode = localStorage.getItem('mode-watcher-mode') || defaultMode;
  const light =
    mode === 'light' ||
    (mode === 'system' && window.matchMedia('(prefers-color-scheme: light)').matches);

  rootEl.classList[light ? 'remove' : 'add']('dark');
  rootEl.style.colorScheme = light ? 'light' : 'dark';

  localStorage.setItem('mode-watcher-mode', mode);
}

but this approach is a bit brittle and it would be great if it could be simplified!

it is also complicated if you want to use nonce, then you can not use the mode-watcher component out of the box

I think it would be possible to avoid FOUC in this use case given that:

At the very least, if cookies are used, the returning visitors can get the behavior without FOUC.


I created a playground with an emulated <svelte:html> via a component as a proof of concept. The main logic is in the SvelteHtml.svelte component and the rest is just for a visual demo. Just wanted to see how difficult it would be to support. Obviously, this is a contrived example outside of svelte's internal code, perhaps the logic might be useful.

  • The attributes are added in the order of component instantiation.
  • If a component is removed and it was overwriting its "predecessors" / parents' attributes, then the predecessor's attributes are restored.
  • Pre-existing attributes added outside of the emulated <svelte:html> remain unless any one of the components overwrites them. I don't think figuring out how to deal with restoring attributes outside of the <svelte:html> makes sense as it can result in various racing conditions.
  • setting an attribute to undefined or null, removes it from the html attributes, unless a component downstream provides some kind of value
  • setting attribute to an empty string, keeps the attribute, the browser just sets the attribute in this case.
  • class as a special case, attribute remains with the predecessor classes are restored on destroy; can be expanded to support style with a different string parser.

Playground: emulated <svelte:html>

- revert to previous value on unmount, if it was the last one changing the value
- special handling for classes: merge them
@dummdidumm
Copy link
Member Author

Updated the implementation to handle duplicates including always correctly reverting to the previous value upon removal, and handling classes as a special case.

dummdidumm added a commit to sveltejs/svelte.dev that referenced this pull request Dec 18, 2024
@Rich-Harris
Copy link
Member

I wish I'd pushed back a little harder when I previously expressed doubts, because I'm looking at concrete use cases and I'm still in the 'we shouldn't do this' camp. Sorry, I'm mad at myself for standing by while all this work was happening.

It does become slightly easier to set lang using this. But I can't think of a single other valid use case. The proposed tutorial uses it for toggling dark mode, but we absolutely mustn't teach people that that's the way to do it, because it isn't — it's an approach that leads to FOUC, and it wouldn't do to add a disclaimer that says 'syke, don't actually do this, it's bad'.

Here's how you might implement a simple FOUC-less theme toggle today:

<script lang="ts">
  function toggle() {
    let theme = document.documentElement.getAttribute('data-theme');

    theme = theme === 'dark' ? 'light' : 'dark';
    localStorage.setItem('mysite:theme', theme);
    document.documentElement.setAttribute('data-theme', theme);
  }
</script>

<svelte:head>
  <script>
    const theme =
      localStorage.getItem('mysite:theme') ??
      (window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light');

    document.documentElement.setAttribute('data-theme', theme);
  </script>
</svelte:head>

<button onclick={toggle}>...</button>

And here's how you'd need to implement it using <svelte:html>:

<script lang="ts">
+  let theme = $state<'dark' | 'light'>(
+    typeof document !== 'undefined' ? document.documentElement.getAttribute('data-theme') : undefined;
+  );

  function toggle() {
-    let theme = document.documentElement.getAttribute('data-theme');

    theme = theme === 'dark' ? 'light' : 'dark';
    localStorage.setItem('mysite:theme', theme);
-    document.documentElement.setAttribute('data-theme', theme);
  }
</script>

+<svelte:html data-theme={theme} />

<svelte:head>
  <script>
    const theme =
      localStorage.getItem('mysite:theme') ??
      (window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light');

    document.documentElement.setAttribute('data-theme', theme);
  </script>
</svelte:head>

<button onclick={toggle}>...</button>

This isn't just more code, it's more confusing code. We're reading from the DOM on hydration, being careful to avoid blowing up in SSR, just so that we can set the value back to what it already is.

If I'm right and there aren't any real use cases besides lang, then this feels like using a sledgehammer to crack a nut. It seems likely that there's an approach that's better suited to solving that specific problem, without all the attendant complexity (like maintaining an ordered history, special-casing class, and the extra bytes required to solve all these problems).

And even for lang, this feels pretty dang weird — having <html> and <head> as siblings:

<!-- src/routes/+layout.svelte -->
<script>
  import { page } from '$app/state';

  let { data, children } = $props();
</script>

<svelte:html lang={data.lang} />

<svelte:head>
  <title>{page.data.title ?? 'My site'}</title>
  <meta name="description" content="{page.data.description}">
</svelte:head>

I do think this exploration was worthwhile, but I don't think <svelte:html> is a net win. I think we should investigate approaches that make i18n feel like more of a first-class part of Svelte and SvelteKit's design.

@Leonidaz
Copy link

I can't speak of the implementation complexity but having an ability to easily set html attributes is a win.

Server-side, yes there are workarounds, in SvelteKit we can use a hook to do some replacements on html but it's not very flexible and everything has to be known in one place. With <svelte:html> components are allowed to have logic to make such decisions.

Certain things like setting a theme, or anything that avoids FOUC, is not a perfect solution and still currently requires script in the head and direct DOM access. I agree that the tutorial should be adjusted. But if client hint headers, such as Sec-CH-Prefers-Color-Scheme, are to become supported by Safari and Firefox (Chrome and Edge do support though), the solution will no longer require additional <script> in the <head>.

Continuing the server-side from the above, depending on an implementation, when someone is logged in, or preference cookies are sent from client, or client hint headers present, you can omit the client-side <head> code so it's not even loaded to the browser.

I think the ease of setting classes is also very useful. And especially with #14714

Any component can now easily set classes and use svelte's reactivity by simply using <html:element>. Yes, we can directly access DOM and set classes that way but it's a workaround, not very intuitive inside a framework or elegant and requires more boilerplate and just overall a bigger cognitive load.

The management of classes added to html on component destruction is also now taken care of, especially with conditionally rendering components. Any components that added classes with <svelte:html> once destroyed, these classes or attributes that were added will also be removed. This way you don't need to create custom code to access DOM to remove what was added.

There is also the pseudo :root and :has styling in css but it's also not as straightforward as just setting classes. As you have to add something in html to base :has on vs just adding a class. Also, :has() calculation, may be more computationally intensive as the browser has to traverse html, especially with large docs. It's a tiny argument but the cognitive load of creating and reading CSS is also increased :root:has(.something) { ... } vs .something {}.

Testing is easier with classes than with pseudo's: e.g. cy.get('html').should('have.class', 'something');

The CSS specificity is higher with pseudos vs simple classes, and it becomes harder to override these styles later.

It's easier to use with tailwind as it's class based and so it's easy to add the classes to <html> if needed vs figuring out pseudo's. Yes, it does have a has selector but it's more learning and figuring out how to use it properly.

Overall, it's arguably a small feature but for those who want to use it I really think it makes Svelte a more attractive framework and definitely improves the quality of life by reducing our cognitive load and makes it more pleasureful to work with Svelte.

@Rich-Harris
Copy link
Member

But if client hint headers, such as Sec-CH-Prefers-Color-Scheme, are to become supported by Safari and Firefox (Chrome and Edge do support though), the solution will no longer require additional <script> in the <head>.

Untrue — it's common for a theme toggle to cycle between dark, light and system, so that users can express a preference on a per-site basis. https://svelte.dev does this, for example. We also store the user's font preference in localStorage, and there's no Sec-CH-Prefers-Nice-Fonts header.

I think the ease of setting classes is also very useful

'Setting classes' isn't really a feature. The feature is being able to style things, which can be accomplished with :has. Conditionally setting Tailwind classes on the root element feels quite niche.

I also don't buy the testing argument. I don't know what the correct Cypress syntax is but I'm sure there's something like this:

-cy.get('html').should('have.class', 'something')
+cy.get('html').should('contain.selector', 'something')

The CSS specificity is higher with pseudos vs simple classes

Classes have higher specificity. It's true that :has(...) adds the specificity in the ..., but you can easily control that with :where.

@7nik
Copy link

7nik commented Dec 19, 2024

Isn't storing theme in cookies, in addition to the account settings, better? Thus, with transformPageChunk you can set it on the server as well. And this solution will work with disabled JS.
The lang is often embedded into the URL, and you can also use cookies.
The downsides are only a bit more coding and, time to time, having to update cookie's expiration.

@ollema
Copy link

ollema commented Dec 19, 2024

the transformPageChunk + cookie approach only works with SSR right? so if you have a static site you will have to restort to adding a FOUC prevention snippet to your head anyways?

also - are cookies available everywhere? I have a vague memory of cookies not being straightforward for e.g. native apps with capacitor. there localStorage is preferred IIRC.

@Rich-Harris
Copy link
Member

Indeed — prerendered pages, like most of https://svelte.dev, can't use cookies. If you can use an SSR-friendly solution then you can put the markup anywhere and use :has; if you can't, then you need to use a blocking script, ideally in the <head>. Either way, <svelte:html> doesn't really help

@Leonidaz
Copy link

But if client hint headers, such as Sec-CH-Prefers-Color-Scheme, are to become supported by Safari and Firefox (Chrome and Edge do support though), the solution will no longer require additional <script> in the <head>.

Untrue — it's common for a theme toggle to cycle between dark, light and system, so that users can express a preference on a per-site basis. https://svelte.dev does this, for example. We also store the user's font preference in localStorage, and there's no Sec-CH-Prefers-Nice-Fonts header.

This was in the context of FOUC. I meant that in the context of FOUC and having to insert into <svelte:head> to avoid it. Yes, you still need code for switching themes. But if there is no need for inserting <script> into <head>, you can just use <svelte:html> in a component to set the attribute with just a signal - more elegant than messing with the DOM.

I think the ease of setting classes is also very useful

'Setting classes' isn't really a feature. The feature is being able to style things, which can be accomplished with :has. Conditionally setting Tailwind classes on the root element feels quite niche.

Yeah, obviously it's about the styling but the point is that it's easier just to set classes vs having to mess around with pseudos. Let's say I'm just using classes in my workflow, which is the most common way, and then if I need to target <html> suddenly I need to start thinking how to do that and look up documentation or search online and then figure out that I have to access the DOM directly or use :root and :has and then maybe :where and then test it all in a different way than I'm used to. It's the point that I've been making, yes, you can do it without classes / without <html> but it can add a lot of cognitive load. One of the things that makes Svelte so attractive is I have to write less code and run into less gotchas, it makes it easy for me.

I also don't buy the testing argument. I don't know what the correct Cypress syntax is but I'm sure there's something like this:

-cy.get('html').should('have.class', 'something')
+cy.get('html').should('contain.selector', 'something')

The point here is that I can just check the effect of the class by checking what classes are set directly on <html> vs I have to find some element that set something and then perhaps also fully make sure that the actual styles were set on the html by checking the computed styles. Doable, yes, but again, it's again more a cognitive load.

The CSS specificity is higher with pseudos vs simple classes

Classes have higher specificity. It's true that :has(...) adds the specificity in the ..., but you can easily control that with :where.

I mean it's just harder:

<style>
	.foo {
		color: red;
	}

	:has(:where(.bar)) {
		color: blue;
	}

	:has(.baz) {
		color: blue;
	}
</style>

vs classes

<style>
	.foo {
		color: red;
	}

	.baz {
		color: blue;
	}
</style>

and reading the output is not fun:

	.svelte-1ffc6t5:has(:where(.bar:where(.svelte-1ffc6t5))) {
		color: blue;
	}

	.svelte-1ffc6t5:has(.baz:where(.svelte-1ffc6t5)) {
		color: blue;
	}

vs classes:

	.foo.svelte-1ffc6t5 {
		color: red;
	}

	.baz.svelte-1ffc6t5 {
		color: blue;
	}

If you think that this feature will be buggy with hydration, etc. and hard to ever fix then it's not worth keeping. Otherwise, since it's already been built, this definitely makes my life easier and that's what I love about Svelte.

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.

Add <svelte:html> special element Change body class via <svelte:body />
5 participants