-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 75e1dea The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
preview: https://svelte-dev-git-preview-svelte-14397-svelte.vercel.app/ this is an automated message |
|
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 |
that sounds like something we need to specifically handle in the playground/during hmr |
What should happen when someone has |
I think it should be removed, and if there's another component with |
will it be possible to make this work with 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 the <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 |
No, it would still be necessary to inject a Maybe the status quo is actually fine? |
I think it's still valuabe:
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). |
I agree with @dummdidumm It would be very valuable.
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
|
- revert to previous value on unmount, if it was the last one changing the value - special handling for classes: merge them
Updated the implementation to handle duplicates including always correctly reverting to the previous value upon removal, and handling classes as a special case. |
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 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 <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 And even for <!-- 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 |
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 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 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 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 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 There is also the pseudo Testing is easier with classes than with pseudo's: e.g. 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 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. |
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
'Setting classes' isn't really a feature. The feature is being able to style things, which can be accomplished with 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')
Classes have higher specificity. It's true that |
Isn't storing |
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. |
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 |
This was in the context of FOUC. I meant that in the context of FOUC and having to insert into
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
The point here is that I can just check the effect of the class by checking what classes are set directly on
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. |
closes #8663
Also closes #3105, because we're not adding that capability for
<svelte:body>
, because you can just add theclass
attribute to<svelte:html>
instead, and with #14714 you get more expressive power forclass
.Companion PRs:
<svelte:html>
blocks kit#13065<svelte:html>
into templates cli#365<svelte:html>
tutorial svelte.dev#1040Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.packages/svelte/src
, add a changeset (npx changeset
).Tests and linting
pnpm test
and lint the project withpnpm lint