Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove overzealous reactive_declaration_non_reactive_property warning #14663

Merged
merged 1 commit into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.

Loading