From 17e6c4f834043b52394b59a4bf0d5fdb0bdcb89a Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 13 Nov 2023 18:32:37 +0000 Subject: [PATCH] fix: address runtime effect issues (#9417) * Fix runtime effect issues * Prettier * Add changeset * Fix operations * Update .changeset/khaki-mails-draw.md * more tweaks * more tweaks --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- .changeset/khaki-mails-draw.md | 5 +++++ .../phases/3-transform/client/utils.js | 2 ++ .../svelte/src/internal/client/operations.js | 19 +++++++--------- .../svelte/src/internal/client/reconciler.js | 6 ++--- packages/svelte/src/internal/client/render.js | 8 +++---- .../svelte/src/internal/client/runtime.js | 15 +++++++++++-- .../_config.js | 13 +++++++++++ .../main.svelte | 14 ++++++++++++ .../_config.js | 13 +++++++++++ .../main.svelte | 22 +++++++++++++++++++ .../samples/effect-cleanup/_config.js | 19 ++++++++++++++++ .../samples/effect-cleanup/main.svelte | 17 ++++++++++++++ 12 files changed, 133 insertions(+), 20 deletions(-) create mode 100644 .changeset/khaki-mails-draw.md create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-cleanup/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-cleanup/main.svelte diff --git a/.changeset/khaki-mails-draw.md b/.changeset/khaki-mails-draw.md new file mode 100644 index 0000000000..cebc5770f4 --- /dev/null +++ b/.changeset/khaki-mails-draw.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: tighten up signals implementation diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index eed663846b..1194cffcde 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -251,6 +251,8 @@ export const function_visitor = (node, context) => { const in_constructor = parent.type === 'MethodDefinition' && parent.kind === 'constructor'; state = { ...context.state, in_constructor }; + } else { + state = { ...context.state, in_constructor: false }; } if (metadata?.hoistable === true) { diff --git a/packages/svelte/src/internal/client/operations.js b/packages/svelte/src/internal/client/operations.js index d3e515286d..d94e20dd3e 100644 --- a/packages/svelte/src/internal/client/operations.js +++ b/packages/svelte/src/internal/client/operations.js @@ -8,21 +8,19 @@ const has_browser_globals = typeof window !== 'undefined'; // than megamorphic. const node_prototype = /** @type {Node} */ (has_browser_globals ? Node.prototype : {}); const element_prototype = /** @type {Element} */ (has_browser_globals ? Element.prototype : {}); -const event_target_prototype = /** @type {EventTarget} */ ( - has_browser_globals ? EventTarget.prototype : {} -); +const text_prototype = /** @type {Text} */ (has_browser_globals ? Text.prototype : {}); const map_prototype = Map.prototype; const append_child_method = node_prototype.appendChild; const clone_node_method = node_prototype.cloneNode; const map_set_method = map_prototype.set; const map_get_method = map_prototype.get; const map_delete_method = map_prototype.delete; -// @ts-expect-error improve perf of expando on DOM nodes for events -event_target_prototype.__click = undefined; -// @ts-expect-error improve perf of expando on DOM textValue updates -event_target_prototype.__nodeValue = ' '; +// @ts-expect-error improve perf of expando on DOM events +element_prototype.__click = undefined; +// @ts-expect-error improve perf of expando on DOM text updates +text_prototype.__nodeValue = ' '; // @ts-expect-error improve perf of expando on DOM className updates -event_target_prototype.__className = ''; +element_prototype.__className = ''; const first_child_get = /** @type {(this: Node) => ChildNode | null} */ ( // @ts-ignore @@ -162,11 +160,10 @@ export function set_class_name(node, class_name) { /** * @template {Node} N * @param {N} node - * @param {string} text * @returns {void} */ -export function text_content(node, text) { - text_content_set.call(node, text); +export function clear_text_content(node) { + text_content_set.call(node, ''); } /** @param {string} name */ diff --git a/packages/svelte/src/internal/client/reconciler.js b/packages/svelte/src/internal/client/reconciler.js index ded5c7ec23..131c85cbd1 100644 --- a/packages/svelte/src/internal/client/reconciler.js +++ b/packages/svelte/src/internal/client/reconciler.js @@ -1,4 +1,4 @@ -import { append_child, map_get, map_set, text_content } from './operations.js'; +import { append_child, map_get, map_set, clear_text_content } from './operations.js'; import { current_hydration_fragment, get_hydration_fragment, @@ -198,7 +198,7 @@ export function reconcile_indexed_array( b_blocks = []; // Remove old blocks if (is_controlled && a !== 0) { - text_content(dom, ''); + clear_text_content(dom); } while (index < length) { block = a_blocks[index++]; @@ -295,7 +295,7 @@ export function reconcile_tracked_array( b_blocks = []; // Remove old blocks if (is_controlled && a !== 0) { - text_content(dom, ''); + clear_text_content(dom); } while (a > 0) { block = a_blocks[--a]; diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 135b99b0c6..e1395666c5 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -1270,15 +1270,15 @@ function handle_event_propagation(root_element, event) { } // composedPath contains list of nodes the event has propagated through. - // We check __handled_event_at to skip all nodes below it in case this is a - // parent of the __handled_event_at node, which indicates that there's nested + // We check __root to skip all nodes below it in case this is a + // parent of the __root node, which indicates that there's nested // mounted apps. In this case we don't want to trigger events multiple times. // We're deliberately not skipping if the index is the same or higher, because // someone could create an event programmatically and emit it multiple times, // in which case we want to handle the whole propagation chain properly each time. let path_idx = 0; // @ts-expect-error is added below - const handled_at = event.__handled_event_at; + const handled_at = event.__root; if (handled_at) { const at_idx = path.indexOf(handled_at); if (at_idx < path.indexOf(root_element)) { @@ -1317,7 +1317,7 @@ function handle_event_propagation(root_element, event) { } // @ts-expect-error is used above - event.__handled_event_at = root_element; + event.__root = root_element; } /** diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index b5fddc4489..1a217c5626 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -343,7 +343,13 @@ function destroy_references(signal) { if (references !== null) { let i; for (i = 0; i < references.length; i++) { - destroy_signal(references[i]); + const reference = references[i]; + if ((reference.flags & IS_EFFECT) !== 0) { + destroy_signal(reference); + } else { + remove_consumer(reference, 0, true); + reference.dependencies = null; + } } } } @@ -710,7 +716,7 @@ export function exposable(fn) { export function get(signal) { const flags = signal.flags; if ((flags & DESTROYED) !== 0) { - return /** @type {V} */ (UNINITIALIZED); + return signal.value; } if (is_signal_exposed && current_should_capture_signal) { @@ -1156,6 +1162,11 @@ export function managed_pre_effect(init, sync) { * @returns {import('./types.js').EffectSignal} */ export function pre_effect(init) { + if (current_effect === null) { + throw new Error( + 'The Svelte $effect.pre rune can only be used during component initialisation.' + ); + } const sync = current_effect !== null && (current_effect.flags & RENDER_EFFECT) !== 0; return internal_create_effect( PRE_EFFECT, diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private/_config.js b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private/_config.js new file mode 100644 index 0000000000..dd847ce2f2 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private/_config.js @@ -0,0 +1,13 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ``, + ssrHtml: ``, + + async test({ assert, target }) { + flushSync(); + + assert.htmlEqual(target.innerHTML, ``); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private/main.svelte new file mode 100644 index 0000000000..88b0398943 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure-private/main.svelte @@ -0,0 +1,14 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure/_config.js b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure/_config.js new file mode 100644 index 0000000000..dd847ce2f2 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure/_config.js @@ -0,0 +1,13 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ``, + ssrHtml: ``, + + async test({ assert, target }) { + flushSync(); + + assert.htmlEqual(target.innerHTML, ``); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure/main.svelte new file mode 100644 index 0000000000..fe7fcf1aa3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-state-constructor-closure/main.svelte @@ -0,0 +1,22 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/effect-cleanup/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-cleanup/_config.js new file mode 100644 index 0000000000..ee690418d6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-cleanup/_config.js @@ -0,0 +1,19 @@ +import { test } from '../../test'; +import { flushSync } from 'svelte'; + +export default test({ + get props() { + return { log: [] }; + }, + + async test({ assert, target, component }) { + const [b1] = target.querySelectorAll('button'); + flushSync(() => { + b1.click(); + }); + flushSync(() => { + b1.click(); + }); + assert.deepEqual(component.log, ['init 0', 'cleanup 2', 'init 2', 'cleanup 4', 'init 4']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-cleanup/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-cleanup/main.svelte new file mode 100644 index 0000000000..7c84aeddff --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-cleanup/main.svelte @@ -0,0 +1,17 @@ + + +