From ca74fcd129b723c56420df0ce38274770092b8bf Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 11 Feb 2025 23:35:20 +0100 Subject: [PATCH 1/3] fix: use `importNode` to clone templates for Firefox --- .changeset/slow-meals-wait.md | 5 ++++ .../phases/2-analyze/visitors/Attribute.js | 5 ---- .../client/visitors/RegularElement.js | 5 ---- .../client/dom/elements/attributes.js | 25 ------------------- .../src/internal/client/dom/operations.js | 4 +++ .../src/internal/client/dom/template.js | 4 +-- packages/svelte/src/internal/client/index.js | 1 - .../svelte/src/internal/client/runtime.js | 3 ++- .../_expected/client/index.svelte.js | 6 +---- 9 files changed, 14 insertions(+), 44 deletions(-) create mode 100644 .changeset/slow-meals-wait.md diff --git a/.changeset/slow-meals-wait.md b/.changeset/slow-meals-wait.md new file mode 100644 index 000000000000..e1408e384929 --- /dev/null +++ b/.changeset/slow-meals-wait.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: use `importNode` to clone templates for Firefox diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js index 41144fc74c5c..42e449896928 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js @@ -23,11 +23,6 @@ export function Attribute(node, context) { if (node.name === 'value' && parent.name === 'option') { mark_subtree_dynamic(context.path); } - - // special case - if (node.name === 'loading' && parent.name === 'img') { - mark_subtree_dynamic(context.path); - } } if (is_event_attribute(node)) { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index c7e218d52143..98036aa9b609 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -300,11 +300,6 @@ export function RegularElement(node, context) { build_class_directives(class_directives, node_id, context, is_attributes_reactive); build_style_directives(style_directives, node_id, context, is_attributes_reactive); - // Apply the src and loading attributes for elements after the element is appended to the document - if (node.name === 'img' && (has_spread || lookup.has('loading'))) { - context.state.after_update.push(b.stmt(b.call('$.handle_lazy_img', node_id))); - } - if ( is_load_error_element(node.name) && (has_spread || has_use || lookup.has('onload') || lookup.has('onerror')) diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 987d1f2086e3..2dba2d797a4a 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -523,28 +523,3 @@ function srcset_url_equal(element, srcset) { ) ); } - -/** - * @param {HTMLImageElement} element - * @returns {void} - */ -export function handle_lazy_img(element) { - // If we're using an image that has a lazy loading attribute, we need to apply - // the loading and src after the img element has been appended to the document. - // Otherwise the lazy behaviour will not work due to our cloneNode heuristic for - // templates. - if (!hydrating && element.loading === 'lazy') { - var src = element.src; - // @ts-expect-error - element[LOADING_ATTR_SYMBOL] = null; - element.loading = 'eager'; - element.removeAttribute('src'); - requestAnimationFrame(() => { - // @ts-expect-error - if (element[LOADING_ATTR_SYMBOL] !== 'eager') { - element.loading = 'lazy'; - } - element.src = src; - }); - } -} diff --git a/packages/svelte/src/internal/client/dom/operations.js b/packages/svelte/src/internal/client/dom/operations.js index 627bf917eee1..83565d17ae68 100644 --- a/packages/svelte/src/internal/client/dom/operations.js +++ b/packages/svelte/src/internal/client/dom/operations.js @@ -11,6 +11,9 @@ export var $window; /** @type {Document} */ export var $document; +/** @type {boolean} */ +export var is_firefox; + /** @type {() => Node | null} */ var first_child_getter; /** @type {() => Node | null} */ @@ -27,6 +30,7 @@ export function init_operations() { $window = window; $document = document; + is_firefox = /Firefox/.test(navigator.userAgent); var element_prototype = Element.prototype; var node_prototype = Node.prototype; diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js index 3e4f45aba862..6ff3b0fa19a0 100644 --- a/packages/svelte/src/internal/client/dom/template.js +++ b/packages/svelte/src/internal/client/dom/template.js @@ -1,6 +1,6 @@ /** @import { Effect, TemplateNode } from '#client' */ import { hydrate_next, hydrate_node, hydrating, set_hydrate_node } from './hydration.js'; -import { create_text, get_first_child } from './operations.js'; +import { create_text, get_first_child, is_firefox } from './operations.js'; import { create_fragment_from_html } from './reconciler.js'; import { active_effect } from '../runtime.js'; import { TEMPLATE_FRAGMENT, TEMPLATE_USE_IMPORT_NODE } from '../../../constants.js'; @@ -48,7 +48,7 @@ export function template(content, flags) { } var clone = /** @type {TemplateNode} */ ( - use_import_node ? document.importNode(node, true) : node.cloneNode(true) + use_import_node || is_firefox ? document.importNode(node, true) : node.cloneNode(true) ); if (is_fragment) { diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 0d64b60496c6..d78f6d452e84 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -35,7 +35,6 @@ export { set_attributes, set_custom_element_data, set_xlink_attribute, - handle_lazy_img, set_value, set_checked, set_selected, diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index d5d01c9b6db8..75d45d9db9fd 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -39,6 +39,7 @@ import { set_component_context, set_dev_current_component_function } from './context.js'; +import { is_firefox } from './dom/operations.js'; const FLUSH_MICROTASK = 0; const FLUSH_SYNC = 1; @@ -333,7 +334,7 @@ export function handle_error(error, effect, previous_effect, component_context) current_context = current_context.p; } - const indent = /Firefox/.test(navigator.userAgent) ? ' ' : '\t'; + const indent = is_firefox ? ' ' : '\t'; define_property(error, 'message', { value: error.message + `\n${component_stack.map((name) => `\n${indent}in ${name}`).join('')}\n` }); diff --git a/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js index 9b203b97e82d..46d376aca2f9 100644 --- a/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js @@ -42,12 +42,8 @@ export default function Skip_static_subtree($$anchor, $$props) { $.reset(select); var img = $.sibling(select, 2); - var div_2 = $.sibling(img, 2); - var img_1 = $.child(div_2); - $.reset(div_2); + $.next(2); $.template_effect(() => $.set_text(text, $$props.title)); - $.handle_lazy_img(img); - $.handle_lazy_img(img_1); $.append($$anchor, fragment); } \ No newline at end of file From 220aba84f73e8f4df9351a4ef556ff6ec37b5cd0 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 12 Feb 2025 09:09:08 +0100 Subject: [PATCH 2/3] fix: move `is_firefox` check to line 28 --- packages/svelte/src/internal/client/dom/template.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js index 6ff3b0fa19a0..4bd1ad4e4885 100644 --- a/packages/svelte/src/internal/client/dom/template.js +++ b/packages/svelte/src/internal/client/dom/template.js @@ -25,7 +25,7 @@ export function assign_nodes(start, end) { /*#__NO_SIDE_EFFECTS__*/ export function template(content, flags) { var is_fragment = (flags & TEMPLATE_FRAGMENT) !== 0; - var use_import_node = (flags & TEMPLATE_USE_IMPORT_NODE) !== 0; + var use_import_node = (flags & TEMPLATE_USE_IMPORT_NODE) !== 0 || is_firefox; /** @type {Node} */ var node; @@ -48,7 +48,7 @@ export function template(content, flags) { } var clone = /** @type {TemplateNode} */ ( - use_import_node || is_firefox ? document.importNode(node, true) : node.cloneNode(true) + use_import_node ? document.importNode(node, true) : node.cloneNode(true) ); if (is_fragment) { From 200bb4c787ce79ac4ee4e75c85469a31815cbebc Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Wed, 12 Feb 2025 11:13:10 +0100 Subject: [PATCH 3/3] fix: revert using `is_firefox` too soon --- packages/svelte/src/internal/client/dom/template.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js index 4bd1ad4e4885..6ff3b0fa19a0 100644 --- a/packages/svelte/src/internal/client/dom/template.js +++ b/packages/svelte/src/internal/client/dom/template.js @@ -25,7 +25,7 @@ export function assign_nodes(start, end) { /*#__NO_SIDE_EFFECTS__*/ export function template(content, flags) { var is_fragment = (flags & TEMPLATE_FRAGMENT) !== 0; - var use_import_node = (flags & TEMPLATE_USE_IMPORT_NODE) !== 0 || is_firefox; + var use_import_node = (flags & TEMPLATE_USE_IMPORT_NODE) !== 0; /** @type {Node} */ var node; @@ -48,7 +48,7 @@ export function template(content, flags) { } var clone = /** @type {TemplateNode} */ ( - use_import_node ? document.importNode(node, true) : node.cloneNode(true) + use_import_node || is_firefox ? document.importNode(node, true) : node.cloneNode(true) ); if (is_fragment) {