From 428c2125d4db78a9359f2d123c11eae2232b6f0a Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 12 Feb 2025 13:24:32 +0100 Subject: [PATCH 1/2] fix: take private and public into account for `constant_assignment` of derived state --- .changeset/five-apes-develop.md | 5 +++++ .../src/compiler/phases/2-analyze/types.d.ts | 2 +- .../phases/2-analyze/visitors/ClassBody.js | 7 +++++-- .../phases/2-analyze/visitors/shared/utils.js | 14 ++++++++++---- .../errors.json | 1 + .../input.svelte | 13 +++++++++++++ 6 files changed, 35 insertions(+), 7 deletions(-) create mode 100644 .changeset/five-apes-develop.md create mode 100644 packages/svelte/tests/validator/samples/reassign-derived-private-public-field/errors.json create mode 100644 packages/svelte/tests/validator/samples/reassign-derived-private-public-field/input.svelte diff --git a/.changeset/five-apes-develop.md b/.changeset/five-apes-develop.md new file mode 100644 index 000000000000..68e13c94a9e9 --- /dev/null +++ b/.changeset/five-apes-develop.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: take private and public into account for `constant_assignment` of derived state diff --git a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts index 14b14f9c84c1..70796a0d59b5 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/types.d.ts +++ b/packages/svelte/src/compiler/phases/2-analyze/types.d.ts @@ -19,7 +19,7 @@ export interface AnalysisState { component_slots: Set; /** Information about the current expression/directive/block value */ expression: ExpressionMetadata | null; - derived_state: string[]; + derived_state: { name: string; private: boolean }[]; function_depth: number; // legacy stuff diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js index ed397258f804..0463e4da8563 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ClassBody.js @@ -7,7 +7,7 @@ import { get_rune } from '../../scope.js'; * @param {Context} context */ export function ClassBody(node, context) { - /** @type {string[]} */ + /** @type {{name: string, private: boolean}[]} */ const derived_state = []; for (const definition of node.body) { @@ -18,7 +18,10 @@ export function ClassBody(node, context) { ) { const rune = get_rune(definition.value, context.state.scope); if (rune === '$derived' || rune === '$derived.by') { - derived_state.push(definition.key.name); + derived_state.push({ + name: definition.key.name, + private: definition.key.type === 'PrivateIdentifier' + }); } } } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js index 2d90c85364cf..603cce80c349 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js @@ -1,4 +1,4 @@ -/** @import { AssignmentExpression, Expression, Literal, Node, Pattern, PrivateIdentifier, Super, UpdateExpression, VariableDeclarator } from 'estree' */ +/** @import { AssignmentExpression, Expression, Identifier, Literal, Node, Pattern, PrivateIdentifier, Super, UpdateExpression, VariableDeclarator } from 'estree' */ /** @import { AST, Binding } from '#compiler' */ /** @import { AnalysisState, Context } from '../../types' */ /** @import { Scope } from '../../../scope' */ @@ -38,16 +38,22 @@ export function validate_assignment(node, argument, state) { e.snippet_parameter_assignment(node); } } - if ( argument.type === 'MemberExpression' && argument.object.type === 'ThisExpression' && (((argument.property.type === 'PrivateIdentifier' || argument.property.type === 'Identifier') && - state.derived_state.includes(argument.property.name)) || + state.derived_state.findIndex( + (derived) => + derived.name === /** @type {PrivateIdentifier | Identifier} */ (argument.property).name && + derived.private === (argument.property.type === 'PrivateIdentifier') + ) !== -1) || (argument.property.type === 'Literal' && argument.property.value && typeof argument.property.value === 'string' && - state.derived_state.includes(argument.property.value))) + state.derived_state.findIndex( + (derived) => + derived.name === /** @type {Literal} */ (argument.property).value && !derived.private + ) !== -1)) ) { e.constant_assignment(node, 'derived state'); } diff --git a/packages/svelte/tests/validator/samples/reassign-derived-private-public-field/errors.json b/packages/svelte/tests/validator/samples/reassign-derived-private-public-field/errors.json new file mode 100644 index 000000000000..fe51488c7066 --- /dev/null +++ b/packages/svelte/tests/validator/samples/reassign-derived-private-public-field/errors.json @@ -0,0 +1 @@ +[] diff --git a/packages/svelte/tests/validator/samples/reassign-derived-private-public-field/input.svelte b/packages/svelte/tests/validator/samples/reassign-derived-private-public-field/input.svelte new file mode 100644 index 000000000000..92f356492042 --- /dev/null +++ b/packages/svelte/tests/validator/samples/reassign-derived-private-public-field/input.svelte @@ -0,0 +1,13 @@ + \ No newline at end of file From 9f10d0122086a7f74df3eacb0a43351c39447c8d Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 12 Feb 2025 17:44:32 +0100 Subject: [PATCH 2/2] use .some(...) instead --- .../compiler/phases/2-analyze/visitors/shared/utils.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js index 603cce80c349..1507123e1342 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js @@ -42,18 +42,18 @@ export function validate_assignment(node, argument, state) { argument.type === 'MemberExpression' && argument.object.type === 'ThisExpression' && (((argument.property.type === 'PrivateIdentifier' || argument.property.type === 'Identifier') && - state.derived_state.findIndex( + state.derived_state.some( (derived) => derived.name === /** @type {PrivateIdentifier | Identifier} */ (argument.property).name && derived.private === (argument.property.type === 'PrivateIdentifier') - ) !== -1) || + )) || (argument.property.type === 'Literal' && argument.property.value && typeof argument.property.value === 'string' && - state.derived_state.findIndex( + state.derived_state.some( (derived) => derived.name === /** @type {Literal} */ (argument.property).value && !derived.private - ) !== -1)) + ))) ) { e.constant_assignment(node, 'derived state'); }