From 1942f87ed9e2d51f5a86a982d1492e55182ba10b Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 6 Aug 2024 16:49:01 +0100 Subject: [PATCH] fix: improve prop binding warning validation for stores (#12745) * fix: improve prop binding warning validation for stores * ts * address feedback * add comment * failing test * fix/simplify --------- Co-authored-by: Rich Harris --- .changeset/healthy-dancers-play.md | 5 +++ .../client/visitors/BindDirective.js | 12 +++--- .../client/visitors/shared/component.js | 4 +- .../client/visitors/shared/utils.js | 40 ++++++++++--------- .../binding-property-store/Child.svelte | 5 +++ .../samples/binding-property-store/_config.js | 11 +++++ .../binding-property-store/main.svelte | 12 ++++++ 7 files changed, 61 insertions(+), 28 deletions(-) create mode 100644 .changeset/healthy-dancers-play.md create mode 100644 packages/svelte/tests/runtime-runes/samples/binding-property-store/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/binding-property-store/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/binding-property-store/main.svelte diff --git a/.changeset/healthy-dancers-play.md b/.changeset/healthy-dancers-play.md new file mode 100644 index 0000000000..6fe572bb67 --- /dev/null +++ b/.changeset/healthy-dancers-play.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: improve prop binding warning validation for stores diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js index 611e5e570d..8b9064d59a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js @@ -7,7 +7,7 @@ import * as b from '../../../../utils/builders.js'; import { binding_properties } from '../../../bindings.js'; import { build_setter } from '../utils.js'; import { build_attribute_value } from './shared/element.js'; -import { build_bind_this, build_validate_binding } from './shared/utils.js'; +import { build_bind_this, validate_binding } from './shared/utils.js'; /** * @param {BindDirective} node @@ -30,12 +30,10 @@ export function BindDirective(node, context) { )) && !is_ignored(node, 'binding_property_non_reactive') ) { - context.state.init.push( - build_validate_binding( - context.state, - node, - /**@type {MemberExpression} */ (context.visit(expression)) - ) + validate_binding( + context.state, + node, + /**@type {MemberExpression} */ (context.visit(expression)) ); } 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 ab1abc82da..6416e26318 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 @@ -6,7 +6,7 @@ import { get_attribute_chunks } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; import { is_element_node } from '../../../../nodes.js'; import { create_derived, build_setter } from '../../utils.js'; -import { build_bind_this, build_validate_binding } from '../shared/utils.js'; +import { build_bind_this, validate_binding } from '../shared/utils.js'; import { build_attribute_value } from '../shared/element.js'; import { build_event_handler } from './events.js'; @@ -151,7 +151,7 @@ export function build_component(node, component_name, context, anchor = context. context.state.analysis.runes && !is_ignored(node, 'binding_property_non_reactive') ) { - context.state.init.push(build_validate_binding(context.state, attribute, expression)); + validate_binding(context.state, attribute, expression); } if (attribute.name === 'this') { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js index 04b7b36186..874ebe3131 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js @@ -204,28 +204,30 @@ export function build_bind_this(expression, value, { state, visit }) { * @param {BindDirective} binding * @param {MemberExpression} expression */ -export function build_validate_binding(state, binding, expression) { - const string = state.analysis.source.slice(binding.start, binding.end); - - const get_object = b.thunk(/** @type {Expression} */ (expression.object)); - const get_property = b.thunk( - /** @type {Expression} */ ( - expression.computed - ? expression.property - : b.literal(/** @type {Identifier} */ (expression.property).name) - ) - ); +export function validate_binding(state, binding, expression) { + // If we are referencing a $store.foo then we don't need to add validation + const left = object(binding.expression); + const left_binding = left && state.scope.get(left.name); + if (left_binding?.kind === 'store_sub') return; const loc = locator(binding.start); - return b.stmt( - b.call( - '$.validate_binding', - b.literal(string), - get_object, - get_property, - loc && b.literal(loc.line), - loc && b.literal(loc.column) + state.init.push( + b.stmt( + b.call( + '$.validate_binding', + b.literal(state.analysis.source.slice(binding.start, binding.end)), + b.thunk(/** @type {Expression} */ (expression.object)), + b.thunk( + /** @type {Expression} */ ( + expression.computed + ? expression.property + : b.literal(/** @type {Identifier} */ (expression.property).name) + ) + ), + loc && b.literal(loc.line), + loc && b.literal(loc.column) + ) ) ); } diff --git a/packages/svelte/tests/runtime-runes/samples/binding-property-store/Child.svelte b/packages/svelte/tests/runtime-runes/samples/binding-property-store/Child.svelte new file mode 100644 index 0000000000..c1bd5ff3ed --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/binding-property-store/Child.svelte @@ -0,0 +1,5 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/binding-property-store/_config.js b/packages/svelte/tests/runtime-runes/samples/binding-property-store/_config.js new file mode 100644 index 0000000000..e93067eb9d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/binding-property-store/_config.js @@ -0,0 +1,11 @@ +import { test } from '../../test'; + +export default test({ + mode: ['client'], + compileOptions: { + dev: true + }, + async test({ warnings, assert }) { + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/binding-property-store/main.svelte b/packages/svelte/tests/runtime-runes/samples/binding-property-store/main.svelte new file mode 100644 index 0000000000..206d0ceb44 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/binding-property-store/main.svelte @@ -0,0 +1,12 @@ + + + + +

{$a.value}

+

{$b.nested.value}