Don't use conditionals inside the head #14
Conversation
|
Encountered a related issue perhaps when upgrading to svelte5 and using the Head component. THe meta tags are not closing correctly. It might be related to this nuance https://svelte.dev/docs/svelte/v5-migration-guide#Other-breaking-changes-null-and-undefined-become-the-empty-string where null values are perhaps being converted to empty strings? |
|
@arcataroger before trying a possible fix, I need to understand what we were doing wrong.. does our starter reproduce the problem? if not, then what triggers it? can we get an example that reproduces it? |
|
Thank you @emileswain! We will investigate. @stefanoverna Left you details in Basecamp |
| const serializedAttributes: string[] = | ||
| attributes && typeof attributes === 'object' | ||
| ? Object.entries(attributes).flatMap(([key, value]) => | ||
| value ? `${escapeHtmlString(key)}="${escapeHtmlString(value)}"` : [] | ||
| ) | ||
| : []; |
There was a problem hiding this comment.
I think this is more modern & easier to read than:
const serializedAttrs = [];
if (attrs) {
for (const attrName in attrs) {
if (Object.prototype.hasOwnProperty.call(attrs, attrName)) {
const value = attrs[attrName];
if (value) {
serializedAttrs.push(
`${escapeString(attrName)}="${escapeString(value)}"`,
);
}
}
}
}Is that OK? Object.entries() should already only enumerate only an object's own props, not anything it inherits: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/entries#description
It should supercede both Object.hasOwnProperty.call() and Object.hasOwn() (es2022)
| export const escapeHtmlString = (html: string) => { | ||
| const escaped = html | ||
| .replace( | ||
| /&(?!amp;|lt;|gt;|quot;|apos;)/g, // Don't re-encode these entities |
There was a problem hiding this comment.
edge case caught by a test; hopefully unlikely in real-world usage
| @@ -0,0 +1,13 @@ | |||
| /** Replaces special chars (&<>"') with HTML entities */ | |||
| export const escapeHtmlString = (html: string) => { | |||
There was a problem hiding this comment.
I think we have several copies of this func (or very similar ones) across several repos? Not a high priority since it's so simple, but maybe we should extract it out into a @datocms/utils lib. Maybe also test it against a proper HTML sanitization lib to make sure we're covering all the cases?
|
@stefanoverna Added some tests. One big change at https://github.com/datocms/datocms-svelte/pull/14/files?diff=split&w=1#r1876437362 and other minor cleanup throughout. @emileswain For now, in here, we'll just patch around it by serializing the head. But I think we should open a new issue with Svelte directly to continue discussing the |
Makes sense, fyi, sorting the seotags so content elements come last, worked as a temporary fix. |
To work around hydration errors, we render the tags as static escaped strings See: #14
|
Thanks, merged and squashed as ec728a1 |
|
@emileswain FYI I was trying to replicate this in a fresh minimal example for the Svelte team, but I could not. Turns out our starter kit just had some old dependencies. Upgrading If you already implemented |
It looks like we're experiencing a hydration error with the Head component in the Svelte Starter Kit (https://www.datocms.com/marketplace/starters/sveltekit-starter-kit) when loading.
It could be that the conditionals are causing Svelte to generate some HTML comments as hydration markers, which might result in a mismatch between what the server outputs and what the client sees.