msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

fix: remove overzealous `reactive_declaration_non_reactive_property` warning

Browse Source 
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. The ones I've seen essentially revolve around a signal being created while reading or writing (to) something in a reactive statement, but in a matter which is harmless.

For example writing to `$$props` or `$$restProps` (like `$: { if ($$restProps.foo) $$restProps.bar = 'x' }`) creates a new signal under the hood, as the props now temporarily can get out of sync, and we need a backing signal for that. For this we call the `prop` function, which in turn creates several deriveds and may also read from user-land signals that were not read previously - in other words, it's not possible to fix the problem by marking all internal signals (which would be a very very tedious undertaking).

The potential benefits of this warning are vastly outnumbered by the false positives it causes, so we should just remove it.
pull/14663/head
Simon Holthausen 2 years ago
parent a2539cfe1f
commit 196c68a16a
10 changed files with 9 additions and 170 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/purple-adults-arrive.md
Unescape View File

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: remove overzealous `reactive_declaration_non_reactive_property` warning

40
documentation/docs/98-reference/.generated/client-warnings.md
Unescape View File

@ -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
```

38
packages/svelte/messages/client-warnings/warnings.md
Unescape View File

@ -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

9
packages/svelte/src/compiler/phases/3-transform/client/visitors/LabeledStatement.js
Unescape View File

@ -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))
)
)
);

3
packages/svelte/src/constants.js
Unescape View File

@ -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'
]);
/**

22
packages/svelte/src/internal/client/reactivity/effects.js vendored
Unescape View File

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

12
packages/svelte/src/internal/client/warnings.js
Unescape View File

@ -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

26
packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/_config.js
Unescape View File

@ -1,26 +0,0 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
compileOptions: {
dev: true
},
html: '<p>42</p><p>42</p><button>update</button><button>reset</button>',
test({ assert, target }) {
const [update, reset] = target.querySelectorAll('button');
flushSync(() => update.click());
assert.htmlEqual(
target.innerHTML,
'<p>42</p><p>42</p><button>update</button><button>reset</button>'
);
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'
]
});

11
packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/data.svelte.js
Unescape View File

@ -1,11 +0,0 @@
export const obj = $state({
prop: 42
});
export function update() {
obj.prop += 1;
}
export function reset() {
obj.prop = 42;
}

13
packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/main.svelte
Unescape View File

@ -1,13 +0,0 @@
<script>
import { obj, update, reset } from './data.svelte.js';
$: a = obj.prop;
// svelte-ignore reactive_declaration_non_reactive_property
$: b = obj.prop;
</script>
<p>{a}</p>
<p>{b}</p>
<button on:click={update}>update</button>
<button on:click={reset}>reset</button>
Write Preview
Loading…
Cancel
Save