Skip to content

Commit

Permalink
fix: don't try to add owners to non-$state class fields (#14533)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
dummdidumm authored Dec 3, 2024
1 parent b558852 commit bbee1fc
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 20 deletions.
5 changes: 5 additions & 0 deletions .changeset/giant-windows-dance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: don't try to add owners to non-`$state` class fields
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 =
Expand Down

0 comments on commit bbee1fc

Please sign in to comment.