From fc896a779f031ef27c8f85734abf9f4d14394748 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Sun, 18 May 2025 15:36:43 +0200 Subject: [PATCH 1/3] fix: add warning when you read object property within attachments in legacy mode --- .changeset/breezy-pears-punch.md | 5 +++++ .../98-reference/.generated/compile-warnings.md | 6 ++++++ .../svelte/messages/compile-warnings/template.md | 4 ++++ .../phases/2-analyze/visitors/AttachTag.js | 13 +++++++++++++ packages/svelte/src/compiler/warnings.js | 9 +++++++++ .../attachment-legacy-member-access/input.svelte | 14 ++++++++++++++ .../attachment-legacy-member-access/warnings.json | 14 ++++++++++++++ 7 files changed, 65 insertions(+) create mode 100644 .changeset/breezy-pears-punch.md create mode 100644 packages/svelte/tests/validator/samples/attachment-legacy-member-access/input.svelte create mode 100644 packages/svelte/tests/validator/samples/attachment-legacy-member-access/warnings.json diff --git a/.changeset/breezy-pears-punch.md b/.changeset/breezy-pears-punch.md new file mode 100644 index 000000000000..e0ff7e746ce0 --- /dev/null +++ b/.changeset/breezy-pears-punch.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: add warning when you read object property within attachments in legacy mode diff --git a/documentation/docs/98-reference/.generated/compile-warnings.md b/documentation/docs/98-reference/.generated/compile-warnings.md index 7069f9020674..3f2aa1161895 100644 --- a/documentation/docs/98-reference/.generated/compile-warnings.md +++ b/documentation/docs/98-reference/.generated/compile-warnings.md @@ -556,6 +556,12 @@ Elements with ARIA roles must use a valid, non-abstract ARIA role. A reference t
``` +### attachment_legacy_member_access + +``` +Using `@attach` with a function from an object in legacy mode can cause unnecessary reruns of the attachment function if you mutate that object. +``` + ### attribute_avoid_is ``` diff --git a/packages/svelte/messages/compile-warnings/template.md b/packages/svelte/messages/compile-warnings/template.md index c1675e59952b..a62882a9cf2c 100644 --- a/packages/svelte/messages/compile-warnings/template.md +++ b/packages/svelte/messages/compile-warnings/template.md @@ -1,3 +1,7 @@ +## attachment_legacy_member_access + +> Using `@attach` with a function from an object in legacy mode can cause unnecessary reruns of the attachment function if you mutate that object. + ## attribute_avoid_is > The "is" attribute is not supported cross-browser and should be avoided diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/AttachTag.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/AttachTag.js index 1e318f228d8b..40333e9bd1c9 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/AttachTag.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/AttachTag.js @@ -1,7 +1,9 @@ /** @import { AST } from '#compiler' */ /** @import { Context } from '../types' */ +import { walk } from 'zimmerframe'; import { mark_subtree_dynamic } from './shared/fragment.js'; +import * as w from '../../../warnings.js'; /** * @param {AST.AttachTag} node @@ -9,5 +11,16 @@ import { mark_subtree_dynamic } from './shared/fragment.js'; */ export function AttachTag(node, context) { mark_subtree_dynamic(context.path); + if (!context.state.analysis.runes) { + walk( + node.expression, + {}, + { + MemberExpression(node) { + w.attachment_legacy_member_access(node); + } + } + ); + } context.next({ ...context.state, expression: node.metadata.expression }); } diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index c281433213e8..4740bdaeb459 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -106,6 +106,7 @@ export const codes = [ 'state_referenced_locally', 'store_rune_conflict', 'css_unused_selector', + 'attachment_legacy_member_access', 'attribute_avoid_is', 'attribute_global_event_reference', 'attribute_illegal_colon', @@ -677,6 +678,14 @@ export function css_unused_selector(node, name) { w(node, 'css_unused_selector', `Unused CSS selector "${name}"\nhttps://svelte.dev/e/css_unused_selector`); } +/** + * Using `@attach` with a function from an object in legacy mode can cause unnecessary reruns of the attachment function if you mutate that object. + * @param {null | NodeLike} node + */ +export function attachment_legacy_member_access(node) { + w(node, 'attachment_legacy_member_access', `Using \`@attach\` with a function from an object can cause unnecessary reruns of the attachment function if you mutate that object in legacy mode.\nhttps://svelte.dev/e/attachment_legacy_member_access`); +} + /** * The "is" attribute is not supported cross-browser and should be avoided * @param {null | NodeLike} node diff --git a/packages/svelte/tests/validator/samples/attachment-legacy-member-access/input.svelte b/packages/svelte/tests/validator/samples/attachment-legacy-member-access/input.svelte new file mode 100644 index 000000000000..cc56759f139f --- /dev/null +++ b/packages/svelte/tests/validator/samples/attachment-legacy-member-access/input.svelte @@ -0,0 +1,14 @@ + + + + +
\ No newline at end of file diff --git a/packages/svelte/tests/validator/samples/attachment-legacy-member-access/warnings.json b/packages/svelte/tests/validator/samples/attachment-legacy-member-access/warnings.json new file mode 100644 index 000000000000..ff981c4fff7a --- /dev/null +++ b/packages/svelte/tests/validator/samples/attachment-legacy-member-access/warnings.json @@ -0,0 +1,14 @@ +[ + { + "code": "attachment_legacy_member_access", + "message": "Using `@attach` with a function from an object in legacy mode can cause unnecessary reruns of the attachment function if you mutate that object.", + "start": { + "line": 14, + "column": 14 + }, + "end": { + "line": 14, + "column": 30 + } + } +] From e5b0136ad165a175d5717af04db354de00c1c71d Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Sun, 18 May 2025 15:45:26 +0200 Subject: [PATCH 2/3] chore: regenerate --- packages/svelte/src/compiler/warnings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 4740bdaeb459..d6e8b1db9609 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -683,7 +683,7 @@ export function css_unused_selector(node, name) { * @param {null | NodeLike} node */ export function attachment_legacy_member_access(node) { - w(node, 'attachment_legacy_member_access', `Using \`@attach\` with a function from an object can cause unnecessary reruns of the attachment function if you mutate that object in legacy mode.\nhttps://svelte.dev/e/attachment_legacy_member_access`); + w(node, 'attachment_legacy_member_access', `Using \`@attach\` with a function from an object in legacy mode can cause unnecessary reruns of the attachment function if you mutate that object.\nhttps://svelte.dev/e/attachment_legacy_member_access`); } /** From 7d1e9f168ec5748b975d35c5f92876ec54391327 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 May 2025 20:51:33 -0400 Subject: [PATCH 3/3] only warn if the object is mutated --- .../phases/2-analyze/visitors/AttachTag.js | 18 ++++++++---------- .../input.svelte | 16 ++++++++++++---- .../warnings.json | 4 ++-- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/AttachTag.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/AttachTag.js index 40333e9bd1c9..17fe546d6dc1 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/AttachTag.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/AttachTag.js @@ -1,7 +1,6 @@ /** @import { AST } from '#compiler' */ /** @import { Context } from '../types' */ -import { walk } from 'zimmerframe'; import { mark_subtree_dynamic } from './shared/fragment.js'; import * as w from '../../../warnings.js'; @@ -11,16 +10,15 @@ import * as w from '../../../warnings.js'; */ export function AttachTag(node, context) { mark_subtree_dynamic(context.path); + + context.next({ ...context.state, expression: node.metadata.expression }); + if (!context.state.analysis.runes) { - walk( - node.expression, - {}, - { - MemberExpression(node) { - w.attachment_legacy_member_access(node); - } + for (const dep of node.metadata.expression.dependencies) { + if (dep.mutated) { + w.attachment_legacy_member_access(node.expression); + break; } - ); + } } - context.next({ ...context.state, expression: node.metadata.expression }); } diff --git a/packages/svelte/tests/validator/samples/attachment-legacy-member-access/input.svelte b/packages/svelte/tests/validator/samples/attachment-legacy-member-access/input.svelte index cc56759f139f..2e0db9938aa8 100644 --- a/packages/svelte/tests/validator/samples/attachment-legacy-member-access/input.svelte +++ b/packages/svelte/tests/validator/samples/attachment-legacy-member-access/input.svelte @@ -1,9 +1,14 @@ @@ -11,4 +16,7 @@ state.count++; }}>{state.count} -
\ No newline at end of file +
+ +
+
diff --git a/packages/svelte/tests/validator/samples/attachment-legacy-member-access/warnings.json b/packages/svelte/tests/validator/samples/attachment-legacy-member-access/warnings.json index ff981c4fff7a..3b62b195879e 100644 --- a/packages/svelte/tests/validator/samples/attachment-legacy-member-access/warnings.json +++ b/packages/svelte/tests/validator/samples/attachment-legacy-member-access/warnings.json @@ -3,11 +3,11 @@ "code": "attachment_legacy_member_access", "message": "Using `@attach` with a function from an object in legacy mode can cause unnecessary reruns of the attachment function if you mutate that object.", "start": { - "line": 14, + "line": 19, "column": 14 }, "end": { - "line": 14, + "line": 19, "column": 30 } }