From bbee1fc7e05a24b3b31fbce50144aee5d898009a Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 3 Dec 2024 23:24:10 +0100 Subject: [PATCH] fix: don't try to add owners to non-`$state` class fields (#14533) * fix: don't try to add owners to non-`$state` class fields `$state.raw` and `$derived(.by)` will not have a state symbol on them, potentially causing a disastrous amount of traversal to potentially not find any state symbol. So it's better to not traverse them. Potentially someone could create a `$state` while creating `$state.raw` or inside a `$derived.by`, but that feels so much of an edge case that it doesn't warrant a perf hit for the common case. Fixes #14491 * for bind:, too --- .changeset/giant-windows-dance.md | 5 ++++ .../3-transform/client/visitors/ClassBody.js | 25 +++++++++------- .../client/visitors/shared/component.js | 30 ++++++++++++------- 3 files changed, 40 insertions(+), 20 deletions(-) create mode 100644 .changeset/giant-windows-dance.md diff --git a/.changeset/giant-windows-dance.md b/.changeset/giant-windows-dance.md new file mode 100644 index 000000000000..7ada8c483643 --- /dev/null +++ b/.changeset/giant-windows-dance.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: don't try to add owners to non-`$state` class fields diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 11a524d33c55..5e842a82febf 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -184,17 +184,22 @@ export function ClassBody(node, context) { 'method', b.id('$.ADD_OWNER'), [b.id('owner')], - Array.from(public_state.keys()).map((name) => - b.stmt( - b.call( - '$.add_owner', - b.call('$.get', b.member(b.this, b.private_id(name))), - b.id('owner'), - b.literal(false), - is_ignored(node, 'ownership_invalid_binding') && b.true + Array.from(public_state) + // Only run ownership addition on $state fields. + // Theoretically someone could create a `$state` while creating `$state.raw` or inside a `$derived.by`, + // but that feels so much of an edge case that it doesn't warrant a perf hit for the common case. + .filter(([_, { kind }]) => kind === 'state') + .map(([name]) => + b.stmt( + b.call( + '$.add_owner', + b.call('$.get', b.member(b.this, b.private_id(name))), + b.id('owner'), + b.literal(false), + is_ignored(node, 'ownership_invalid_binding') && b.true + ) ) - ) - ), + ), true ) ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index 8e1a53670708..5dde60b3b414 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -2,7 +2,7 @@ /** @import { AST, TemplateNode } from '#compiler' */ /** @import { ComponentContext } from '../../types.js' */ import { dev, is_ignored } from '../../../../../state.js'; -import { get_attribute_chunks } from '../../../../../utils/ast.js'; +import { get_attribute_chunks, object } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; import { create_derived } from '../../utils.js'; import { build_bind_this, validate_binding } from '../shared/utils.js'; @@ -176,16 +176,26 @@ export function build_component(node, component_name, context, anchor = context. bind_this = attribute.expression; } else { if (dev) { - binding_initializers.push( - b.stmt( - b.call( - b.id('$.add_owner_effect'), - b.thunk(expression), - b.id(component_name), - is_ignored(node, 'ownership_invalid_binding') && b.true + const left = object(attribute.expression); + let binding; + if (left?.type === 'Identifier') { + binding = context.state.scope.get(left.name); + } + // Only run ownership addition on $state fields. + // Theoretically someone could create a `$state` while creating `$state.raw` or inside a `$derived.by`, + // but that feels so much of an edge case that it doesn't warrant a perf hit for the common case. + if (binding?.kind !== 'derived' && binding?.kind !== 'raw_state') { + binding_initializers.push( + b.stmt( + b.call( + b.id('$.add_owner_effect'), + b.thunk(expression), + b.id(component_name), + is_ignored(node, 'ownership_invalid_binding') && b.true + ) ) - ) - ); + ); + } } const is_store_sub =