From 68d266e0f8e47e5f240e801b20fa4211424968b8 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 11 Dec 2024 22:34:42 +0100 Subject: [PATCH] fix: remove overzealous `reactive_declaration_non_reactive_property` warning (#14663) fixes #14532 This removes the `reactive_declaration_non_reactive_property` warning altogether. The first version caused many false positives at compile time. The refined runtime version (introduced in #14192) was hoped to fix this, but it turns out we now get loads of false positives at runtime. --- .changeset/purple-adults-arrive.md | 5 +++ .../.generated/client-warnings.md | 40 ------------------- .../messages/client-warnings/warnings.md | 38 ------------------ .../client/visitors/LabeledStatement.js | 9 +---- packages/svelte/src/constants.js | 3 +- .../src/internal/client/reactivity/effects.js | 22 +--------- .../svelte/src/internal/client/warnings.js | 12 ------ .../_config.js | 26 ------------ .../data.svelte.js | 11 ----- .../main.svelte | 13 ------ 10 files changed, 9 insertions(+), 170 deletions(-) create mode 100644 .changeset/purple-adults-arrive.md delete mode 100644 packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/_config.js delete mode 100644 packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/data.svelte.js delete mode 100644 packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/main.svelte diff --git a/.changeset/purple-adults-arrive.md b/.changeset/purple-adults-arrive.md new file mode 100644 index 000000000000..e3e3511de249 --- /dev/null +++ b/.changeset/purple-adults-arrive.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: remove overzealous `reactive_declaration_non_reactive_property` warning diff --git a/documentation/docs/98-reference/.generated/client-warnings.md b/documentation/docs/98-reference/.generated/client-warnings.md index b0490b84ffd9..06ca578b7658 100644 --- a/documentation/docs/98-reference/.generated/client-warnings.md +++ b/documentation/docs/98-reference/.generated/client-warnings.md @@ -204,46 +204,6 @@ Consider the following code: To fix it, either create callback props to communicate changes, or mark `person` as [`$bindable`]($bindable). -### reactive_declaration_non_reactive_property - -``` -A `$:` statement (%location%) read reactive state that was not visible to the compiler. Updates to this state will not cause the statement to re-run. The behaviour of this code will change if you migrate it to runes mode -``` - -In legacy mode, a `$:` [reactive statement](https://svelte.dev/docs/svelte/legacy-reactive-assignments) re-runs when the state it _references_ changes. This is determined at compile time, by analysing the code. - -In runes mode, effects and deriveds re-run when there are changes to the values that are read during the function's _execution_. - -Often, the result is the same — for example these can be considered equivalent: - -```js -let a = 1, b = 2, sum = 3; -// ---cut--- -$: sum = a + b; -``` - -```js -let a = 1, b = 2; -// ---cut--- -const sum = $derived(a + b); -``` - -In some cases — such as the one that triggered the above warning — they are _not_ the same: - -```js -let a = 1, b = 2, sum = 3; -// ---cut--- -const add = () => a + b; - -// the compiler can't 'see' that `sum` depends on `a` and `b`, but -// they _would_ be read while executing the `$derived` version -$: sum = add(); -``` - -Similarly, reactive properties of [deep state](https://svelte.dev/docs/svelte/$state#Deep-state) are not visible to the compiler. As such, changes to these properties will cause effects and deriveds to re-run but will _not_ cause `$:` statements to re-run. - -When you [migrate this component](https://svelte.dev/docs/svelte/v5-migration-guide) to runes mode, the behaviour will change accordingly. - ### state_proxy_equality_mismatch ``` diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index 122c1b92a64e..543be71080eb 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -170,44 +170,6 @@ Consider the following code: To fix it, either create callback props to communicate changes, or mark `person` as [`$bindable`]($bindable). -## reactive_declaration_non_reactive_property - -> A `$:` statement (%location%) read reactive state that was not visible to the compiler. Updates to this state will not cause the statement to re-run. The behaviour of this code will change if you migrate it to runes mode - -In legacy mode, a `$:` [reactive statement](https://svelte.dev/docs/svelte/legacy-reactive-assignments) re-runs when the state it _references_ changes. This is determined at compile time, by analysing the code. - -In runes mode, effects and deriveds re-run when there are changes to the values that are read during the function's _execution_. - -Often, the result is the same — for example these can be considered equivalent: - -```js -let a = 1, b = 2, sum = 3; -// ---cut--- -$: sum = a + b; -``` - -```js -let a = 1, b = 2; -// ---cut--- -const sum = $derived(a + b); -``` - -In some cases — such as the one that triggered the above warning — they are _not_ the same: - -```js -let a = 1, b = 2, sum = 3; -// ---cut--- -const add = () => a + b; - -// the compiler can't 'see' that `sum` depends on `a` and `b`, but -// they _would_ be read while executing the `$derived` version -$: sum = add(); -``` - -Similarly, reactive properties of [deep state](https://svelte.dev/docs/svelte/$state#Deep-state) are not visible to the compiler. As such, changes to these properties will cause effects and deriveds to re-run but will _not_ cause `$:` statements to re-run. - -When you [migrate this component](https://svelte.dev/docs/svelte/v5-migration-guide) to runes mode, the behaviour will change accordingly. - ## state_proxy_equality_mismatch > Reactive `$state(...)` proxies and the values they proxy have different identities. Because of this, comparisons with `%operator%` will produce unexpected results diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/LabeledStatement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/LabeledStatement.js index 8ca6534457ec..3c8f57f46b47 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/LabeledStatement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/LabeledStatement.js @@ -50,11 +50,6 @@ export function LabeledStatement(node, context) { sequence.push(serialized); } - const location = - dev && !is_ignored(node, 'reactive_declaration_non_reactive_property') - ? locator(/** @type {number} */ (node.start)) - : undefined; - // these statements will be topologically ordered later context.state.legacy_reactive_statements.set( node, @@ -62,9 +57,7 @@ export function LabeledStatement(node, context) { b.call( '$.legacy_pre_effect', sequence.length > 0 ? b.thunk(b.sequence(sequence)) : b.thunk(b.block([])), - b.thunk(b.block(body)), - location && b.literal(location.line), - location && b.literal(location.column) + b.thunk(b.block(body)) ) ) ); diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js index f8a7143b2047..03fddc5ebd28 100644 --- a/packages/svelte/src/constants.js +++ b/packages/svelte/src/constants.js @@ -44,8 +44,7 @@ export const IGNORABLE_RUNTIME_WARNINGS = /** @type {const} */ ([ 'hydration_attribute_changed', 'hydration_html_changed', 'ownership_invalid_binding', - 'ownership_invalid_mutation', - 'reactive_declaration_non_reactive_property' + 'ownership_invalid_mutation' ]); /** diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 912aab37b6b3..77502edb904a 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -265,21 +265,14 @@ export function effect(fn) { * Internal representation of `$: ..` * @param {() => any} deps * @param {() => void | (() => void)} fn - * @param {number} [line] - * @param {number} [column] */ -export function legacy_pre_effect(deps, fn, line, column) { +export function legacy_pre_effect(deps, fn) { var context = /** @type {ComponentContextLegacy} */ (component_context); /** @type {{ effect: null | Effect, ran: boolean }} */ var token = { effect: null, ran: false }; context.l.r1.push(token); - if (DEV && line !== undefined) { - var location = get_location(line, column); - var explicit_deps = capture_signals(deps); - } - token.effect = render_effect(() => { deps(); @@ -289,18 +282,7 @@ export function legacy_pre_effect(deps, fn, line, column) { token.ran = true; set(context.l.r2, true); - - if (DEV && location) { - var implicit_deps = capture_signals(() => untrack(fn)); - - for (var signal of implicit_deps) { - if (!explicit_deps.has(signal)) { - w.reactive_declaration_non_reactive_property(/** @type {string} */ (location)); - } - } - } else { - untrack(fn); - } + untrack(fn); }); } diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index 8daba601ef2a..24d86e2e0b35 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -155,18 +155,6 @@ export function ownership_invalid_mutation(component, owner) { } } -/** - * A `$:` statement (%location%) read reactive state that was not visible to the compiler. Updates to this state will not cause the statement to re-run. The behaviour of this code will change if you migrate it to runes mode - * @param {string} location - */ -export function reactive_declaration_non_reactive_property(location) { - if (DEV) { - console.warn(`%c[svelte] reactive_declaration_non_reactive_property\n%cA \`$:\` statement (${location}) read reactive state that was not visible to the compiler. Updates to this state will not cause the statement to re-run. The behaviour of this code will change if you migrate it to runes mode\nhttps://svelte.dev/e/reactive_declaration_non_reactive_property`, bold, normal); - } else { - console.warn(`https://svelte.dev/e/reactive_declaration_non_reactive_property`); - } -} - /** * Reactive `$state(...)` proxies and the values they proxy have different identities. Because of this, comparisons with `%operator%` will produce unexpected results * @param {string} operator diff --git a/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/_config.js b/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/_config.js deleted file mode 100644 index 7a77424bb271..000000000000 --- a/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/_config.js +++ /dev/null @@ -1,26 +0,0 @@ -import { flushSync } from 'svelte'; -import { test } from '../../test'; - -export default test({ - compileOptions: { - dev: true - }, - - html: '

42

42

', - - test({ assert, target }) { - const [update, reset] = target.querySelectorAll('button'); - flushSync(() => update.click()); - - assert.htmlEqual( - target.innerHTML, - '

42

42

' - ); - - flushSync(() => reset.click()); - }, - - warnings: [ - 'A `$:` statement (main.svelte:4:1) read reactive state that was not visible to the compiler. Updates to this state will not cause the statement to re-run. The behaviour of this code will change if you migrate it to runes mode' - ] -}); diff --git a/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/data.svelte.js b/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/data.svelte.js deleted file mode 100644 index 70fd0a6abcd8..000000000000 --- a/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/data.svelte.js +++ /dev/null @@ -1,11 +0,0 @@ -export const obj = $state({ - prop: 42 -}); - -export function update() { - obj.prop += 1; -} - -export function reset() { - obj.prop = 42; -} diff --git a/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/main.svelte b/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/main.svelte deleted file mode 100644 index 2a9b1e1f2311..000000000000 --- a/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/main.svelte +++ /dev/null @@ -1,13 +0,0 @@ - - -

{a}

-

{b}

- -