From 81dd84307b76072187c424ca64ce0324e5686fb2 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 6 Nov 2024 19:46:53 +0000 Subject: [PATCH 1/2] chore: ensure binding events are without context --- .../client/dom/elements/bindings/input.js | 34 ++++++++++--------- .../client/dom/elements/bindings/shared.js | 25 +++++++++++++- .../client/dom/elements/bindings/window.js | 17 +++++----- .../internal/client/dom/elements/events.js | 13 ++----- 4 files changed, 54 insertions(+), 35 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/input.js b/packages/svelte/src/internal/client/dom/elements/bindings/input.js index 7af20b932ad0..1bdd5e8eef54 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/input.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/input.js @@ -1,6 +1,6 @@ import { DEV } from 'esm-env'; import { render_effect, teardown } from '../../../reactivity/effects.js'; -import { listen_to_event_and_reset_event } from './shared.js'; +import { listen_to_event_and_reset_event, without_reactive_context } from './shared.js'; import * as e from '../../../errors.js'; import { is } from '../../../proxy.js'; import { queue_micro_task } from '../../task.js'; @@ -16,23 +16,25 @@ import { is_runes } from '../../../runtime.js'; export function bind_value(input, get, set = get) { var runes = is_runes(); - listen_to_event_and_reset_event(input, 'input', () => { - if (DEV && input.type === 'checkbox') { - // TODO should this happen in prod too? - e.bind_invalid_checkbox_value(); - } + listen_to_event_and_reset_event(input, 'input', () => + without_reactive_context(() => { + if (DEV && input.type === 'checkbox') { + // TODO should this happen in prod too? + e.bind_invalid_checkbox_value(); + } - /** @type {unknown} */ - var value = is_numberlike_input(input) ? to_number(input.value) : input.value; - set(value); + /** @type {unknown} */ + var value = is_numberlike_input(input) ? to_number(input.value) : input.value; + set(value); - // In runes mode, respect any validation in accessors (doesn't apply in legacy mode, - // because we use mutable state which ensures the render effect always runs) - if (runes && value !== (value = get())) { - // @ts-expect-error the value is coerced on assignment - input.value = value ?? ''; - } - }); + // In runes mode, respect any validation in accessors (doesn't apply in legacy mode, + // because we use mutable state which ensures the render effect always runs) + if (runes && value !== (value = get())) { + // @ts-expect-error the value is coerced on assignment + input.value = value ?? ''; + } + }) + ); render_effect(() => { if (DEV && input.type === 'checkbox') { diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/shared.js b/packages/svelte/src/internal/client/dom/elements/bindings/shared.js index 195eae2200ae..832b7f45e5d2 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/shared.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/shared.js @@ -1,4 +1,10 @@ import { teardown } from '../../../reactivity/effects.js'; +import { + active_effect, + active_reaction, + set_active_effect, + set_active_reaction +} from '../../../runtime.js'; import { add_form_reset_listener } from '../misc.js'; /** @@ -25,6 +31,23 @@ export function listen(target, events, handler, call_handler_immediately = true) }); } +/** + * @template T + * @param {() => T} fn + */ +export function without_reactive_context(fn) { + var previous_reaction = active_reaction; + var previous_effect = active_effect; + set_active_reaction(null); + set_active_effect(null); + try { + return fn(); + } finally { + set_active_reaction(previous_reaction); + set_active_effect(previous_effect); + } +} + /** * Listen to the given event, and then instantiate a global form reset listener if not already done, * to notify all bindings when the form is reset @@ -34,7 +57,7 @@ export function listen(target, events, handler, call_handler_immediately = true) * @param {() => void} [on_reset] */ export function listen_to_event_and_reset_event(element, event, handler, on_reset = handler) { - element.addEventListener(event, handler); + element.addEventListener(event, () => without_reactive_context(handler)); // @ts-expect-error const prev = element.__on_r; if (prev) { diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/window.js b/packages/svelte/src/internal/client/dom/elements/bindings/window.js index 61775b9d4bea..2f7e44c5d988 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/window.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/window.js @@ -1,5 +1,5 @@ import { effect, render_effect, teardown } from '../../../reactivity/effects.js'; -import { listen } from './shared.js'; +import { listen, without_reactive_context } from './shared.js'; /** * @param {'x' | 'y'} type @@ -10,13 +10,14 @@ import { listen } from './shared.js'; export function bind_window_scroll(type, get, set = get) { var is_scrolling_x = type === 'x'; - var target_handler = () => { - scrolling = true; - clearTimeout(timeout); - timeout = setTimeout(clear, 100); // TODO use scrollend event if supported (or when supported everywhere?) + var target_handler = () => + without_reactive_context(() => { + scrolling = true; + clearTimeout(timeout); + timeout = setTimeout(clear, 100); // TODO use scrollend event if supported (or when supported everywhere?) - set(window[is_scrolling_x ? 'scrollX' : 'scrollY']); - }; + set(window[is_scrolling_x ? 'scrollX' : 'scrollY']); + }); addEventListener('scroll', target_handler, { passive: true @@ -61,5 +62,5 @@ export function bind_window_scroll(type, get, set = get) { * @param {(size: number) => void} set */ export function bind_window_size(type, set) { - listen(window, ['resize'], () => set(window[type])); + listen(window, ['resize'], () => without_reactive_context(() => set(window[type]))); } diff --git a/packages/svelte/src/internal/client/dom/elements/events.js b/packages/svelte/src/internal/client/dom/elements/events.js index 12108b00f8c2..f2038f96ada3 100644 --- a/packages/svelte/src/internal/client/dom/elements/events.js +++ b/packages/svelte/src/internal/client/dom/elements/events.js @@ -11,6 +11,7 @@ import { set_active_effect, set_active_reaction } from '../../runtime.js'; +import { without_reactive_context } from './bindings/shared.js'; /** @type {Set} */ export const all_registered_events = new Set(); @@ -61,17 +62,9 @@ export function create_event(event_name, dom, handler, options) { handle_event_propagation.call(dom, event); } if (!event.cancelBubble) { - var previous_reaction = active_reaction; - var previous_effect = active_effect; - - set_active_reaction(null); - set_active_effect(null); - try { + return without_reactive_context(() => { return handler.call(this, event); - } finally { - set_active_reaction(previous_reaction); - set_active_effect(previous_effect); - } + }); } } From 2076608acf9b9389cc6044e51fe9742a8fae8bb6 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 6 Nov 2024 22:07:10 +0000 Subject: [PATCH 2/2] doh --- .../client/dom/elements/bindings/input.js | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/input.js b/packages/svelte/src/internal/client/dom/elements/bindings/input.js index 1bdd5e8eef54..e528d1699e48 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/input.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/input.js @@ -16,25 +16,23 @@ import { is_runes } from '../../../runtime.js'; export function bind_value(input, get, set = get) { var runes = is_runes(); - listen_to_event_and_reset_event(input, 'input', () => - without_reactive_context(() => { - if (DEV && input.type === 'checkbox') { - // TODO should this happen in prod too? - e.bind_invalid_checkbox_value(); - } + listen_to_event_and_reset_event(input, 'input', () => { + if (DEV && input.type === 'checkbox') { + // TODO should this happen in prod too? + e.bind_invalid_checkbox_value(); + } - /** @type {unknown} */ - var value = is_numberlike_input(input) ? to_number(input.value) : input.value; - set(value); + /** @type {unknown} */ + var value = is_numberlike_input(input) ? to_number(input.value) : input.value; + set(value); - // In runes mode, respect any validation in accessors (doesn't apply in legacy mode, - // because we use mutable state which ensures the render effect always runs) - if (runes && value !== (value = get())) { - // @ts-expect-error the value is coerced on assignment - input.value = value ?? ''; - } - }) - ); + // In runes mode, respect any validation in accessors (doesn't apply in legacy mode, + // because we use mutable state which ensures the render effect always runs) + if (runes && value !== (value = get())) { + // @ts-expect-error the value is coerced on assignment + input.value = value ?? ''; + } + }); render_effect(() => { if (DEV && input.type === 'checkbox') {