From 94694a5e63baf5537128527d08c424de1639bc1f Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 3 Dec 2024 17:09:59 +0000 Subject: [PATCH 01/36] chore: add test for binding event reactive context (#14522) * chore: add test for binding event reactive context * revised --- .../client/dom/elements/bindings/input.js | 2 +- .../effect-tracking-binding-set/_config.js | 7 ++++++ .../effect-tracking-binding-set/main.svelte | 24 +++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-tracking-binding-set/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-tracking-binding-set/main.svelte diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/input.js b/packages/svelte/src/internal/client/dom/elements/bindings/input.js index aec6f815a012..1cbe2a731f01 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/input.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/input.js @@ -1,6 +1,6 @@ import { DEV } from 'esm-env'; import { render_effect, teardown } from '../../../reactivity/effects.js'; -import { listen_to_event_and_reset_event, without_reactive_context } from './shared.js'; +import { listen_to_event_and_reset_event } from './shared.js'; import * as e from '../../../errors.js'; import { is } from '../../../proxy.js'; import { queue_micro_task } from '../../task.js'; diff --git a/packages/svelte/tests/runtime-runes/samples/effect-tracking-binding-set/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-tracking-binding-set/_config.js new file mode 100644 index 000000000000..aebbfec832f6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-tracking-binding-set/_config.js @@ -0,0 +1,7 @@ +import { test } from '../../test'; + +export default test({ + test({ assert, logs }) { + assert.deepEqual(logs, [false]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-tracking-binding-set/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-tracking-binding-set/main.svelte new file mode 100644 index 000000000000..f21ecfdce6cb --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-tracking-binding-set/main.svelte @@ -0,0 +1,24 @@ + + + From aac929d50355ab7cfbade2d1fdb76674a233c460 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 3 Dec 2024 13:08:22 -0500 Subject: [PATCH 02/36] fix: leave update expressions untransformed unless a transformer is provided (#14507) * fix: leave update expressions untransformed unless a transformer is provided * fix more cases --- .changeset/neat-news-dance.md | 5 +++ .../phases/3-transform/client/types.d.ts | 2 +- .../3-transform/client/visitors/EachBlock.js | 5 ++- .../3-transform/client/visitors/Program.js | 16 ++++---- .../client/visitors/UpdateExpression.js | 38 +++++++------------ .../bigint-increment-mutation/_config.js | 15 ++++++++ .../bigint-increment-mutation/main.svelte | 16 ++++++++ .../samples/bigint-increment/_config.js | 7 ++++ .../samples/bigint-increment/main.svelte | 5 +++ 9 files changed, 75 insertions(+), 34 deletions(-) create mode 100644 .changeset/neat-news-dance.md create mode 100644 packages/svelte/tests/runtime-runes/samples/bigint-increment-mutation/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/bigint-increment-mutation/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/bigint-increment/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/bigint-increment/main.svelte diff --git a/.changeset/neat-news-dance.md b/.changeset/neat-news-dance.md new file mode 100644 index 000000000000..817c012330d6 --- /dev/null +++ b/.changeset/neat-news-dance.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: leave update expressions untransformed unless a transformer is provided diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index 80136159956a..47af9813e2a0 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -32,7 +32,7 @@ export interface ClientTransformState extends TransformState { /** turn `foo = bar` into e.g. `$.set(foo, bar)` */ assign?: (node: Identifier, value: Expression) => Expression; /** turn `foo.bar = baz` into e.g. `$.mutate(foo, $.get(foo).bar = baz);` */ - mutate?: (node: Identifier, mutation: AssignmentExpression) => Expression; + mutate?: (node: Identifier, mutation: AssignmentExpression | UpdateExpression) => Expression; /** turn `foo++` into e.g. `$.update(foo)` */ update?: (node: UpdateExpression) => Expression; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js index d34f39f4c7b0..9f70981205a1 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js @@ -214,7 +214,10 @@ export function EachBlock(node, context) { return b.sequence([b.assignment('=', left, value), ...sequence]); }, - mutate: (_, mutation) => b.sequence([mutation, ...sequence]) + mutate: (_, mutation) => { + uses_index = true; + return b.sequence([mutation, ...sequence]); + } }; delete key_state.transform[node.context.name]; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js index c34a9b05c69e..29403ca6edef 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js @@ -77,13 +77,15 @@ export function Program(_, context) { return b.call( '$.store_mutate', get_store(), - b.assignment( - mutation.operator, - /** @type {MemberExpression} */ ( - replace(/** @type {MemberExpression} */ (mutation.left)) - ), - mutation.right - ), + mutation.type === 'AssignmentExpression' + ? b.assignment( + mutation.operator, + /** @type {MemberExpression} */ ( + replace(/** @type {MemberExpression} */ (mutation.left)) + ), + mutation.right + ) + : b.update(mutation.operator, replace(mutation.argument), mutation.prefix), untracked ); }, diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js index 91383a56793c..13c1b4bc51e1 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js @@ -1,4 +1,4 @@ -/** @import { Expression, Node, Pattern, Statement, UpdateExpression } from 'estree' */ +/** @import { AssignmentExpression, Expression, UpdateExpression } from 'estree' */ /** @import { Context } from '../types' */ import { is_ignored } from '../../../../state.js'; import { object } from '../../../../utils/ast.js'; @@ -34,34 +34,22 @@ export function UpdateExpression(node, context) { } const left = object(argument); - if (left === null) return context.next(); + const transformers = left && context.state.transform[left.name]; - if (left === argument) { - const transform = context.state.transform; - const update = transform[left.name]?.update; - - if (update && Object.hasOwn(transform, left.name)) { - return update(node); - } + if (left === argument && transformers?.update) { + // we don't need to worry about ownership_invalid_mutation here, because + // we're not mutating but reassigning + return transformers.update(node); } - const assignment = /** @type {Expression} */ ( - context.visit( - b.assignment( - node.operator === '++' ? '+=' : '-=', - /** @type {Pattern} */ (argument), - b.literal(1) - ) - ) - ); + let update = /** @type {Expression} */ (context.next()); - const parent = /** @type {Node} */ (context.path.at(-1)); - const is_standalone = parent.type === 'ExpressionStatement'; // TODO and possibly others, but not e.g. the `test` of a WhileStatement - - const update = - node.prefix || is_standalone - ? assignment - : b.binary(node.operator === '++' ? '-' : '+', assignment, b.literal(1)); + if (left && transformers?.mutate) { + update = transformers.mutate( + left, + /** @type {AssignmentExpression | UpdateExpression} */ (update) + ); + } return is_ignored(node, 'ownership_invalid_mutation') ? b.call('$.skip_ownership_validation', b.thunk(update)) diff --git a/packages/svelte/tests/runtime-runes/samples/bigint-increment-mutation/_config.js b/packages/svelte/tests/runtime-runes/samples/bigint-increment-mutation/_config.js new file mode 100644 index 000000000000..fe1e962de23b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bigint-increment-mutation/_config.js @@ -0,0 +1,15 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: '

0

', + test({ assert, target }) { + const [btn1, btn2] = target.querySelectorAll('button'); + + flushSync(() => btn1.click()); + assert.htmlEqual(target.innerHTML, '

1

'); + + flushSync(() => btn2.click()); + assert.htmlEqual(target.innerHTML, '

0

'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/bigint-increment-mutation/main.svelte b/packages/svelte/tests/runtime-runes/samples/bigint-increment-mutation/main.svelte new file mode 100644 index 000000000000..148c7cef9fc7 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bigint-increment-mutation/main.svelte @@ -0,0 +1,16 @@ + + + + + +

{object.n}

diff --git a/packages/svelte/tests/runtime-runes/samples/bigint-increment/_config.js b/packages/svelte/tests/runtime-runes/samples/bigint-increment/_config.js new file mode 100644 index 000000000000..95b9addae6e3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bigint-increment/_config.js @@ -0,0 +1,7 @@ +import { test } from '../../test'; + +export default test({ + test({ assert, logs }) { + assert.deepEqual(logs, [0n, 1n, 2n, 3n, 4n, 5n]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/bigint-increment/main.svelte b/packages/svelte/tests/runtime-runes/samples/bigint-increment/main.svelte new file mode 100644 index 000000000000..a3370b5467ea --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bigint-increment/main.svelte @@ -0,0 +1,5 @@ + From 8ad9295535f5a5945f70b86e8d66b01d6af71c18 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 3 Dec 2024 13:16:52 -0500 Subject: [PATCH 03/36] docs: hints on preserving reactivity (#14514) --- documentation/docs/02-runes/02-$state.md | 55 +++++++++++++++++++++--- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/documentation/docs/02-runes/02-$state.md b/documentation/docs/02-runes/02-$state.md index e8213d3cf4a8..cc4ba0992c1b 100644 --- a/documentation/docs/02-runes/02-$state.md +++ b/documentation/docs/02-runes/02-$state.md @@ -36,12 +36,7 @@ let todos = $state([ ...modifying an individual todo's property will trigger updates to anything in your UI that depends on that specific property: ```js -// @filename: ambient.d.ts -declare global { - const todos: Array<{ done: boolean, text: string }> -} - -// @filename: index.js +let todos = [{ done: false, text: 'add more todos' }]; // ---cut--- todos[0].done = !todos[0].done; ``` @@ -64,6 +59,17 @@ todos.push({ > [!NOTE] When you update properties of proxies, the original object is _not_ mutated. +Note that if you destructure a reactive value, the references are not reactive — as in normal JavaScript, they are evaluated at the point of destructuring: + +```js +let todos = [{ done: false, text: 'add more todos' }]; +// ---cut--- +let { done, text } = todos[0]; + +// this will not affect the value of `done` +todos[0].done = !todos[0].done; +``` + ### Classes You can also use `$state` in class fields (whether public or private): @@ -85,7 +91,42 @@ class Todo { } ``` -> [!NOTE] The compiler transforms `done` and `text` into `get`/`set` methods on the class prototype referencing private fields. +> [!NOTE] The compiler transforms `done` and `text` into `get`/`set` methods on the class prototype referencing private fields. This means the properties are not enumerable. + +When calling methods in JavaScript, the value of [`this`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/this) matters. This won't work, because `this` inside the `reset` method will be the ` +``` + +You can either use an inline function... + +```svelte + +``` + +...or use an arrow function in the class definition: + +```js +// @errors: 7006 2554 +class Todo { + done = $state(false); + text = $state(); + + constructor(text) { + this.text = text; + } + + +++reset = () => {+++ + this.text = ''; + this.done = false; + } +} +``` ## `$state.raw` From 87863da6ff892a8678eca35876e8d863065cb1ef Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 3 Dec 2024 13:17:12 -0500 Subject: [PATCH 04/36] docs: how to javascript (#14528) --- documentation/docs/02-runes/02-$state.md | 87 ++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/documentation/docs/02-runes/02-$state.md b/documentation/docs/02-runes/02-$state.md index cc4ba0992c1b..80bf6bd03f25 100644 --- a/documentation/docs/02-runes/02-$state.md +++ b/documentation/docs/02-runes/02-$state.md @@ -168,3 +168,90 @@ To take a static snapshot of a deeply reactive `$state` proxy, use `$state.snaps ``` This is handy when you want to pass some state to an external library or API that doesn't expect a proxy, such as `structuredClone`. + +## Passing state into functions + +JavaScript is a _pass-by-value_ language — when you call a function, the arguments are the _values_ rather than the _variables_. In other words: + +```js +/// file: index.js +// @filename: index.js +// ---cut--- +/** + * @param {number} a + * @param {number} b + */ +function add(a, b) { + return a + b; +} + +let a = 1; +let b = 2; +let total = add(a, b); +console.log(total); // 3 + +a = 3; +b = 4; +console.log(total); // still 3! +``` + +If `add` wanted to have access to the _current_ values of `a` and `b`, and to return the current `total` value, you would need to use functions instead: + +```js +/// file: index.js +// @filename: index.js +// ---cut--- +/** + * @param {() => number} getA + * @param {() => number} getB + */ +function add(+++getA, getB+++) { + return +++() => getA() + getB()+++; +} + +let a = 1; +let b = 2; +let total = add+++(() => a, () => b)+++; +console.log(+++total()+++); // 3 + +a = 3; +a = 4; +console.log(+++total()+++); // 7 +``` + +State in Svelte is no different — when you reference something declared with the `$state` rune... + +```js +let a = +++$state(1)+++; +let b = +++$state(2)+++; +``` + +...you're accessing its _current value_. + +Note that 'functions' is broad — it encompasses properties of proxies and [`get`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get)/[`set`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/set) properties... + +```js +/// file: index.js +// @filename: index.js +// ---cut--- +/** + * @param {{ a: number, b: number }} input + */ +function add(input) { + return { + get value() { + return input.a + input.b; + } + }; +} + +let input = $state({ a: 1, b: 2 }); +let total = add(input); +console.log(total.value); // 3 + +input.a = 3; +input.b = 4; +console.log(total.value); // 7 +``` + +...though if you find yourself writing code like that, consider using [classes](#Classes) instead. From a5de086f9507a6a1852da4926d02ec08551ae3bd Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 3 Dec 2024 13:45:46 -0500 Subject: [PATCH 05/36] feat: turn `reactive_declaration_non_reactive_property` into a runtime warning (#14192) * turn `reactive_declaration_non_reactive_property` into a runtime warning * ignore warning * Update packages/svelte/src/internal/client/reactivity/effects.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> * Update packages/svelte/src/internal/client/runtime.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> * fix * test * changeset * Update .changeset/witty-turtles-bake.md Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> * add some details * check * regenerate --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- .changeset/witty-turtles-bake.md | 5 +++ .../.generated/client-warnings.md | 34 +++++++++++++++++++ .../.generated/compile-warnings.md | 6 ---- .../messages/client-warnings/warnings.md | 32 +++++++++++++++++ .../messages/compile-warnings/script.md | 4 --- .../2-analyze/visitors/MemberExpression.js | 25 -------------- .../client/visitors/LabeledStatement.js | 11 +++++- packages/svelte/src/compiler/warnings.js | 9 ----- packages/svelte/src/constants.js | 3 +- .../src/internal/client/dev/location.js | 25 ++++++++++++++ .../src/internal/client/dom/blocks/html.js | 5 ++- .../src/internal/client/reactivity/effects.js | 28 +++++++++++++-- .../svelte/src/internal/client/runtime.js | 24 ++++++++++--- .../svelte/src/internal/client/warnings.js | 13 +++++++ .../_config.js | 26 ++++++++++++++ .../data.svelte.js | 11 ++++++ .../main.svelte | 13 +++++++ .../_config.js | 3 -- .../input.svelte | 8 ----- .../warnings.json | 14 -------- 20 files changed, 217 insertions(+), 82 deletions(-) create mode 100644 .changeset/witty-turtles-bake.md create mode 100644 packages/svelte/src/internal/client/dev/location.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/data.svelte.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/main.svelte delete mode 100644 packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/_config.js delete mode 100644 packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/input.svelte delete mode 100644 packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/warnings.json diff --git a/.changeset/witty-turtles-bake.md b/.changeset/witty-turtles-bake.md new file mode 100644 index 000000000000..04637516f86c --- /dev/null +++ b/.changeset/witty-turtles-bake.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: turn reactive_declaration_non_reactive_property into a runtime warning diff --git a/documentation/docs/98-reference/.generated/client-warnings.md b/documentation/docs/98-reference/.generated/client-warnings.md index e9f1a88c3cc6..6e9721134119 100644 --- a/documentation/docs/98-reference/.generated/client-warnings.md +++ b/documentation/docs/98-reference/.generated/client-warnings.md @@ -86,6 +86,40 @@ Mutating a value outside the component that created it is strongly discouraged. %component% mutated a value owned by %owner%. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead ``` +### 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 +$: sum = a + b; +``` + +```js +const sum = $derived(a + b); +``` + +In some cases — such as the one that triggered the above warning — they are _not_ the same: + +```js +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/documentation/docs/98-reference/.generated/compile-warnings.md b/documentation/docs/98-reference/.generated/compile-warnings.md index 775e0681c94f..cc948e2547ee 100644 --- a/documentation/docs/98-reference/.generated/compile-warnings.md +++ b/documentation/docs/98-reference/.generated/compile-warnings.md @@ -726,12 +726,6 @@ Reactive declarations only exist at the top level of the instance script Reassignments of module-level declarations will not cause reactive statements to update ``` -### reactive_declaration_non_reactive_property - -``` -Properties of objects and arrays are not reactive unless in runes mode. Changes to this property will not cause the reactive statement to update -``` - ### script_context_deprecated ``` diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index e9014207fd2b..d82cdc47a1a5 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -54,6 +54,38 @@ The easiest way to log a value as it changes over time is to use the [`$inspect` > %component% mutated a value owned by %owner%. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead +## 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 +$: sum = a + b; +``` + +```js +const sum = $derived(a + b); +``` + +In some cases — such as the one that triggered the above warning — they are _not_ the same: + +```js +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/messages/compile-warnings/script.md b/packages/svelte/messages/compile-warnings/script.md index 293f065ba768..2c891b4fc791 100644 --- a/packages/svelte/messages/compile-warnings/script.md +++ b/packages/svelte/messages/compile-warnings/script.md @@ -26,10 +26,6 @@ > Reassignments of module-level declarations will not cause reactive statements to update -## reactive_declaration_non_reactive_property - -> Properties of objects and arrays are not reactive unless in runes mode. Changes to this property will not cause the reactive statement to update - ## state_referenced_locally > State referenced in its own scope will never update. Did you mean to reference it inside a closure? diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js index 1cc20c96dac8..6ea8f238e150 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/MemberExpression.js @@ -26,30 +26,5 @@ export function MemberExpression(node, context) { context.state.analysis.needs_context = true; } - if (context.state.reactive_statement) { - const left = object(node); - - if (left !== null) { - const binding = context.state.scope.get(left.name); - - if (binding && binding.kind === 'normal') { - const parent = /** @type {Node} */ (context.path.at(-1)); - - if ( - binding.scope === context.state.analysis.module.scope || - binding.declaration_kind === 'import' || - (binding.initial && - binding.initial.type !== 'ArrayExpression' && - binding.initial.type !== 'ObjectExpression' && - binding.scope.function_depth <= 1) - ) { - if (parent.type !== 'MemberExpression' && parent.type !== 'CallExpression') { - w.reactive_declaration_non_reactive_property(node); - } - } - } - } - } - context.next(); } 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 87f56262a8ba..8ca6534457ec 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 @@ -1,6 +1,8 @@ +/** @import { Location } from 'locate-character' */ /** @import { Expression, LabeledStatement, Statement } from 'estree' */ /** @import { ReactiveStatement } from '#compiler' */ /** @import { ComponentContext } from '../types' */ +import { dev, is_ignored, locator } from '../../../../state.js'; import * as b from '../../../../utils/builders.js'; import { build_getter } from '../utils.js'; @@ -48,6 +50,11 @@ 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, @@ -55,7 +62,9 @@ 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)) + b.thunk(b.block(body)), + location && b.literal(location.line), + location && b.literal(location.column) ) ) ); diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index bd2895623050..f6ed7de7d5d2 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -102,7 +102,6 @@ export const codes = [ "perf_avoid_nested_class", "reactive_declaration_invalid_placement", "reactive_declaration_module_script_dependency", - "reactive_declaration_non_reactive_property", "state_referenced_locally", "store_rune_conflict", "css_unused_selector", @@ -641,14 +640,6 @@ export function reactive_declaration_module_script_dependency(node) { w(node, "reactive_declaration_module_script_dependency", "Reassignments of module-level declarations will not cause reactive statements to update"); } -/** - * Properties of objects and arrays are not reactive unless in runes mode. Changes to this property will not cause the reactive statement to update - * @param {null | NodeLike} node - */ -export function reactive_declaration_non_reactive_property(node) { - w(node, "reactive_declaration_non_reactive_property", "Properties of objects and arrays are not reactive unless in runes mode. Changes to this property will not cause the reactive statement to update"); -} - /** * State referenced in its own scope will never update. Did you mean to reference it inside a closure? * @param {null | NodeLike} node diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js index 03fddc5ebd28..f8a7143b2047 100644 --- a/packages/svelte/src/constants.js +++ b/packages/svelte/src/constants.js @@ -44,7 +44,8 @@ export const IGNORABLE_RUNTIME_WARNINGS = /** @type {const} */ ([ 'hydration_attribute_changed', 'hydration_html_changed', 'ownership_invalid_binding', - 'ownership_invalid_mutation' + 'ownership_invalid_mutation', + 'reactive_declaration_non_reactive_property' ]); /** diff --git a/packages/svelte/src/internal/client/dev/location.js b/packages/svelte/src/internal/client/dev/location.js new file mode 100644 index 000000000000..b2e16c371e66 --- /dev/null +++ b/packages/svelte/src/internal/client/dev/location.js @@ -0,0 +1,25 @@ +import { DEV } from 'esm-env'; +import { FILENAME } from '../../../constants.js'; +import { dev_current_component_function } from '../runtime.js'; + +/** + * + * @param {number} [line] + * @param {number} [column] + */ +export function get_location(line, column) { + if (!DEV || line === undefined) return undefined; + + var filename = dev_current_component_function?.[FILENAME]; + var location = filename && `${filename}:${line}:${column}`; + + return sanitize_location(location); +} + +/** + * Prevent devtools trying to make `location` a clickable link by inserting a zero-width space + * @param {string | undefined} location + */ +export function sanitize_location(location) { + return location?.replace(/\//g, '/\u200b'); +} diff --git a/packages/svelte/src/internal/client/dom/blocks/html.js b/packages/svelte/src/internal/client/dom/blocks/html.js index aa13336b296b..2f815b454e05 100644 --- a/packages/svelte/src/internal/client/dom/blocks/html.js +++ b/packages/svelte/src/internal/client/dom/blocks/html.js @@ -9,6 +9,7 @@ import { hash } from '../../../../utils.js'; import { DEV } from 'esm-env'; import { dev_current_component_function } from '../../runtime.js'; import { get_first_child, get_next_sibling } from '../operations.js'; +import { sanitize_location } from '../../dev/location.js'; /** * @param {Element} element @@ -28,9 +29,7 @@ function check_hash(element, server_hash, value) { location = `in ${dev_current_component_function[FILENAME]}`; } - w.hydration_html_changed( - location?.replace(/\//g, '/\u200b') // prevent devtools trying to make it a clickable link by inserting a zero-width space - ); + w.hydration_html_changed(sanitize_location(location)); } /** diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 69b006fc0f9a..912aab37b6b3 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -16,7 +16,8 @@ import { set_is_flushing_effect, set_signal_status, untrack, - skip_reaction + skip_reaction, + capture_signals } from '../runtime.js'; import { DIRTY, @@ -39,10 +40,13 @@ import { } from '../constants.js'; import { set } from './sources.js'; import * as e from '../errors.js'; +import * as w from '../warnings.js'; import { DEV } from 'esm-env'; import { define_property } from '../../shared/utils.js'; import { get_next_sibling } from '../dom/operations.js'; import { destroy_derived } from './deriveds.js'; +import { FILENAME } from '../../../constants.js'; +import { get_location } from '../dev/location.js'; /** * @param {'$effect' | '$effect.pre' | '$inspect'} rune @@ -261,14 +265,21 @@ 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) { +export function legacy_pre_effect(deps, fn, line, column) { 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(); @@ -278,7 +289,18 @@ export function legacy_pre_effect(deps, fn) { token.ran = true; set(context.l.r2, true); - untrack(fn); + + 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); + } }); } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 2779898d6ffe..a3ec5bca34a5 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -894,15 +894,17 @@ export function safe_get(signal) { } /** - * Invokes a function and captures all signals that are read during the invocation, - * then invalidates them. - * @param {() => any} fn + * Capture an array of all the signals that are read when `fn` is called + * @template T + * @param {() => T} fn */ -export function invalidate_inner_signals(fn) { +export function capture_signals(fn) { var previous_captured_signals = captured_signals; captured_signals = new Set(); + var captured = captured_signals; var signal; + try { untrack(fn); if (previous_captured_signals !== null) { @@ -913,7 +915,19 @@ export function invalidate_inner_signals(fn) { } finally { captured_signals = previous_captured_signals; } - for (signal of captured) { + + return captured; +} + +/** + * Invokes a function and captures all signals that are read during the invocation, + * then invalidates them. + * @param {() => any} fn + */ +export function invalidate_inner_signals(fn) { + var captured = capture_signals(() => untrack(fn)); + + for (var signal of captured) { // Go one level up because derived signals created as part of props in legacy mode if ((signal.f & LEGACY_DERIVED_PROP) !== 0) { for (const dep of /** @type {Derived} */ (signal).deps || []) { diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index 047831371912..5e38db52f020 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -153,6 +153,19 @@ 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`, bold, normal); + } else { + // TODO print a link to the documentation + console.warn("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 new file mode 100644 index 000000000000..7a77424bb271 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/_config.js @@ -0,0 +1,26 @@ +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 new file mode 100644 index 000000000000..70fd0a6abcd8 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/data.svelte.js @@ -0,0 +1,11 @@ +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 new file mode 100644 index 000000000000..2a9b1e1f2311 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/reactive-statement-non-reactive-import-statement/main.svelte @@ -0,0 +1,13 @@ + + +

{a}

+

{b}

+ + diff --git a/packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/_config.js b/packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/_config.js deleted file mode 100644 index f47bee71df87..000000000000 --- a/packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/_config.js +++ /dev/null @@ -1,3 +0,0 @@ -import { test } from '../../test'; - -export default test({}); diff --git a/packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/input.svelte b/packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/input.svelte deleted file mode 100644 index 2582974b71b3..000000000000 --- a/packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/input.svelte +++ /dev/null @@ -1,8 +0,0 @@ - - -

{prop}

diff --git a/packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/warnings.json b/packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/warnings.json deleted file mode 100644 index 4184902753ab..000000000000 --- a/packages/svelte/tests/validator/samples/reactive-statement-non-reactive-import-statement/warnings.json +++ /dev/null @@ -1,14 +0,0 @@ -[ - { - "code": "reactive_declaration_non_reactive_property", - "message": "Properties of objects and arrays are not reactive unless in runes mode. Changes to this property will not cause the reactive statement to update", - "start": { - "line": 4, - "column": 11 - }, - "end": { - "line": 4, - "column": 19 - } - } -] From 39275684e5f19c86ea976af4b2235e6b6588c27d Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Tue, 3 Dec 2024 19:49:32 +0100 Subject: [PATCH 06/36] fix: don't throw with nullish actions (#13559) Co-authored-by: Oscar Dominguez --- .changeset/afraid-worms-clean.md | 5 +++++ .../phases/3-transform/client/visitors/UseDirective.js | 5 ++++- .../tests/runtime-runes/samples/nullish-actions/_config.js | 5 +++++ .../runtime-runes/samples/nullish-actions/main.svelte | 7 +++++++ 4 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 .changeset/afraid-worms-clean.md create mode 100644 packages/svelte/tests/runtime-runes/samples/nullish-actions/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/nullish-actions/main.svelte diff --git a/.changeset/afraid-worms-clean.md b/.changeset/afraid-worms-clean.md new file mode 100644 index 000000000000..800177b95b87 --- /dev/null +++ b/.changeset/afraid-worms-clean.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: don't throw with nullish actions diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UseDirective.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UseDirective.js index f6d918ce4552..be9eb2d51669 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UseDirective.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UseDirective.js @@ -20,7 +20,10 @@ export function UseDirective(node, context) { context.state.node, b.arrow( params, - b.call(/** @type {Expression} */ (context.visit(parse_directive_name(node.name))), ...params) + b.maybe_call( + /** @type {Expression} */ (context.visit(parse_directive_name(node.name))), + ...params + ) ) ]; diff --git a/packages/svelte/tests/runtime-runes/samples/nullish-actions/_config.js b/packages/svelte/tests/runtime-runes/samples/nullish-actions/_config.js new file mode 100644 index 000000000000..5d96dc96a481 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/nullish-actions/_config.js @@ -0,0 +1,5 @@ +import { test } from '../../test'; + +export default test({ + html: '
' +}); diff --git a/packages/svelte/tests/runtime-runes/samples/nullish-actions/main.svelte b/packages/svelte/tests/runtime-runes/samples/nullish-actions/main.svelte new file mode 100644 index 000000000000..2cbeb6722d1a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/nullish-actions/main.svelte @@ -0,0 +1,7 @@ + + +
+
\ No newline at end of file From 2f8eda94f34f756f5f5c6ea50d202f83e0d875a8 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 3 Dec 2024 13:49:57 -0500 Subject: [PATCH 07/36] fix: ensure SvelteDate cached methods have correct reactive context (#14525) * fix: ensure SvelteDate cached methods have no reactive context * fix: ensure SvelteDate cached methods have no reactive context * fix * lint * use active reaction at time of instance creation * tweak changeset * Update packages/svelte/src/internal/client/dom/elements/bindings/shared.js Co-authored-by: Paolo Ricciuti --------- Co-authored-by: Dominic Gannaway Co-authored-by: Paolo Ricciuti --- .changeset/wicked-zebras-exist.md | 5 ++++ packages/svelte/src/reactivity/date.js | 14 +++++++--- packages/svelte/src/reactivity/date.test.ts | 30 +++++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 .changeset/wicked-zebras-exist.md diff --git a/.changeset/wicked-zebras-exist.md b/.changeset/wicked-zebras-exist.md new file mode 100644 index 000000000000..f76d77bb9078 --- /dev/null +++ b/.changeset/wicked-zebras-exist.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: use correct reaction when lazily creating deriveds inside `SvelteDate` diff --git a/packages/svelte/src/reactivity/date.js b/packages/svelte/src/reactivity/date.js index 2d8de624bcc7..33da2e176159 100644 --- a/packages/svelte/src/reactivity/date.js +++ b/packages/svelte/src/reactivity/date.js @@ -1,8 +1,7 @@ /** @import { Source } from '#client' */ -import { DESTROYED } from '../internal/client/constants.js'; import { derived } from '../internal/client/index.js'; import { source, set } from '../internal/client/reactivity/sources.js'; -import { get } from '../internal/client/runtime.js'; +import { active_reaction, get, set_active_reaction } from '../internal/client/runtime.js'; var inited = false; @@ -12,6 +11,8 @@ export class SvelteDate extends Date { /** @type {Map>} */ #deriveds = new Map(); + #reaction = active_reaction; + /** @param {any[]} params */ constructor(...params) { // @ts-ignore @@ -43,7 +44,12 @@ export class SvelteDate extends Date { var d = this.#deriveds.get(method); - if (d === undefined || (d.f & DESTROYED) !== 0) { + if (d === undefined) { + // lazily create the derived, but as though it were being + // created at the same time as the class instance + const reaction = active_reaction; + set_active_reaction(this.#reaction); + d = derived(() => { get(this.#time); // @ts-ignore @@ -51,6 +57,8 @@ export class SvelteDate extends Date { }); this.#deriveds.set(method, d); + + set_active_reaction(reaction); } return get(d); diff --git a/packages/svelte/src/reactivity/date.test.ts b/packages/svelte/src/reactivity/date.test.ts index 87bfde41c89c..f90c5a102c52 100644 --- a/packages/svelte/src/reactivity/date.test.ts +++ b/packages/svelte/src/reactivity/date.test.ts @@ -642,3 +642,33 @@ test('Date methods invoked for the first time in a derived', () => { cleanup(); }); + +test('Date methods shared between deriveds', () => { + const date = new SvelteDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + const year = derived(() => { + return date.getFullYear(); + }); + const year2 = derived(() => { + return date.getTime(), date.getFullYear(); + }); + + render_effect(() => { + log.push(get(year) + '/' + get(year2).toString()); + }); + + flushSync(() => { + date.setFullYear(date.getFullYear() + 1); + }); + + flushSync(() => { + date.setFullYear(date.getFullYear() + 1); + }); + }); + + assert.deepEqual(log, ['2023/2023', '2024/2024', '2025/2025']); + + cleanup(); +}); From d595cc16fbc5a88e3ce0dff8c3e36c10a9e78ac9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 3 Dec 2024 13:50:54 -0500 Subject: [PATCH 08/36] Version Packages (#14529) Co-authored-by: github-actions[bot] --- .changeset/afraid-worms-clean.md | 5 ----- .changeset/neat-news-dance.md | 5 ----- .changeset/witty-turtles-bake.md | 5 ----- packages/svelte/CHANGELOG.md | 10 ++++++++++ packages/svelte/package.json | 2 +- packages/svelte/src/version.js | 2 +- 6 files changed, 12 insertions(+), 17 deletions(-) delete mode 100644 .changeset/afraid-worms-clean.md delete mode 100644 .changeset/neat-news-dance.md delete mode 100644 .changeset/witty-turtles-bake.md diff --git a/.changeset/afraid-worms-clean.md b/.changeset/afraid-worms-clean.md deleted file mode 100644 index 800177b95b87..000000000000 --- a/.changeset/afraid-worms-clean.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -fix: don't throw with nullish actions diff --git a/.changeset/neat-news-dance.md b/.changeset/neat-news-dance.md deleted file mode 100644 index 817c012330d6..000000000000 --- a/.changeset/neat-news-dance.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -fix: leave update expressions untransformed unless a transformer is provided diff --git a/.changeset/witty-turtles-bake.md b/.changeset/witty-turtles-bake.md deleted file mode 100644 index 04637516f86c..000000000000 --- a/.changeset/witty-turtles-bake.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -chore: turn reactive_declaration_non_reactive_property into a runtime warning diff --git a/packages/svelte/CHANGELOG.md b/packages/svelte/CHANGELOG.md index a53738689fdf..c1a102378a50 100644 --- a/packages/svelte/CHANGELOG.md +++ b/packages/svelte/CHANGELOG.md @@ -1,5 +1,15 @@ # svelte +## 5.5.1 + +### Patch Changes + +- fix: don't throw with nullish actions ([#13559](https://github.com/sveltejs/svelte/pull/13559)) + +- fix: leave update expressions untransformed unless a transformer is provided ([#14507](https://github.com/sveltejs/svelte/pull/14507)) + +- chore: turn reactive_declaration_non_reactive_property into a runtime warning ([#14192](https://github.com/sveltejs/svelte/pull/14192)) + ## 5.5.0 ### Minor Changes diff --git a/packages/svelte/package.json b/packages/svelte/package.json index fe25b0bb3472..6d1229415b61 100644 --- a/packages/svelte/package.json +++ b/packages/svelte/package.json @@ -2,7 +2,7 @@ "name": "svelte", "description": "Cybernetically enhanced web apps", "license": "MIT", - "version": "5.5.0", + "version": "5.5.1", "type": "module", "types": "./types/index.d.ts", "engines": { diff --git a/packages/svelte/src/version.js b/packages/svelte/src/version.js index 1271d6cb90b7..1fbb82c6fb0a 100644 --- a/packages/svelte/src/version.js +++ b/packages/svelte/src/version.js @@ -6,5 +6,5 @@ * https://svelte.dev/docs/svelte-compiler#svelte-version * @type {string} */ -export const VERSION = '5.5.0'; +export const VERSION = '5.5.1'; export const PUBLIC_VERSION = '5'; From a65e68ca37d171663feec99382efd46e7cea364f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 3 Dec 2024 13:59:16 -0500 Subject: [PATCH 09/36] Version Packages (#14530) Co-authored-by: github-actions[bot] --- .changeset/wicked-zebras-exist.md | 5 ----- packages/svelte/CHANGELOG.md | 6 ++++++ packages/svelte/package.json | 2 +- packages/svelte/src/version.js | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) delete mode 100644 .changeset/wicked-zebras-exist.md diff --git a/.changeset/wicked-zebras-exist.md b/.changeset/wicked-zebras-exist.md deleted file mode 100644 index f76d77bb9078..000000000000 --- a/.changeset/wicked-zebras-exist.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -fix: use correct reaction when lazily creating deriveds inside `SvelteDate` diff --git a/packages/svelte/CHANGELOG.md b/packages/svelte/CHANGELOG.md index c1a102378a50..8bf0885277b8 100644 --- a/packages/svelte/CHANGELOG.md +++ b/packages/svelte/CHANGELOG.md @@ -1,5 +1,11 @@ # svelte +## 5.5.2 + +### Patch Changes + +- fix: use correct reaction when lazily creating deriveds inside `SvelteDate` ([#14525](https://github.com/sveltejs/svelte/pull/14525)) + ## 5.5.1 ### Patch Changes diff --git a/packages/svelte/package.json b/packages/svelte/package.json index 6d1229415b61..0728adf5fc27 100644 --- a/packages/svelte/package.json +++ b/packages/svelte/package.json @@ -2,7 +2,7 @@ "name": "svelte", "description": "Cybernetically enhanced web apps", "license": "MIT", - "version": "5.5.1", + "version": "5.5.2", "type": "module", "types": "./types/index.d.ts", "engines": { diff --git a/packages/svelte/src/version.js b/packages/svelte/src/version.js index 1fbb82c6fb0a..0394adda8224 100644 --- a/packages/svelte/src/version.js +++ b/packages/svelte/src/version.js @@ -6,5 +6,5 @@ * https://svelte.dev/docs/svelte-compiler#svelte-version * @type {string} */ -export const VERSION = '5.5.1'; +export const VERSION = '5.5.2'; export const PUBLIC_VERSION = '5'; From d5a28a01a16b1151c62dde878f69c4cb6e80d108 Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Tue, 3 Dec 2024 22:31:28 +0100 Subject: [PATCH 10/36] docs: fix typo in `$state` (#14535) --- documentation/docs/02-runes/02-$state.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/docs/02-runes/02-$state.md b/documentation/docs/02-runes/02-$state.md index 80bf6bd03f25..77140dc6903d 100644 --- a/documentation/docs/02-runes/02-$state.md +++ b/documentation/docs/02-runes/02-$state.md @@ -215,7 +215,7 @@ let total = add+++(() => a, () => b)+++; console.log(+++total()+++); // 3 a = 3; -a = 4; +b = 4; console.log(+++total()+++); // 7 ``` From b5588523fc03cddec877161c17dd93d524a15129 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 3 Dec 2024 22:22:23 +0000 Subject: [PATCH 11/36] fix: capture infinite_loop_guard in error boundary (#14534) * fix: capture infinite_loop_guard in error boundary * fix --- .changeset/hip-forks-exercise.md | 5 ++ .../svelte/src/internal/client/runtime.js | 52 ++++++++++++++----- .../samples/error-boundary-20/Child.svelte | 17 ++++++ .../samples/error-boundary-20/_config.js | 13 +++++ .../samples/error-boundary-20/main.svelte | 10 ++++ 5 files changed, 85 insertions(+), 12 deletions(-) create mode 100644 .changeset/hip-forks-exercise.md create mode 100644 packages/svelte/tests/runtime-runes/samples/error-boundary-20/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/error-boundary-20/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/error-boundary-20/main.svelte diff --git a/.changeset/hip-forks-exercise.md b/.changeset/hip-forks-exercise.md new file mode 100644 index 000000000000..41df40bd6a65 --- /dev/null +++ b/.changeset/hip-forks-exercise.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: capture infinite_loop_guard in error boundary diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index a3ec5bca34a5..4928419d16af 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -48,6 +48,9 @@ let scheduler_mode = FLUSH_MICROTASK; // Used for handling scheduling let is_micro_task_queued = false; +/** @type {Effect | null} */ +let last_scheduled_effect = null; + export let is_flushing_effect = false; export let is_destroying_effect = false; @@ -532,27 +535,47 @@ export function update_effect(effect) { } } +function log_effect_stack() { + // eslint-disable-next-line no-console + console.error( + 'Last ten effects were: ', + dev_effect_stack.slice(-10).map((d) => d.fn) + ); + dev_effect_stack = []; +} + function infinite_loop_guard() { if (flush_count > 1000) { flush_count = 0; - if (DEV) { - try { - e.effect_update_depth_exceeded(); - } catch (error) { + try { + e.effect_update_depth_exceeded(); + } catch (error) { + if (DEV) { // stack is garbage, ignore. Instead add a console.error message. define_property(error, 'stack', { value: '' }); - // eslint-disable-next-line no-console - console.error( - 'Last ten effects were: ', - dev_effect_stack.slice(-10).map((d) => d.fn) - ); - dev_effect_stack = []; + } + // Try and handle the error so it can be caught at a boundary, that's + // if there's an effect available from when it was last scheduled + if (last_scheduled_effect !== null) { + if (DEV) { + try { + handle_error(error, last_scheduled_effect, null, null); + } catch (e) { + // Only log the effect stack if the error is re-thrown + log_effect_stack(); + throw e; + } + } else { + handle_error(error, last_scheduled_effect, null, null); + } + } else { + if (DEV) { + log_effect_stack(); + } throw error; } - } else { - e.effect_update_depth_exceeded(); } } flush_count++; @@ -637,8 +660,10 @@ function process_deferred() { const previous_queued_root_effects = queued_root_effects; queued_root_effects = []; flush_queued_root_effects(previous_queued_root_effects); + if (!is_micro_task_queued) { flush_count = 0; + last_scheduled_effect = null; if (DEV) { dev_effect_stack = []; } @@ -657,6 +682,8 @@ export function schedule_effect(signal) { } } + last_scheduled_effect = signal; + var effect = signal; while (effect.parent !== null) { @@ -776,6 +803,7 @@ export function flush_sync(fn) { } flush_count = 0; + last_scheduled_effect = null; if (DEV) { dev_effect_stack = []; } diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-20/Child.svelte b/packages/svelte/tests/runtime-runes/samples/error-boundary-20/Child.svelte new file mode 100644 index 000000000000..ff97dd96f0b6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-20/Child.svelte @@ -0,0 +1,17 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-20/_config.js b/packages/svelte/tests/runtime-runes/samples/error-boundary-20/_config.js new file mode 100644 index 000000000000..ccff614ade0d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-20/_config.js @@ -0,0 +1,13 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + test({ assert, target }) { + let btn = target.querySelector('button'); + + btn?.click(); + flushSync(); + + assert.htmlEqual(target.innerHTML, `
An error occurred!
`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/error-boundary-20/main.svelte b/packages/svelte/tests/runtime-runes/samples/error-boundary-20/main.svelte new file mode 100644 index 000000000000..18f216147fc4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/error-boundary-20/main.svelte @@ -0,0 +1,10 @@ + + + + + {#snippet failed()} +
An error occurred!
+ {/snippet} + From bbee1fc7e05a24b3b31fbce50144aee5d898009a Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 3 Dec 2024 23:24:10 +0100 Subject: [PATCH 12/36] fix: don't try to add owners to non-`$state` class fields (#14533) * fix: don't try to add owners to non-`$state` class fields `$state.raw` and `$derived(.by)` will not have a state symbol on them, potentially causing a disastrous amount of traversal to potentially not find any state symbol. So it's better to not traverse them. Potentially someone could create a `$state` while creating `$state.raw` or inside a `$derived.by`, but that feels so much of an edge case that it doesn't warrant a perf hit for the common case. Fixes #14491 * for bind:, too --- .changeset/giant-windows-dance.md | 5 ++++ .../3-transform/client/visitors/ClassBody.js | 25 +++++++++------- .../client/visitors/shared/component.js | 30 ++++++++++++------- 3 files changed, 40 insertions(+), 20 deletions(-) create mode 100644 .changeset/giant-windows-dance.md diff --git a/.changeset/giant-windows-dance.md b/.changeset/giant-windows-dance.md new file mode 100644 index 000000000000..7ada8c483643 --- /dev/null +++ b/.changeset/giant-windows-dance.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: don't try to add owners to non-`$state` class fields diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js index 11a524d33c55..5e842a82febf 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js @@ -184,17 +184,22 @@ export function ClassBody(node, context) { 'method', b.id('$.ADD_OWNER'), [b.id('owner')], - Array.from(public_state.keys()).map((name) => - b.stmt( - b.call( - '$.add_owner', - b.call('$.get', b.member(b.this, b.private_id(name))), - b.id('owner'), - b.literal(false), - is_ignored(node, 'ownership_invalid_binding') && b.true + Array.from(public_state) + // Only run ownership addition on $state fields. + // Theoretically someone could create a `$state` while creating `$state.raw` or inside a `$derived.by`, + // but that feels so much of an edge case that it doesn't warrant a perf hit for the common case. + .filter(([_, { kind }]) => kind === 'state') + .map(([name]) => + b.stmt( + b.call( + '$.add_owner', + b.call('$.get', b.member(b.this, b.private_id(name))), + b.id('owner'), + b.literal(false), + is_ignored(node, 'ownership_invalid_binding') && b.true + ) ) - ) - ), + ), true ) ); 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 8e1a53670708..5dde60b3b414 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 @@ -2,7 +2,7 @@ /** @import { AST, TemplateNode } from '#compiler' */ /** @import { ComponentContext } from '../../types.js' */ import { dev, is_ignored } from '../../../../../state.js'; -import { get_attribute_chunks } from '../../../../../utils/ast.js'; +import { get_attribute_chunks, object } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; import { create_derived } from '../../utils.js'; import { build_bind_this, validate_binding } from '../shared/utils.js'; @@ -176,16 +176,26 @@ export function build_component(node, component_name, context, anchor = context. bind_this = attribute.expression; } else { if (dev) { - binding_initializers.push( - b.stmt( - b.call( - b.id('$.add_owner_effect'), - b.thunk(expression), - b.id(component_name), - is_ignored(node, 'ownership_invalid_binding') && b.true + const left = object(attribute.expression); + let binding; + if (left?.type === 'Identifier') { + binding = context.state.scope.get(left.name); + } + // Only run ownership addition on $state fields. + // Theoretically someone could create a `$state` while creating `$state.raw` or inside a `$derived.by`, + // but that feels so much of an edge case that it doesn't warrant a perf hit for the common case. + if (binding?.kind !== 'derived' && binding?.kind !== 'raw_state') { + binding_initializers.push( + b.stmt( + b.call( + b.id('$.add_owner_effect'), + b.thunk(expression), + b.id(component_name), + is_ignored(node, 'ownership_invalid_binding') && b.true + ) ) - ) - ); + ); + } } const is_store_sub = From fe0bd299a5101c7dc24546cd92dac04857f3b53d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 3 Dec 2024 17:24:47 -0500 Subject: [PATCH 13/36] fix: proxify values when assigning using `||=`, `&&=` and `??=` operators (#14273) * add failing test for #14268 * simplify * proxify values when using ||=, &&= and ??= assignment operators * proxify values assigned to private state fields * changeset * fix * fix * add warning * update test --- .changeset/shaggy-spies-happen.md | 5 + .../.generated/client-warnings.md | 32 ++++++ .../messages/client-warnings/warnings.md | 30 +++++ .../client/visitors/AssignmentExpression.js | 104 ++++++++++++++++-- .../svelte/src/internal/client/dev/assign.js | 57 ++++++++++ packages/svelte/src/internal/client/index.js | 1 + .../svelte/src/internal/client/warnings.js | 14 +++ .../_config.js | 24 ++++ .../main.svelte | 7 ++ .../_config.js | 16 +++ .../main.svelte | 7 ++ 11 files changed, 287 insertions(+), 10 deletions(-) create mode 100644 .changeset/shaggy-spies-happen.md create mode 100644 packages/svelte/src/internal/client/dev/assign.js create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment-warning/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment-warning/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment/main.svelte diff --git a/.changeset/shaggy-spies-happen.md b/.changeset/shaggy-spies-happen.md new file mode 100644 index 000000000000..bb80ffd8f84c --- /dev/null +++ b/.changeset/shaggy-spies-happen.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: proxify values when assigning using `||=`, `&&=` and `??=` operators diff --git a/documentation/docs/98-reference/.generated/client-warnings.md b/documentation/docs/98-reference/.generated/client-warnings.md index 6e9721134119..60eb3e126c42 100644 --- a/documentation/docs/98-reference/.generated/client-warnings.md +++ b/documentation/docs/98-reference/.generated/client-warnings.md @@ -1,5 +1,37 @@ +### assignment_value_stale + +``` +Assignment to `%property%` property (%location%) will evaluate to the right-hand side, not the value of `%property%` following the assignment. This may result in unexpected behaviour. +``` + +Given a case like this... + +```svelte + + + +

items: {JSON.stringify(object.items)}

+``` + +...the array being pushed to when the button is first clicked is the `[]` on the right-hand side of the assignment, but the resulting value of `object.array` is an empty state proxy. As a result, the pushed value will be discarded. + +You can fix this by separating it into two statements: + +```js +function add() { + object.array ??= []; + object.array.push(object.array.length); +} +``` + ### binding_property_non_reactive ``` diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index d82cdc47a1a5..482a3d7faad3 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -1,3 +1,33 @@ +## assignment_value_stale + +> Assignment to `%property%` property (%location%) will evaluate to the right-hand side, not the value of `%property%` following the assignment. This may result in unexpected behaviour. + +Given a case like this... + +```svelte + + + +

items: {JSON.stringify(object.items)}

+``` + +...the array being pushed to when the button is first clicked is the `[]` on the right-hand side of the assignment, but the resulting value of `object.array` is an empty state proxy. As a result, the pushed value will be discarded. + +You can fix this by separating it into two statements: + +```js +function add() { + object.array ??= []; + object.array.push(object.array.length); +} +``` + ## binding_property_non_reactive > `%binding%` is binding to a non-reactive property diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index ee909ede91bf..66ea2c4941a4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -1,8 +1,14 @@ -/** @import { AssignmentExpression, AssignmentOperator, Expression, Pattern } from 'estree' */ +/** @import { Location } from 'locate-character' */ +/** @import { AssignmentExpression, AssignmentOperator, Expression, Identifier, Literal, MemberExpression, Pattern } from 'estree' */ +/** @import { AST } from '#compiler' */ /** @import { Context } from '../types.js' */ import * as b from '../../../../utils/builders.js'; -import { build_assignment_value } from '../../../../utils/ast.js'; -import { is_ignored } from '../../../../state.js'; +import { + build_assignment_value, + get_attribute_expression, + is_event_attribute +} from '../../../../utils/ast.js'; +import { dev, filename, is_ignored, locator } from '../../../../state.js'; import { build_proxy_reassignment, should_proxy } from '../utils.js'; import { visit_assignment_expression } from '../../shared/assignments.js'; @@ -20,6 +26,24 @@ export function AssignmentExpression(node, context) { : expression; } +/** + * Determines whether the value will be coerced on assignment (as with e.g. `+=`). + * If not, we may need to proxify the value, or warn that the value will not be + * proxified in time + * @param {AssignmentOperator} operator + */ +function is_non_coercive_operator(operator) { + return ['=', '||=', '&&=', '??='].includes(operator); +} + +/** @type {Record} */ +const callees = { + '=': '$.assign', + '&&=': '$.assign_and', + '||=': '$.assign_or', + '??=': '$.assign_nullish' +}; + /** * @param {AssignmentOperator} operator * @param {Pattern} left @@ -41,7 +65,11 @@ function build_assignment(operator, left, right, context) { context.visit(build_assignment_value(operator, left, right)) ); - if (private_state.kind !== 'raw_state' && should_proxy(value, context.state.scope)) { + if ( + private_state.kind === 'state' && + is_non_coercive_operator(operator) && + should_proxy(value, context.state.scope) + ) { value = build_proxy_reassignment(value, b.member(b.this, private_state.id)); } @@ -73,24 +101,28 @@ function build_assignment(operator, left, right, context) { ? context.state.transform[object.name] : null; + const path = context.path.map((node) => node.type); + // reassignment if (object === left && transform?.assign) { + // special case — if an element binding, we know it's a primitive + + const is_primitive = path.at(-1) === 'BindDirective' && path.at(-2) === 'RegularElement'; + let value = /** @type {Expression} */ ( context.visit(build_assignment_value(operator, left, right)) ); - // special case — if an element binding, we know it's a primitive - const path = context.path.map((node) => node.type); - const is_primitive = path.at(-1) === 'BindDirective' && path.at(-2) === 'RegularElement'; - if ( !is_primitive && binding.kind !== 'prop' && binding.kind !== 'bindable_prop' && + binding.kind !== 'raw_state' && context.state.analysis.runes && - should_proxy(value, context.state.scope) + should_proxy(right, context.state.scope) && + is_non_coercive_operator(operator) ) { - value = binding.kind === 'raw_state' ? value : build_proxy_reassignment(value, object); + value = build_proxy_reassignment(value, object); } return transform.assign(object, value); @@ -108,5 +140,57 @@ function build_assignment(operator, left, right, context) { ); } + // in cases like `(object.items ??= []).push(value)`, we may need to warn + // if the value gets proxified, since the proxy _isn't_ the thing that + // will be pushed to. we do this by transforming it to something like + // `$.assign_nullish(object, 'items', [])` + let should_transform = + dev && path.at(-1) !== 'ExpressionStatement' && is_non_coercive_operator(operator); + + // special case — ignore `onclick={() => (...)}` + if ( + path.at(-1) === 'ArrowFunctionExpression' && + (path.at(-2) === 'RegularElement' || path.at(-2) === 'SvelteElement') + ) { + const element = /** @type {AST.RegularElement} */ (context.path.at(-2)); + + const attribute = element.attributes.find((attribute) => { + if (attribute.type !== 'Attribute' || !is_event_attribute(attribute)) { + return false; + } + + const expression = get_attribute_expression(attribute); + + return expression === context.path.at(-1); + }); + + if (attribute) { + should_transform = false; + } + } + + if (left.type === 'MemberExpression' && should_transform) { + const callee = callees[operator]; + + const loc = /** @type {Location} */ (locator(/** @type {number} */ (left.start))); + const location = `${filename}:${loc.line}:${loc.column}`; + + return /** @type {Expression} */ ( + context.visit( + b.call( + callee, + /** @type {Expression} */ (left.object), + /** @type {Expression} */ ( + left.computed + ? left.property + : b.literal(/** @type {Identifier} */ (left.property).name) + ), + right, + b.literal(location) + ) + ) + ); + } + return null; } diff --git a/packages/svelte/src/internal/client/dev/assign.js b/packages/svelte/src/internal/client/dev/assign.js new file mode 100644 index 000000000000..cf8c31a941dd --- /dev/null +++ b/packages/svelte/src/internal/client/dev/assign.js @@ -0,0 +1,57 @@ +import * as w from '../warnings.js'; +import { sanitize_location } from './location.js'; + +/** + * + * @param {any} a + * @param {any} b + * @param {string} property + * @param {string} location + */ +function compare(a, b, property, location) { + if (a !== b) { + w.assignment_value_stale(property, /** @type {string} */ (sanitize_location(location))); + } + + return a; +} + +/** + * @param {any} object + * @param {string} property + * @param {any} value + * @param {string} location + */ +export function assign(object, property, value, location) { + return compare((object[property] = value), object[property], property, location); +} + +/** + * @param {any} object + * @param {string} property + * @param {any} value + * @param {string} location + */ +export function assign_and(object, property, value, location) { + return compare((object[property] &&= value), object[property], property, location); +} + +/** + * @param {any} object + * @param {string} property + * @param {any} value + * @param {string} location + */ +export function assign_or(object, property, value, location) { + return compare((object[property] ||= value), object[property], property, location); +} + +/** + * @param {any} object + * @param {string} property + * @param {any} value + * @param {string} location + */ +export function assign_nullish(object, property, value, location) { + return compare((object[property] ??= value), object[property], property, location); +} diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index e8cbefb090c0..6fae2893e678 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -1,4 +1,5 @@ export { FILENAME, HMR, NAMESPACE_SVG } from '../../constants.js'; +export { assign, assign_and, assign_or, assign_nullish } from './dev/assign.js'; export { cleanup_styles } from './dev/css.js'; export { add_locations } from './dev/elements.js'; export { hmr } from './dev/hmr.js'; diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index 5e38db52f020..2f28d3e9e3a4 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -5,6 +5,20 @@ import { DEV } from 'esm-env'; var bold = 'font-weight: bold'; var normal = 'font-weight: normal'; +/** + * Assignment to `%property%` property (%location%) will evaluate to the right-hand side, not the value of `%property%` following the assignment. This may result in unexpected behaviour. + * @param {string} property + * @param {string} location + */ +export function assignment_value_stale(property, location) { + if (DEV) { + console.warn(`%c[svelte] assignment_value_stale\n%cAssignment to \`${property}\` property (${location}) will evaluate to the right-hand side, not the value of \`${property}\` following the assignment. This may result in unexpected behaviour.`, bold, normal); + } else { + // TODO print a link to the documentation + console.warn("assignment_value_stale"); + } +} + /** * `%binding%` (%location%) is binding to a non-reactive property * @param {string} binding diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment-warning/_config.js b/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment-warning/_config.js new file mode 100644 index 000000000000..a6d79c05ed31 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment-warning/_config.js @@ -0,0 +1,24 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + compileOptions: { + dev: true + }, + + html: ``, + + test({ assert, target, warnings }) { + const btn = target.querySelector('button'); + + flushSync(() => btn?.click()); + assert.htmlEqual(target.innerHTML, ``); + + flushSync(() => btn?.click()); + assert.htmlEqual(target.innerHTML, ``); + + assert.deepEqual(warnings, [ + 'Assignment to `items` property (main.svelte:5:24) will evaluate to the right-hand side, not the value of `items` following the assignment. This may result in unexpected behaviour.' + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment-warning/main.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment-warning/main.svelte new file mode 100644 index 000000000000..f151336046f1 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment-warning/main.svelte @@ -0,0 +1,7 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment/_config.js b/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment/_config.js new file mode 100644 index 000000000000..99d957e980fe --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment/_config.js @@ -0,0 +1,16 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ``, + + test({ assert, target }) { + const [btn1, btn2] = target.querySelectorAll('button'); + + flushSync(() => btn1.click()); + assert.htmlEqual(target.innerHTML, ``); + + flushSync(() => btn1.click()); + assert.htmlEqual(target.innerHTML, ``); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment/main.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment/main.svelte new file mode 100644 index 000000000000..84c1c32c5cc8 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment/main.svelte @@ -0,0 +1,7 @@ + + + From 3d9122fcc430b65fcf6b32722d77928d62bc6a0e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 3 Dec 2024 17:51:23 -0500 Subject: [PATCH 14/36] Version Packages (#14537) Co-authored-by: github-actions[bot] --- .changeset/giant-windows-dance.md | 5 ----- .changeset/hip-forks-exercise.md | 5 ----- .changeset/shaggy-spies-happen.md | 5 ----- packages/svelte/CHANGELOG.md | 10 ++++++++++ packages/svelte/package.json | 2 +- packages/svelte/src/version.js | 2 +- 6 files changed, 12 insertions(+), 17 deletions(-) delete mode 100644 .changeset/giant-windows-dance.md delete mode 100644 .changeset/hip-forks-exercise.md delete mode 100644 .changeset/shaggy-spies-happen.md diff --git a/.changeset/giant-windows-dance.md b/.changeset/giant-windows-dance.md deleted file mode 100644 index 7ada8c483643..000000000000 --- a/.changeset/giant-windows-dance.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -fix: don't try to add owners to non-`$state` class fields diff --git a/.changeset/hip-forks-exercise.md b/.changeset/hip-forks-exercise.md deleted file mode 100644 index 41df40bd6a65..000000000000 --- a/.changeset/hip-forks-exercise.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -fix: capture infinite_loop_guard in error boundary diff --git a/.changeset/shaggy-spies-happen.md b/.changeset/shaggy-spies-happen.md deleted file mode 100644 index bb80ffd8f84c..000000000000 --- a/.changeset/shaggy-spies-happen.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -fix: proxify values when assigning using `||=`, `&&=` and `??=` operators diff --git a/packages/svelte/CHANGELOG.md b/packages/svelte/CHANGELOG.md index 8bf0885277b8..4024b6ec7700 100644 --- a/packages/svelte/CHANGELOG.md +++ b/packages/svelte/CHANGELOG.md @@ -1,5 +1,15 @@ # svelte +## 5.5.3 + +### Patch Changes + +- fix: don't try to add owners to non-`$state` class fields ([#14533](https://github.com/sveltejs/svelte/pull/14533)) + +- fix: capture infinite_loop_guard in error boundary ([#14534](https://github.com/sveltejs/svelte/pull/14534)) + +- fix: proxify values when assigning using `||=`, `&&=` and `??=` operators ([#14273](https://github.com/sveltejs/svelte/pull/14273)) + ## 5.5.2 ### Patch Changes diff --git a/packages/svelte/package.json b/packages/svelte/package.json index 0728adf5fc27..3c925c30f676 100644 --- a/packages/svelte/package.json +++ b/packages/svelte/package.json @@ -2,7 +2,7 @@ "name": "svelte", "description": "Cybernetically enhanced web apps", "license": "MIT", - "version": "5.5.2", + "version": "5.5.3", "type": "module", "types": "./types/index.d.ts", "engines": { diff --git a/packages/svelte/src/version.js b/packages/svelte/src/version.js index 0394adda8224..a9ec7f8cf26a 100644 --- a/packages/svelte/src/version.js +++ b/packages/svelte/src/version.js @@ -6,5 +6,5 @@ * https://svelte.dev/docs/svelte-compiler#svelte-version * @type {string} */ -export const VERSION = '5.5.2'; +export const VERSION = '5.5.3'; export const PUBLIC_VERSION = '5'; From 4c4f18b24c644f7e17cd9fea7fde777f3324e206 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 3 Dec 2024 18:31:53 -0500 Subject: [PATCH 15/36] chore: fix docs (#14538) --- .../docs/98-reference/.generated/client-warnings.md | 8 ++++++++ packages/svelte/messages/client-warnings/warnings.md | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/documentation/docs/98-reference/.generated/client-warnings.md b/documentation/docs/98-reference/.generated/client-warnings.md index 60eb3e126c42..ef19a28994bd 100644 --- a/documentation/docs/98-reference/.generated/client-warnings.md +++ b/documentation/docs/98-reference/.generated/client-warnings.md @@ -26,6 +26,8 @@ Given a case like this... You can fix this by separating it into two statements: ```js +let object = { array: [0] }; +// ---cut--- function add() { object.array ??= []; object.array.push(object.array.length); @@ -131,16 +133,22 @@ In runes mode, effects and deriveds re-run when there are changes to the values 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 diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index 482a3d7faad3..019e83e7b120 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -22,6 +22,8 @@ Given a case like this... You can fix this by separating it into two statements: ```js +let object = { array: [0] }; +// ---cut--- function add() { object.array ??= []; object.array.push(object.array.length); @@ -95,16 +97,22 @@ In runes mode, effects and deriveds re-run when there are changes to the values 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 From 1f973b1770a7689b9d54012e72adfd65434da50e Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Wed, 4 Dec 2024 13:06:07 +0100 Subject: [PATCH 16/36] fix: remove spreaded event handlers when they become nullish (#14546) Fixes #14539 --- .changeset/itchy-cooks-yawn.md | 5 +++ .../client/dom/elements/attributes.js | 3 ++ .../remove-spreaded-handlers/_config.js | 32 +++++++++++++++++++ .../remove-spreaded-handlers/main.svelte | 12 +++++++ 4 files changed, 52 insertions(+) create mode 100644 .changeset/itchy-cooks-yawn.md create mode 100644 packages/svelte/tests/runtime-runes/samples/remove-spreaded-handlers/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/remove-spreaded-handlers/main.svelte diff --git a/.changeset/itchy-cooks-yawn.md b/.changeset/itchy-cooks-yawn.md new file mode 100644 index 000000000000..2b23ca5d9391 --- /dev/null +++ b/.changeset/itchy-cooks-yawn.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: remove spreaded event handlers when they become nullish diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index d927af543ff2..ced4c4cf25b3 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -281,6 +281,9 @@ export function set_attributes( element[`__${event_name}`] = value; delegate([event_name]); } + } else if (delegated) { + // @ts-ignore + element[`__${event_name}`] = undefined; } } else if (key === 'style' && value != null) { element.style.cssText = value + ''; diff --git a/packages/svelte/tests/runtime-runes/samples/remove-spreaded-handlers/_config.js b/packages/svelte/tests/runtime-runes/samples/remove-spreaded-handlers/_config.js new file mode 100644 index 000000000000..9cff16d9f54b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/remove-spreaded-handlers/_config.js @@ -0,0 +1,32 @@ +import { ok, test } from '../../test'; +import { flushSync } from 'svelte'; + +export default test({ + async test({ assert, target, instance }) { + const p = target.querySelector('p'); + const btn = target.querySelector('button'); + const input = target.querySelector('input'); + ok(p); + + flushSync(() => { + btn?.click(); + }); + assert.equal(p.innerHTML, '1'); + + flushSync(() => { + input?.click(); + }); + flushSync(() => { + btn?.click(); + }); + assert.equal(p.innerHTML, '1'); + + flushSync(() => { + input?.click(); + }); + flushSync(() => { + btn?.click(); + }); + assert.equal(p.innerHTML, '2'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/remove-spreaded-handlers/main.svelte b/packages/svelte/tests/runtime-runes/samples/remove-spreaded-handlers/main.svelte new file mode 100644 index 000000000000..4ca6e613392b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/remove-spreaded-handlers/main.svelte @@ -0,0 +1,12 @@ + + +

+ {count} +

+ + From 348185794d48544e7efb982b3b4074d82e05b860 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 4 Dec 2024 07:07:25 -0500 Subject: [PATCH 17/36] docs: tweak slot docs (#14543) --- .../docs/99-legacy/20-legacy-slots.md | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/documentation/docs/99-legacy/20-legacy-slots.md b/documentation/docs/99-legacy/20-legacy-slots.md index 5189f2017db8..3474782e93ae 100644 --- a/documentation/docs/99-legacy/20-legacy-slots.md +++ b/documentation/docs/99-legacy/20-legacy-slots.md @@ -74,39 +74,45 @@ If no slotted content is provided, a component can define fallback content by pu Slots can be rendered zero or more times and can pass values _back_ to the parent using props. The parent exposes the values to the slot template using the `let:` directive. -The usual shorthand rules apply — `let:item` is equivalent to `let:item={item}`, and `` is equivalent to ``. - ```svelte - +
    - {#each items as item} + {#each items as data}
  • - + +
    • {/each}
+``` - - -
{thing.text}
+```svelte + + + +
{processed.text}
 ``` +The usual shorthand rules apply — `let:item` is equivalent to `let:item={item}`, and `` is equivalent to ``. + Named slots can also expose values. The `let:` directive goes on the element with the `slot` attribute. ```svelte - +
    {#each items as item}
  • - +
    • {/each}
+``` - +```svelte +
{item.text}

Copyright (c) 2019 Svelte Industries

From d511104ee0f77272ffb81b7ff4a3a0528e0e0302 Mon Sep 17 00:00:00 2001 From: adiGuba Date: Wed, 4 Dec 2024 14:23:24 +0100 Subject: [PATCH 18/36] chore: make test more robust (#14550) --- .../tests/runtime-runes/samples/inline-expressions/main.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/tests/runtime-runes/samples/inline-expressions/main.svelte b/packages/svelte/tests/runtime-runes/samples/inline-expressions/main.svelte index 38a97415d2c1..ab48f4d5e6b6 100644 --- a/packages/svelte/tests/runtime-runes/samples/inline-expressions/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/inline-expressions/main.svelte @@ -1,6 +1,6 @@

Without text expression: 7.36

With text expression: {7.36}

-

With text expression and function call: {(7.36).toLocaleString()}

+

With text expression and function call: {(7.36).toString()}

With text expression and property access: {"test".length}

Hello {('name').toUpperCase().toLowerCase()}!

{"test".length}

From 4e77bdef74e75a8816f09e43cd439d527a9d9e3f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 4 Dec 2024 08:30:20 -0500 Subject: [PATCH 19/36] fix: respect the unidirectional nature of time (#14541) * fix: respect the unidirectional nature of time * note to our future selves --- .changeset/sixty-tables-fold.md | 5 +++++ packages/svelte/src/internal/client/loop.js | 7 +++++-- 2 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 .changeset/sixty-tables-fold.md diff --git a/.changeset/sixty-tables-fold.md b/.changeset/sixty-tables-fold.md new file mode 100644 index 000000000000..83182344cece --- /dev/null +++ b/.changeset/sixty-tables-fold.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: respect the unidirectional nature of time diff --git a/packages/svelte/src/internal/client/loop.js b/packages/svelte/src/internal/client/loop.js index d1c73e344fa3..9c1e972800fd 100644 --- a/packages/svelte/src/internal/client/loop.js +++ b/packages/svelte/src/internal/client/loop.js @@ -4,10 +4,13 @@ import { raf } from './timing.js'; // TODO move this into timing.js where it probably belongs /** - * @param {number} now * @returns {void} */ -function run_tasks(now) { +function run_tasks() { + // use `raf.now()` instead of the `requestAnimationFrame` callback argument, because + // otherwise things can get wonky https://github.com/sveltejs/svelte/pull/14541 + const now = raf.now(); + raf.tasks.forEach((task) => { if (!task.c(now)) { raf.tasks.delete(task); From 86ae349bcc0acc613b6f91757ce661a3a33f0855 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 4 Dec 2024 15:32:02 +0100 Subject: [PATCH 20/36] fix: better error messages for invalid HTML trees (#14445) * fix: better error messages for invalid HTML trees closes #13331 * fix test * more concise * tweak * tweak messages * adjust tests * tweak message slightly, so it doesn't sound like the bad element is the one we're currently encountering * put locations in generated message * tidy up * consistency * fix --------- Co-authored-by: Rich Harris --- .changeset/green-pumpkins-ring.md | 5 ++ .../98-reference/.generated/compile-errors.md | 4 +- .../.generated/compile-warnings.md | 4 +- .../messages/compile-errors/template.md | 4 +- .../messages/compile-warnings/template.md | 4 +- packages/svelte/src/compiler/errors.js | 9 +- .../2-analyze/visitors/ExpressionTag.js | 5 +- .../2-analyze/visitors/RegularElement.js | 18 ++-- .../phases/2-analyze/visitors/Text.js | 5 +- packages/svelte/src/compiler/warnings.js | 9 +- packages/svelte/src/html-tree-validation.js | 85 ++++++++++++------- packages/svelte/src/internal/server/dev.js | 29 ++++--- .../invalid-html-ssr-ancestor/_config.js | 21 +++++ .../invalid-html-ssr-ancestor/form.svelte | 1 + .../invalid-html-ssr-ancestor/main.svelte | 9 ++ .../samples/invalid-html-ssr/_config.js | 4 +- .../invalid-nested-svelte-element/_config.js | 2 +- .../_expected_head.html | 2 +- .../invalid-node-placement-2/errors.json | 2 +- .../invalid-node-placement-4/errors.json | 2 +- .../invalid-node-placement-5/warnings.json | 2 +- .../invalid-node-placement-6/errors.json | 2 +- .../invalid-node-placement-7/errors.json | 2 +- .../invalid-node-placement-8/errors.json | 2 +- .../invalid-node-placement/errors.json | 2 +- 25 files changed, 150 insertions(+), 84 deletions(-) create mode 100644 .changeset/green-pumpkins-ring.md create mode 100644 packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/form.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/main.svelte diff --git a/.changeset/green-pumpkins-ring.md b/.changeset/green-pumpkins-ring.md new file mode 100644 index 000000000000..20bed8e3471e --- /dev/null +++ b/.changeset/green-pumpkins-ring.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: better error messages for invalid HTML trees diff --git a/documentation/docs/98-reference/.generated/compile-errors.md b/documentation/docs/98-reference/.generated/compile-errors.md index e1925324e6ed..3bd162d8d74d 100644 --- a/documentation/docs/98-reference/.generated/compile-errors.md +++ b/documentation/docs/98-reference/.generated/compile-errors.md @@ -487,12 +487,12 @@ A component cannot have a default export ### node_invalid_placement ``` -%thing% is invalid inside `<%parent%>` +%message%. The browser will 'repair' the HTML (by moving, removing, or inserting elements) which breaks Svelte's assumptions about the structure of your components. ``` HTML restricts where certain elements can appear. In case of a violation the browser will 'repair' the HTML in a way that breaks Svelte's assumptions about the structure of your components. Some examples: -- `

hello

world

` will result in `

hello

world

` for example (the `
` autoclosed the `

` because `

` cannot contain block-level elements) +- `

hello

world

` will result in `

hello

world

` (the `
` autoclosed the `

` because `

` cannot contain block-level elements) - `

option a
` will result in `` (the `
` is removed) - `
cell
` will result in `
cell
` (a `` is auto-inserted) diff --git a/documentation/docs/98-reference/.generated/compile-warnings.md b/documentation/docs/98-reference/.generated/compile-warnings.md index cc948e2547ee..481959ba3d5c 100644 --- a/documentation/docs/98-reference/.generated/compile-warnings.md +++ b/documentation/docs/98-reference/.generated/compile-warnings.md @@ -643,12 +643,12 @@ Svelte 5 components are no longer classes. Instantiate them using `mount` or `hy ### node_invalid_placement_ssr ``` -%thing% is invalid inside `<%parent%>`. When rendering this component on the server, the resulting HTML will be modified by the browser, likely resulting in a `hydration_mismatch` warning +%message%. When rendering this component on the server, the resulting HTML will be modified by the browser (by moving, removing, or inserting elements), likely resulting in a `hydration_mismatch` warning ``` HTML restricts where certain elements can appear. In case of a violation the browser will 'repair' the HTML in a way that breaks Svelte's assumptions about the structure of your components. Some examples: -- `

hello

world

` will result in `

hello

world

` for example (the `
` autoclosed the `

` because `

` cannot contain block-level elements) +- `

hello

world

` will result in `

hello

world

` (the `
` autoclosed the `

` because `

` cannot contain block-level elements) - `

option a
` will result in `` (the `
` is removed) - `
cell
` will result in `
cell
` (a `` is auto-inserted) diff --git a/packages/svelte/messages/compile-errors/template.md b/packages/svelte/messages/compile-errors/template.md index 613d11ae5165..9621a6457ba9 100644 --- a/packages/svelte/messages/compile-errors/template.md +++ b/packages/svelte/messages/compile-errors/template.md @@ -190,11 +190,11 @@ ## node_invalid_placement -> %thing% is invalid inside `<%parent%>` +> %message%. The browser will 'repair' the HTML (by moving, removing, or inserting elements) which breaks Svelte's assumptions about the structure of your components. HTML restricts where certain elements can appear. In case of a violation the browser will 'repair' the HTML in a way that breaks Svelte's assumptions about the structure of your components. Some examples: -- `

hello

world

` will result in `

hello

world

` for example (the `
` autoclosed the `

` because `

` cannot contain block-level elements) +- `

hello

world

` will result in `

hello

world

` (the `
` autoclosed the `

` because `

` cannot contain block-level elements) - `

option a
` will result in `` (the `
` is removed) - `
cell
` will result in `
cell
` (a `` is auto-inserted) diff --git a/packages/svelte/messages/compile-warnings/template.md b/packages/svelte/messages/compile-warnings/template.md index bfa75ac7f02e..690681c172a3 100644 --- a/packages/svelte/messages/compile-warnings/template.md +++ b/packages/svelte/messages/compile-warnings/template.md @@ -40,11 +40,11 @@ ## node_invalid_placement_ssr -> %thing% is invalid inside `<%parent%>`. When rendering this component on the server, the resulting HTML will be modified by the browser, likely resulting in a `hydration_mismatch` warning +> %message%. When rendering this component on the server, the resulting HTML will be modified by the browser (by moving, removing, or inserting elements), likely resulting in a `hydration_mismatch` warning HTML restricts where certain elements can appear. In case of a violation the browser will 'repair' the HTML in a way that breaks Svelte's assumptions about the structure of your components. Some examples: -- `

hello

world

` will result in `

hello

world

` for example (the `
` autoclosed the `

` because `

` cannot contain block-level elements) +- `

hello

world

` will result in `

hello

world

` (the `
` autoclosed the `

` because `

` cannot contain block-level elements) - `

option a
` will result in `` (the `
` is removed) - `
cell
` will result in `
cell
` (a `` is auto-inserted) diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index a6e4549e2f0e..901ea1983ea7 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -1043,14 +1043,13 @@ export function mixed_event_handler_syntaxes(node, name) { } /** - * %thing% is invalid inside `<%parent%>` + * %message%. The browser will 'repair' the HTML (by moving, removing, or inserting elements) which breaks Svelte's assumptions about the structure of your components. * @param {null | number | NodeLike} node - * @param {string} thing - * @param {string} parent + * @param {string} message * @returns {never} */ -export function node_invalid_placement(node, thing, parent) { - e(node, "node_invalid_placement", `${thing} is invalid inside \`<${parent}>\``); +export function node_invalid_placement(node, message) { + e(node, "node_invalid_placement", `${message}. The browser will 'repair' the HTML (by moving, removing, or inserting elements) which breaks Svelte's assumptions about the structure of your components.`); } /** diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js index e0d2710a08f0..88fe4e6afaee 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js @@ -12,8 +12,9 @@ export function ExpressionTag(node, context) { const in_template = context.path.at(-1)?.type === 'Fragment'; if (in_template && context.state.parent_element) { - if (!is_tag_valid_with_parent('#text', context.state.parent_element)) { - e.node_invalid_placement(node, '`{expression}`', context.state.parent_element); + const message = is_tag_valid_with_parent('#text', context.state.parent_element); + if (message) { + e.node_invalid_placement(node, message); } } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js index fa6ca0f6e970..7454ab810354 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js @@ -114,15 +114,12 @@ export function RegularElement(node, context) { if (!past_parent) { if (ancestor.type === 'RegularElement' && ancestor.name === context.state.parent_element) { - if (!is_tag_valid_with_parent(node.name, context.state.parent_element)) { + const message = is_tag_valid_with_parent(node.name, context.state.parent_element); + if (message) { if (only_warn) { - w.node_invalid_placement_ssr( - node, - `\`<${node.name}>\``, - context.state.parent_element - ); + w.node_invalid_placement_ssr(node, message); } else { - e.node_invalid_placement(node, `\`<${node.name}>\``, context.state.parent_element); + e.node_invalid_placement(node, message); } } @@ -131,11 +128,12 @@ export function RegularElement(node, context) { } else if (ancestor.type === 'RegularElement') { ancestors.push(ancestor.name); - if (!is_tag_valid_with_ancestor(node.name, ancestors)) { + const message = is_tag_valid_with_ancestor(node.name, ancestors); + if (message) { if (only_warn) { - w.node_invalid_placement_ssr(node, `\`<${node.name}>\``, ancestor.name); + w.node_invalid_placement_ssr(node, message); } else { - e.node_invalid_placement(node, `\`<${node.name}>\``, ancestor.name); + e.node_invalid_placement(node, message); } } } else if ( diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js index b60030f6389d..363a111b7dc6 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js @@ -12,8 +12,9 @@ export function Text(node, context) { const in_template = context.path.at(-1)?.type === 'Fragment'; if (in_template && context.state.parent_element && regex_not_whitespace.test(node.data)) { - if (!is_tag_valid_with_parent('#text', context.state.parent_element)) { - e.node_invalid_placement(node, 'Text node', context.state.parent_element); + const message = is_tag_valid_with_parent('#text', context.state.parent_element); + if (message) { + e.node_invalid_placement(node, message); } } } diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index f6ed7de7d5d2..e193ad0f1109 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -754,13 +754,12 @@ export function event_directive_deprecated(node, name) { } /** - * %thing% is invalid inside `<%parent%>`. When rendering this component on the server, the resulting HTML will be modified by the browser, likely resulting in a `hydration_mismatch` warning + * %message%. When rendering this component on the server, the resulting HTML will be modified by the browser (by moving, removing, or inserting elements), likely resulting in a `hydration_mismatch` warning * @param {null | NodeLike} node - * @param {string} thing - * @param {string} parent + * @param {string} message */ -export function node_invalid_placement_ssr(node, thing, parent) { - w(node, "node_invalid_placement_ssr", `${thing} is invalid inside \`<${parent}>\`. When rendering this component on the server, the resulting HTML will be modified by the browser, likely resulting in a \`hydration_mismatch\` warning`); +export function node_invalid_placement_ssr(node, message) { + w(node, "node_invalid_placement_ssr", `${message}. When rendering this component on the server, the resulting HTML will be modified by the browser (by moving, removing, or inserting elements), likely resulting in a \`hydration_mismatch\` warning`); } /** diff --git a/packages/svelte/src/html-tree-validation.js b/packages/svelte/src/html-tree-validation.js index 0ebf45e166c2..98e74b638f1e 100644 --- a/packages/svelte/src/html-tree-validation.js +++ b/packages/svelte/src/html-tree-validation.js @@ -135,59 +135,85 @@ const disallowed_children = { }; /** - * Returns false if the tag is not allowed inside the ancestor tag (which is grandparent and above) such that it will result + * Returns an error message if the tag is not allowed inside the ancestor tag (which is grandparent and above) such that it will result * in the browser repairing the HTML, which will likely result in an error during hydration. - * @param {string} tag + * @param {string} child_tag * @param {string[]} ancestors All nodes starting with the parent, up until the ancestor, which means two entries minimum - * @returns {boolean} + * @param {string} [child_loc] + * @param {string} [ancestor_loc] + * @returns {string | null} */ -export function is_tag_valid_with_ancestor(tag, ancestors) { - if (tag.includes('-')) return true; // custom elements can be anything +export function is_tag_valid_with_ancestor(child_tag, ancestors, child_loc, ancestor_loc) { + if (child_tag.includes('-')) return null; // custom elements can be anything - const target = ancestors[ancestors.length - 1]; - const disallowed = disallowed_children[target]; - if (!disallowed) return true; + const ancestor_tag = ancestors[ancestors.length - 1]; + const disallowed = disallowed_children[ancestor_tag]; + if (!disallowed) return null; if ('reset_by' in disallowed && disallowed.reset_by) { for (let i = ancestors.length - 2; i >= 0; i--) { const ancestor = ancestors[i]; - if (ancestor.includes('-')) return true; // custom elements can be anything + if (ancestor.includes('-')) return null; // custom elements can be anything // A reset means that forbidden descendants are allowed again if (disallowed.reset_by.includes(ancestors[i])) { - return true; + return null; } } } - return 'descendant' in disallowed ? !disallowed.descendant.includes(tag) : true; + if ('descendant' in disallowed && disallowed.descendant.includes(child_tag)) { + const child = child_loc ? `\`<${child_tag}>\` (${child_loc})` : `\`<${child_tag}>\``; + const ancestor = ancestor_loc + ? `\`<${ancestor_tag}>\` (${ancestor_loc})` + : `\`<${ancestor_tag}>\``; + + return `${child} cannot be a descendant of ${ancestor}`; + } + + return null; } /** - * Returns false if the tag is not allowed inside the parent tag such that it will result + * Returns an error message if the tag is not allowed inside the parent tag such that it will result * in the browser repairing the HTML, which will likely result in an error during hydration. - * @param {string} tag + * @param {string} child_tag * @param {string} parent_tag - * @returns {boolean} + * @param {string} [child_loc] + * @param {string} [parent_loc] + * @returns {string | null} */ -export function is_tag_valid_with_parent(tag, parent_tag) { - if (tag.includes('-') || parent_tag?.includes('-')) return true; // custom elements can be anything +export function is_tag_valid_with_parent(child_tag, parent_tag, child_loc, parent_loc) { + if (child_tag.includes('-') || parent_tag?.includes('-')) return null; // custom elements can be anything const disallowed = disallowed_children[parent_tag]; + const child = child_loc ? `\`<${child_tag}>\` (${child_loc})` : `\`<${child_tag}>\``; + const parent = parent_loc ? `\`<${parent_tag}>\` (${parent_loc})` : `\`<${parent_tag}>\``; + if (disallowed) { - if ('direct' in disallowed && disallowed.direct.includes(tag)) { - return false; + if ('direct' in disallowed && disallowed.direct.includes(child_tag)) { + return `${child} cannot be a direct child of ${parent}`; } - if ('descendant' in disallowed && disallowed.descendant.includes(tag)) { - return false; + + if ('descendant' in disallowed && disallowed.descendant.includes(child_tag)) { + return `${child} cannot be a child of ${parent}`; } + if ('only' in disallowed && disallowed.only) { - return disallowed.only.includes(tag); + if (disallowed.only.includes(child_tag)) { + return null; + } else { + return `${child} cannot be a child of ${parent}. \`<${parent_tag}>\` only allows these children: ${disallowed.only.map((d) => `\`<${d}>\``).join(', ')}`; + } } } - switch (tag) { + // These tags are only valid with a few parents that have special child + // parsing rules - if we're down here, then none of those matched and + // so we allow it only if we don't know what the parent is, as all other + // cases are invalid (and we only get into this function if we know the parent). + switch (child_tag) { case 'body': case 'caption': case 'col': @@ -196,18 +222,17 @@ export function is_tag_valid_with_parent(tag, parent_tag) { case 'frame': case 'head': case 'html': + return `${child} cannot be a child of ${parent}`; + case 'thead': case 'tbody': - case 'td': case 'tfoot': + return `${child} must be the child of a \`\`, not a ${parent}`; + case 'td': case 'th': - case 'thead': + return `${child} must be the child of a \`\`, not a ${parent}`; case 'tr': - // These tags are only valid with a few parents that have special child - // parsing rules - if we're down here, then none of those matched and - // so we allow it only if we don't know what the parent is, as all other - // cases are invalid (and we only get into this function if we know the parent). - return false; + return `\`\` must be the child of a \`\`, \`\`, or \`\`, not a ${parent}`; } - return true; + return null; } diff --git a/packages/svelte/src/internal/server/dev.js b/packages/svelte/src/internal/server/dev.js index 145b37479b3f..ecf4e67429ac 100644 --- a/packages/svelte/src/internal/server/dev.js +++ b/packages/svelte/src/internal/server/dev.js @@ -34,12 +34,11 @@ function stringify(element) { /** * @param {Payload} payload - * @param {Element} parent - * @param {Element} child + * @param {string} message */ -function print_error(payload, parent, child) { - var message = - `node_invalid_placement_ssr: ${stringify(parent)} cannot contain ${stringify(child)}\n\n` + +function print_error(payload, message) { + message = + `node_invalid_placement_ssr: ${message}\n\n` + 'This can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.'; if ((seen ??= new Set()).has(message)) return; @@ -72,15 +71,23 @@ export function push_element(payload, tag, line, column) { var ancestor = parent.parent; var ancestors = [parent.tag]; - if (!is_tag_valid_with_parent(tag, parent.tag)) { - print_error(payload, parent, child); - } + const child_loc = filename ? `${filename}:${line}:${column}` : undefined; + const parent_loc = parent.filename + ? `${parent.filename}:${parent.line}:${parent.column}` + : undefined; + + const message = is_tag_valid_with_parent(tag, parent.tag, child_loc, parent_loc); + if (message) print_error(payload, message); while (ancestor != null) { ancestors.push(ancestor.tag); - if (!is_tag_valid_with_ancestor(tag, ancestors)) { - print_error(payload, ancestor, child); - } + const ancestor_loc = ancestor.filename + ? `${ancestor.filename}:${ancestor.line}:${ancestor.column}` + : undefined; + + const message = is_tag_valid_with_ancestor(tag, ancestors, child_loc, ancestor_loc); + if (message) print_error(payload, message); + ancestor = ancestor.parent; } } diff --git a/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/_config.js b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/_config.js new file mode 100644 index 000000000000..4c45b2617965 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/_config.js @@ -0,0 +1,21 @@ +import { test } from '../../test'; + +export default test({ + compileOptions: { + dev: true + }, + + html: `
`, + + recover: true, + + mode: ['hydrate'], + + errors: [ + 'node_invalid_placement_ssr: `` (form.svelte:1:0) cannot be a descendant of `` (main.svelte:5:0)\n\nThis can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.' + ], + + warnings: [ + 'Hydration failed because the initial UI does not match what was rendered on the server' + ] +}); diff --git a/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/form.svelte b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/form.svelte new file mode 100644 index 000000000000..cddb6e37947d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/form.svelte @@ -0,0 +1 @@ + diff --git a/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/main.svelte b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/main.svelte new file mode 100644 index 000000000000..8aea34332288 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr-ancestor/main.svelte @@ -0,0 +1,9 @@ + + + +
+ +
+ diff --git a/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/_config.js b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/_config.js index d815d10fc73e..bc4d709b1924 100644 --- a/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/_config.js @@ -12,8 +12,8 @@ export default test({ mode: ['hydrate'], errors: [ - 'node_invalid_placement_ssr: `

` (main.svelte:6:0) cannot contain `

` (h1.svelte:1:0)\n\nThis can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.', - 'node_invalid_placement_ssr: `
` (main.svelte:9:0) cannot contain `` (form.svelte:1:0)\n\nThis can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.' + 'node_invalid_placement_ssr: `

` (h1.svelte:1:0) cannot be a child of `

` (main.svelte:6:0)\n\nThis can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.', + 'node_invalid_placement_ssr: `` (form.svelte:1:0) cannot be a child of `` (main.svelte:9:0)\n\nThis can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.' ], warnings: [ diff --git a/packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/_config.js b/packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/_config.js index 9b2688711ded..71edff6a6836 100644 --- a/packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/_config.js +++ b/packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/_config.js @@ -6,6 +6,6 @@ export default test({ }, errors: [ - 'node_invalid_placement_ssr: `

` (packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/main.svelte:1:0) cannot contain `

` (packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/main.svelte:2:1)\n\nThis can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.' + 'node_invalid_placement_ssr: `

` (packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/main.svelte:2:1) cannot be a child of `

` (packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/main.svelte:1:0)\n\nThis can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.' ] }); diff --git a/packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/_expected_head.html b/packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/_expected_head.html index 27c37f693baa..6d9ea9de5f77 100644 --- a/packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/_expected_head.html +++ b/packages/svelte/tests/server-side-rendering/samples/invalid-nested-svelte-element/_expected_head.html @@ -1 +1 @@ - \ No newline at end of file + diff --git a/packages/svelte/tests/validator/samples/invalid-node-placement-2/errors.json b/packages/svelte/tests/validator/samples/invalid-node-placement-2/errors.json index 3d9786582343..abbded296af1 100644 --- a/packages/svelte/tests/validator/samples/invalid-node-placement-2/errors.json +++ b/packages/svelte/tests/validator/samples/invalid-node-placement-2/errors.json @@ -1,7 +1,7 @@ [ { "code": "node_invalid_placement", - "message": "`

` is invalid inside `

`", + "message": "`

` cannot be a descendant of `

`. The browser will 'repair' the HTML (by moving, removing, or inserting elements) which breaks Svelte's assumptions about the structure of your components.", "start": { "line": 4, "column": 3 diff --git a/packages/svelte/tests/validator/samples/invalid-node-placement-4/errors.json b/packages/svelte/tests/validator/samples/invalid-node-placement-4/errors.json index 4d637aed805a..727bf6c258ca 100644 --- a/packages/svelte/tests/validator/samples/invalid-node-placement-4/errors.json +++ b/packages/svelte/tests/validator/samples/invalid-node-placement-4/errors.json @@ -1,7 +1,7 @@ [ { "code": "node_invalid_placement", - "message": "`` is invalid inside ``", + "message": "`` cannot be a descendant of ``. The browser will 'repair' the HTML (by moving, removing, or inserting elements) which breaks Svelte's assumptions about the structure of your components.", "start": { "line": 4, "column": 3 diff --git a/packages/svelte/tests/validator/samples/invalid-node-placement-5/warnings.json b/packages/svelte/tests/validator/samples/invalid-node-placement-5/warnings.json index e85050beb721..59c73c4e736d 100644 --- a/packages/svelte/tests/validator/samples/invalid-node-placement-5/warnings.json +++ b/packages/svelte/tests/validator/samples/invalid-node-placement-5/warnings.json @@ -1,7 +1,7 @@ [ { "code": "node_invalid_placement_ssr", - "message": "`` is invalid inside ``. When rendering this component on the server, the resulting HTML will be modified by the browser, likely resulting in a `hydration_mismatch` warning", + "message": "`` cannot be a child of ``. When rendering this component on the server, the resulting HTML will be modified by the browser (by moving, removing, or inserting elements), likely resulting in a `hydration_mismatch` warning", "start": { "line": 4, "column": 3 diff --git a/packages/svelte/tests/validator/samples/invalid-node-placement-6/errors.json b/packages/svelte/tests/validator/samples/invalid-node-placement-6/errors.json index 63fc9c517e47..4849717e146d 100644 --- a/packages/svelte/tests/validator/samples/invalid-node-placement-6/errors.json +++ b/packages/svelte/tests/validator/samples/invalid-node-placement-6/errors.json @@ -1,7 +1,7 @@ [ { "code": "node_invalid_placement", - "message": "`

` is invalid inside `
`", + "message": "`
` cannot be a descendant of `
`. The browser will 'repair' the HTML (by moving, removing, or inserting elements) which breaks Svelte's assumptions about the structure of your components.", "start": { "line": 16, "column": 3 diff --git a/packages/svelte/tests/validator/samples/invalid-node-placement-7/errors.json b/packages/svelte/tests/validator/samples/invalid-node-placement-7/errors.json index edfc158c9d56..a1422c001a0c 100644 --- a/packages/svelte/tests/validator/samples/invalid-node-placement-7/errors.json +++ b/packages/svelte/tests/validator/samples/invalid-node-placement-7/errors.json @@ -1,7 +1,7 @@ [ { "code": "node_invalid_placement", - "message": "`

` is invalid inside `
`", + "message": "`
` must be the child of a `
`, not a `
`. The browser will 'repair' the HTML (by moving, removing, or inserting elements) which breaks Svelte's assumptions about the structure of your components.", "start": { "line": 8, "column": 1 diff --git a/packages/svelte/tests/validator/samples/invalid-node-placement-8/errors.json b/packages/svelte/tests/validator/samples/invalid-node-placement-8/errors.json index 1127a7633624..d75d5d410521 100644 --- a/packages/svelte/tests/validator/samples/invalid-node-placement-8/errors.json +++ b/packages/svelte/tests/validator/samples/invalid-node-placement-8/errors.json @@ -1,7 +1,7 @@ [ { "code": "node_invalid_placement", - "message": "Text node is invalid inside `
`", + "message": "`<#text>` cannot be a child of ``. `` only allows these children: ``, `