From bcbdac71a83fedda22db7d50a2e1cbc3d9a21880 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Fri, 9 Aug 2024 16:09:06 +0100 Subject: [PATCH 1/6] fix: ensure custom element styles append correctly during prod --- .changeset/hip-goats-smoke.md | 5 +++ .../svelte/src/internal/client/dom/css.js | 36 +++++++++++++------ 2 files changed, 31 insertions(+), 10 deletions(-) create mode 100644 .changeset/hip-goats-smoke.md diff --git a/.changeset/hip-goats-smoke.md b/.changeset/hip-goats-smoke.md new file mode 100644 index 000000000000..ce023460b743 --- /dev/null +++ b/.changeset/hip-goats-smoke.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure custom element styles append correctly during prod diff --git a/packages/svelte/src/internal/client/dom/css.js b/packages/svelte/src/internal/client/dom/css.js index 1cccb42b3eec..17990055cf76 100644 --- a/packages/svelte/src/internal/client/dom/css.js +++ b/packages/svelte/src/internal/client/dom/css.js @@ -1,21 +1,14 @@ import { DEV } from 'esm-env'; -import { queue_micro_task } from './task.js'; +import { effect } from '../reactivity/effects.js'; -var seen = new Set(); +var css_counter = new Map(); /** * @param {Node} anchor * @param {{ hash: string, code: string }} css */ export function append_styles(anchor, css) { - // in dev, always check the DOM, so that styles can be replaced with HMR - if (!DEV) { - if (seen.has(css)) return; - seen.add(css); - } - - // Use `queue_micro_task` to ensure `anchor` is in the DOM, otherwise getRootNode() will yield wrong results - queue_micro_task(() => { + const maybe_append_styles = () => { var root = anchor.getRootNode(); var target = /** @type {ShadowRoot} */ (root).host @@ -29,5 +22,28 @@ export function append_styles(anchor, css) { target.appendChild(style); } + }; + + // Use an effect to ensure `anchor` is in the DOM, otherwise getRootNode() will yield wrong results + effect(() => { + // In dev, always check the DOM, so that styles can be replaced with HMR + if (DEV) { + maybe_append_styles(); + return; + } + // Otherwise, for prod we can use the css object as a key and count the usage to skip the lookup + var count = css_counter.get(css) ?? 0; + + css_counter.set(css, count + 1); + + if (count > 0) return; + + maybe_append_styles(); + + return () => { + var count = css_counter.get(css) - 1; + css_counter.set(css, count); + if (count === 0) css_counter.delete(css); + }; }); } From 8f45fc7b5efb6ce390cd253f8b150a55ad158340 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 10 Aug 2024 13:29:43 +0100 Subject: [PATCH 2/6] fix --- .../svelte/src/internal/client/dom/css.js | 50 ++++--------------- 1 file changed, 10 insertions(+), 40 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/css.js b/packages/svelte/src/internal/client/dom/css.js index 17990055cf76..b7a8ca68e851 100644 --- a/packages/svelte/src/internal/client/dom/css.js +++ b/packages/svelte/src/internal/client/dom/css.js @@ -1,49 +1,19 @@ -import { DEV } from 'esm-env'; -import { effect } from '../reactivity/effects.js'; - -var css_counter = new Map(); - /** * @param {Node} anchor * @param {{ hash: string, code: string }} css */ export function append_styles(anchor, css) { - const maybe_append_styles = () => { - var root = anchor.getRootNode(); - - var target = /** @type {ShadowRoot} */ (root).host - ? /** @type {ShadowRoot} */ (root) - : /** @type {Document} */ (root).head ?? /** @type {Document} */ (root.ownerDocument).head; - - if (!target.querySelector('#' + css.hash)) { - const style = document.createElement('style'); - style.id = css.hash; - style.textContent = css.code; - - target.appendChild(style); - } - }; - - // Use an effect to ensure `anchor` is in the DOM, otherwise getRootNode() will yield wrong results - effect(() => { - // In dev, always check the DOM, so that styles can be replaced with HMR - if (DEV) { - maybe_append_styles(); - return; - } - // Otherwise, for prod we can use the css object as a key and count the usage to skip the lookup - var count = css_counter.get(css) ?? 0; - - css_counter.set(css, count + 1); + var root = anchor.getRootNode(); - if (count > 0) return; + var target = /** @type {ShadowRoot} */ (root).host + ? /** @type {ShadowRoot} */ (root) + : /** @type {Document} */ (root).head ?? /** @type {Document} */ (root.ownerDocument).head; - maybe_append_styles(); + if (!target.querySelector('#' + css.hash)) { + const style = document.createElement('style'); + style.id = css.hash; + style.textContent = css.code; - return () => { - var count = css_counter.get(css) - 1; - css_counter.set(css, count); - if (count === 0) css_counter.delete(css); - }; - }); + target.appendChild(style); + } } From 049b0d0e13a965b0f2235d4036a6514a6dc135c7 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 10 Aug 2024 13:30:24 +0100 Subject: [PATCH 3/6] fix --- .../svelte/src/internal/client/dom/css.js | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/css.js b/packages/svelte/src/internal/client/dom/css.js index b7a8ca68e851..cf51b193d9e2 100644 --- a/packages/svelte/src/internal/client/dom/css.js +++ b/packages/svelte/src/internal/client/dom/css.js @@ -1,19 +1,24 @@ +import { queue_micro_task } from './task'; + /** * @param {Node} anchor * @param {{ hash: string, code: string }} css */ export function append_styles(anchor, css) { - var root = anchor.getRootNode(); + // Use `queue_micro_task` to ensure `anchor` is in the DOM, otherwise getRootNode() will yield wrong results + queue_micro_task(() => { + var root = anchor.getRootNode(); - var target = /** @type {ShadowRoot} */ (root).host - ? /** @type {ShadowRoot} */ (root) - : /** @type {Document} */ (root).head ?? /** @type {Document} */ (root.ownerDocument).head; + var target = /** @type {ShadowRoot} */ (root).host + ? /** @type {ShadowRoot} */ (root) + : /** @type {Document} */ (root).head ?? /** @type {Document} */ (root.ownerDocument).head; - if (!target.querySelector('#' + css.hash)) { - const style = document.createElement('style'); - style.id = css.hash; - style.textContent = css.code; + if (!target.querySelector('#' + css.hash)) { + const style = document.createElement('style'); + style.id = css.hash; + style.textContent = css.code; - target.appendChild(style); - } + target.appendChild(style); + } + }); } From 45e87b11d04002615b2b4ba49f3abea605c1385f Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 10 Aug 2024 13:31:32 +0100 Subject: [PATCH 4/6] fix --- packages/svelte/src/internal/client/dom/css.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/dom/css.js b/packages/svelte/src/internal/client/dom/css.js index cf51b193d9e2..5658de2c3368 100644 --- a/packages/svelte/src/internal/client/dom/css.js +++ b/packages/svelte/src/internal/client/dom/css.js @@ -1,4 +1,4 @@ -import { queue_micro_task } from './task'; +import { queue_micro_task } from './task.js'; /** * @param {Node} anchor From 232cf4a5995bc792cd9b676ea6236ee4e5cee358 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 10 Aug 2024 13:51:55 +0100 Subject: [PATCH 5/6] feedback --- .../phases/3-transform/client/transform-client.js | 4 +++- packages/svelte/src/internal/client/dom/css.js | 12 +++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 82d49f0ad7e6..d828a7c585ae 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -380,7 +380,9 @@ export function client_component(analysis, options) { state.hoisted.push(b.const('$$css', b.object([b.init('hash', hash), b.init('code', code)]))); component_block.body.unshift( - b.stmt(b.call('$.append_styles', b.id('$$anchor'), b.id('$$css'))) + b.stmt( + b.call('$.append_styles', b.id('$$anchor'), b.id('$$css'), b.literal(options.customElement)) + ) ); } diff --git a/packages/svelte/src/internal/client/dom/css.js b/packages/svelte/src/internal/client/dom/css.js index 5658de2c3368..c5493ca28498 100644 --- a/packages/svelte/src/internal/client/dom/css.js +++ b/packages/svelte/src/internal/client/dom/css.js @@ -1,10 +1,20 @@ +import { DEV } from 'esm-env'; import { queue_micro_task } from './task.js'; +var seen = new Set(); + /** * @param {Node} anchor * @param {{ hash: string, code: string }} css + * @param {boolean} is_custom_element */ -export function append_styles(anchor, css) { +export function append_styles(anchor, css, is_custom_element) { + // in dev, always check the DOM, so that styles can be replaced with HMR + if (!DEV && !is_custom_element) { + if (seen.has(css)) return; + seen.add(css); + } + // Use `queue_micro_task` to ensure `anchor` is in the DOM, otherwise getRootNode() will yield wrong results queue_micro_task(() => { var root = anchor.getRootNode(); From a09f81ff2cbfd1986e465ea858428bf880f8e13d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 10 Aug 2024 13:19:13 -0400 Subject: [PATCH 6/6] only add third argument for custom elements --- .../compiler/phases/3-transform/client/transform-client.js | 2 +- packages/svelte/src/internal/client/dom/css.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index d828a7c585ae..6ebb77f2e92c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -381,7 +381,7 @@ export function client_component(analysis, options) { component_block.body.unshift( b.stmt( - b.call('$.append_styles', b.id('$$anchor'), b.id('$$css'), b.literal(options.customElement)) + b.call('$.append_styles', b.id('$$anchor'), b.id('$$css'), options.customElement && b.true) ) ); } diff --git a/packages/svelte/src/internal/client/dom/css.js b/packages/svelte/src/internal/client/dom/css.js index c5493ca28498..dbf862543f1f 100644 --- a/packages/svelte/src/internal/client/dom/css.js +++ b/packages/svelte/src/internal/client/dom/css.js @@ -6,9 +6,9 @@ var seen = new Set(); /** * @param {Node} anchor * @param {{ hash: string, code: string }} css - * @param {boolean} is_custom_element + * @param {boolean} [is_custom_element] */ -export function append_styles(anchor, css, is_custom_element) { +export function append_styles(anchor, css, is_custom_element = false) { // in dev, always check the DOM, so that styles can be replaced with HMR if (!DEV && !is_custom_element) { if (seen.has(css)) return;