svelte
svelte copied to clipboard
fix: allow attributes on the title element
The spec states the title element supports global attributes. https://html.spec.whatwg.org/#the-title-element
Reverts the title attribute validation introduced in #1721.
Fixes #5198.
Before submitting the PR, please make sure you do the following
- [x] 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
- [x] Prefix your PR title with
feat:,fix:,chore:, ordocs:. - [x] This message body should clearly illustrate what problems it solves.
- [x] Ideally, include a test that fails without this PR but passes with it.
- [x] If this PR changes code within
packages/svelte/src, add a changeset (npx changeset).
Tests and linting
The test plan is to re-use packages/svelte/tests/validator/samples/title-no-attributes. By removing errors.json, we test that svelte compiles the title element with attributes.
- [x] Run the tests with
pnpm testand lint the project withpnpm lint
The tests pass, but pnpm lint failed with:
> [email protected] lint /opt/p/svelte
> eslint && prettier --check .
Oops! Something went wrong! :(
ESLint: 9.9.1
Error: Cannot find module '/opt/p/svelte/node_modules/.pnpm/[email protected]_svelte@packages+svelte/node_modules/svelte/compiler/index.js'
at createEsmNotFoundErr (node:internal/modules/cjs/loader:1441:15)
at finalizeEsmResolution (node:internal/modules/cjs/loader:1430:15)
🦋 Changeset detected
Latest commit: 874011089b62bf98b91c53430297a0d4d0ccde24
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
Preview: https://svelte-dev-git-preview-svelte-15983-svelte.vercel.app/
Thank you. The aria-live thing seems like a good reason to have this but it's not quite as straightforward as this PR — you've removed the validation, but the <title> still won't have any attributes.
When you create a <title> in a component, Svelte doesn't actually create a <title> element and append it to the head, it instead sets document.title, because you're only supposed to have one. We could probably change that (each new <title> inside a <svelte:head> would be prepended to the <head> instead, and removed when the component was unmounted — browsers will do the right thing in that case) but we'd need to change the implementation in addition to removing the validation.
Thank you. The
aria-livething seems like a good reason to have this but it's not quite as straightforward as this PR — you've removed the validation, but the<title>still won't have any attributes.
Ah, I see. I'll take a crack at rendering the attributes. Should I limit attributes to a subset?
I don't think there's any reason to limit it, it's extra work to prevent something that someone might find useful in ways we don't anticipate.
I think the most interesting challenge here will be hydration, since in the (admittedly rare!) case of duplicate elements they need to be rendered in reverse order, so that 'later' <title> elements (e.g. in a +page.svelte) take precedence over 'earlier' ones (e.g. a fallback in a +layout.svelte). Quite why browsers work that way is a mystery, of course.
I added SSR title tag attributes by cribbing code from RegularElement in the latest commit. Client-side rendering is trickier, and I could use some pointers.
The TitleElement doesn't have a reference to the HTML element because we didn't need one before. To call build_set_attributes, I need a node_id, from context.state.node.
The code for adding attributes is inlined in the client RegularElement. I extracted the smallest chunk of code into TitleElement to make it work.
-
How should I go about getting the node_id for a TitleElement?
-
Should I use RegularElement to render TitleElement?
-
I'm still using the assignment to document.title to set the title value. Should I keep that approach or append the title element to the head element The challenge with appending is that spec states multiple title tags are not allowed: The title tag may be used in "head element containing no other title elements."