From 6eab0fd6eb4a8ab83f7c12426bceef7ed6de69ab Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 19 Oct 2024 12:10:03 +0100 Subject: [PATCH 1/5] fix: avoid chromium issue with dispatching blur on element removal --- .changeset/seven-dancers-brush.md | 5 +++++ package.json | 4 +--- packages/svelte/src/internal/client/reactivity/effects.js | 6 ++++++ 3 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 .changeset/seven-dancers-brush.md diff --git a/.changeset/seven-dancers-brush.md b/.changeset/seven-dancers-brush.md new file mode 100644 index 000000000000..8f0bcd694005 --- /dev/null +++ b/.changeset/seven-dancers-brush.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: avoid chromium issue with dispatching blur on element removal diff --git a/package.json b/package.json index b213cfa94e9b..fd31052f7aff 100644 --- a/package.json +++ b/package.json @@ -6,9 +6,7 @@ "type": "module", "license": "MIT", "packageManager": "pnpm@9.4.0", - "engines": { - "pnpm": "^9.0.0" - }, + "repository": { "type": "git", "url": "git+https://github.com/sveltejs/svelte.git" diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 73286861d2b5..9385555f3eb8 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -375,7 +375,12 @@ export function destroy_effect(effect, remove_dom = true) { /** @type {TemplateNode | null} */ var node = effect.nodes_start; var end = effect.nodes_end; + var previous_reaction = active_reaction; + // Really we only need to do this in Chromium because of https://chromestatus.com/feature/5128696823545856, + // as removal of the DOM can cause sync `blur` events to fire, which can cause logic to run inside + // the current `active_reaction`, which isn't what we want at all + set_active_reaction(null) while (node !== null) { /** @type {TemplateNode | null} */ var next = node === end ? null : /** @type {TemplateNode} */ (get_next_sibling(node)); @@ -383,6 +388,7 @@ export function destroy_effect(effect, remove_dom = true) { node.remove(); node = next; } + set_active_reaction(previous_reaction) removed = true; } From ec68c95aa7c2dde23fb957749418a3fc55c4056e Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 19 Oct 2024 12:10:56 +0100 Subject: [PATCH 2/5] fix: avoid chromium issue with dispatching blur on element removal --- package.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index fd31052f7aff..b213cfa94e9b 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,9 @@ "type": "module", "license": "MIT", "packageManager": "pnpm@9.4.0", - + "engines": { + "pnpm": "^9.0.0" + }, "repository": { "type": "git", "url": "git+https://github.com/sveltejs/svelte.git" From 2bc9af4686c67a14d71cdecbe90b82fc5dd898d2 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 19 Oct 2024 12:17:16 +0100 Subject: [PATCH 3/5] fix: avoid chromium issue with dispatching blur on element removal --- packages/svelte/src/internal/client/reactivity/effects.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 9385555f3eb8..4ce62d812b90 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -380,7 +380,7 @@ export function destroy_effect(effect, remove_dom = true) { // Really we only need to do this in Chromium because of https://chromestatus.com/feature/5128696823545856, // as removal of the DOM can cause sync `blur` events to fire, which can cause logic to run inside // the current `active_reaction`, which isn't what we want at all - set_active_reaction(null) + set_active_reaction(null); while (node !== null) { /** @type {TemplateNode | null} */ var next = node === end ? null : /** @type {TemplateNode} */ (get_next_sibling(node)); @@ -388,7 +388,7 @@ export function destroy_effect(effect, remove_dom = true) { node.remove(); node = next; } - set_active_reaction(previous_reaction) + set_active_reaction(previous_reaction); removed = true; } From a9826e83a3ce7bc94366c2e94406f9404ba20eb4 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 19 Oct 2024 12:56:13 +0100 Subject: [PATCH 4/5] active effect too --- .../svelte/src/internal/client/reactivity/effects.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 4ce62d812b90..ee76b8681673 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -16,7 +16,8 @@ import { set_is_destroying_effect, set_is_flushing_effect, set_signal_status, - untrack + untrack, + set_active_effect } from '../runtime.js'; import { DIRTY, @@ -376,11 +377,14 @@ export function destroy_effect(effect, remove_dom = true) { var node = effect.nodes_start; var end = effect.nodes_end; var previous_reaction = active_reaction; + var previous_effect = active_effect; // Really we only need to do this in Chromium because of https://chromestatus.com/feature/5128696823545856, // as removal of the DOM can cause sync `blur` events to fire, which can cause logic to run inside - // the current `active_reaction`, which isn't what we want at all + // the current `active_reaction`, which isn't what we want at all. Additionally, the blur event handler + // might create a derived or effect and they will be incorrectly attached to the wrong thing set_active_reaction(null); + set_active_effect(null); while (node !== null) { /** @type {TemplateNode | null} */ var next = node === end ? null : /** @type {TemplateNode} */ (get_next_sibling(node)); @@ -389,6 +393,7 @@ export function destroy_effect(effect, remove_dom = true) { node = next; } set_active_reaction(previous_reaction); + set_active_effect(previous_effect); removed = true; } From 212a06800552ac54f5525d75a15f1a8bc1eaf77a Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sat, 19 Oct 2024 12:58:08 +0100 Subject: [PATCH 5/5] try/finally --- .../src/internal/client/reactivity/effects.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index ee76b8681673..d044b79efc3c 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -385,15 +385,18 @@ export function destroy_effect(effect, remove_dom = true) { // might create a derived or effect and they will be incorrectly attached to the wrong thing set_active_reaction(null); set_active_effect(null); - while (node !== null) { - /** @type {TemplateNode | null} */ - var next = node === end ? null : /** @type {TemplateNode} */ (get_next_sibling(node)); + try { + while (node !== null) { + /** @type {TemplateNode | null} */ + var next = node === end ? null : /** @type {TemplateNode} */ (get_next_sibling(node)); - node.remove(); - node = next; + node.remove(); + node = next; + } + } finally { + set_active_reaction(previous_reaction); + set_active_effect(previous_effect); } - set_active_reaction(previous_reaction); - set_active_effect(previous_effect); removed = true; }