Skip to content

Commit

Permalink
fix: remove overzealous reactive_declaration_non_reactive_property
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
dummdidumm authored Dec 11, 2024
1 parent ea6fd95 commit 68d266e
Show file tree
Hide file tree
Showing 10 changed files with 9 additions and 170 deletions.
5 changes: 5 additions & 0 deletions .changeset/purple-adults-arrive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: remove overzealous `reactive_declaration_non_reactive_property` warning
40 changes: 0 additions & 40 deletions documentation/docs/98-reference/.generated/client-warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand Down
38 changes: 0 additions & 38 deletions packages/svelte/messages/client-warnings/warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,14 @@ 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,
b.stmt(
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))
)
)
);
Expand Down
3 changes: 1 addition & 2 deletions packages/svelte/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
]);

/**
Expand Down
22 changes: 2 additions & 20 deletions packages/svelte/src/internal/client/reactivity/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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);
});
}

Expand Down
12 changes: 0 additions & 12 deletions packages/svelte/src/internal/client/warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

0 comments on commit 68d266e

Please sign in to comment.