From 2ffb55ce7d69835651ad70ec316f3fb0e0a2a992 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 8 Jul 2024 10:32:59 +0200 Subject: [PATCH 01/16] fix: reorder reactive statements during migration (#12329) fixes #12183 --- .changeset/few-cheetahs-taste.md | 5 ++ packages/svelte/src/compiler/migrate/index.js | 73 ++++++++++++++++++- .../input.svelte | 7 ++ .../output.svelte | 7 ++ .../input.svelte | 7 ++ .../output.svelte | 11 +++ 6 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 .changeset/few-cheetahs-taste.md create mode 100644 packages/svelte/tests/migrate/samples/reactive-statements-reorder-1/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/reactive-statements-reorder-1/output.svelte create mode 100644 packages/svelte/tests/migrate/samples/reactive-statements-reorder-2/input.svelte create mode 100644 packages/svelte/tests/migrate/samples/reactive-statements-reorder-2/output.svelte diff --git a/.changeset/few-cheetahs-taste.md b/.changeset/few-cheetahs-taste.md new file mode 100644 index 0000000000..34da32f362 --- /dev/null +++ b/.changeset/few-cheetahs-taste.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: reorder reactive statements during migration diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index b5a1e5596e..308f5ab235 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -145,6 +145,47 @@ export function migrate(source) { } } + /** + * If true, then we need to move all reactive statements to the end of the script block, + * in their correct order. Svelte 4 reordered reactive statements, $derived/$effect.pre + * don't have this behavior. + */ + let needs_reordering = false; + + for (const [node, { dependencies }] of state.analysis.reactive_statements) { + /** @type {Compiler.Binding[]} */ + let ids = []; + if ( + node.body.type === 'ExpressionStatement' && + node.body.expression.type === 'AssignmentExpression' + ) { + ids = extract_identifiers(node.body.expression.left) + .map((id) => state.scope.get(id.name)) + .filter((id) => !!id); + } + + if ( + dependencies.some( + (dep) => + !ids.includes(dep) && + /** @type {number} */ (dep.node.start) > /** @type {number} */ (node.start) + ) + ) { + needs_reordering = true; + break; + } + } + + if (needs_reordering) { + const nodes = Array.from(state.analysis.reactive_statements.keys()); + for (const node of nodes) { + const { start, end } = get_node_range(source, node); + str.appendLeft(end, '\n'); + str.move(start, end, /** @type {number} */ (parsed.instance?.content.end)); + str.remove(start - (source[start - 2] === '\r' ? 2 : 1), start); + } + } + if (state.needs_run && !added_legacy_import) { if (parsed.instance) { str.appendRight( @@ -377,7 +418,7 @@ const instance_script = { /** @type {number} */ (node.body.expression.start), 'let ' ); - state.str.prependLeft( + state.str.prependRight( /** @type {number} */ (node.body.expression.right.start), '$derived(' ); @@ -388,14 +429,14 @@ const instance_script = { ');' ); } else { - state.str.appendRight(/** @type {number} */ (node.end), ');'); + state.str.appendLeft(/** @type {number} */ (node.end), ');'); } return; } else { for (const binding of reassigned_bindings) { if (binding && ids.includes(binding.node)) { // implicitly-declared variable which we need to make explicit - state.str.prependLeft( + state.str.prependRight( /** @type {number} */ (node.start), `let ${binding.node.name}${binding.kind === 'state' ? ' = $state()' : ''};\n${state.indent}` ); @@ -428,7 +469,7 @@ const instance_script = { [/** @type {number} */ (node.body.end), state.end] ] }); - state.str.appendRight(/** @type {number} */ (node.end), `\n${state.indent}});`); + state.str.appendLeft(/** @type {number} */ (node.end), `\n${state.indent}});`); } } }; @@ -808,6 +849,30 @@ function get_indent(state, ...nodes) { return indent; } +/** + * Returns start and end of the node. If the start is preceeded with white-space-only before a line break, + * the start will be the start of the line. + * @param {string} source + * @param {Node} node + */ +function get_node_range(source, node) { + let start = /** @type {number} */ (node.start); + let end = /** @type {number} */ (node.end); + + let idx = start; + while (source[idx - 1] !== '\n' && source[idx - 1] !== '\r') { + idx--; + if (source[idx] !== ' ' && source[idx] !== '\t') { + idx = start; + break; + } + } + + start = idx; + + return { start, end }; +} + /** * @param {Compiler.OnDirective} last * @param {State} state diff --git a/packages/svelte/tests/migrate/samples/reactive-statements-reorder-1/input.svelte b/packages/svelte/tests/migrate/samples/reactive-statements-reorder-1/input.svelte new file mode 100644 index 0000000000..fe4c6724b2 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/reactive-statements-reorder-1/input.svelte @@ -0,0 +1,7 @@ + + +{width / mobile / x} diff --git a/packages/svelte/tests/migrate/samples/reactive-statements-reorder-1/output.svelte b/packages/svelte/tests/migrate/samples/reactive-statements-reorder-1/output.svelte new file mode 100644 index 0000000000..bf54a1fd7c --- /dev/null +++ b/packages/svelte/tests/migrate/samples/reactive-statements-reorder-1/output.svelte @@ -0,0 +1,7 @@ + + +{width / mobile / x} \ No newline at end of file diff --git a/packages/svelte/tests/migrate/samples/reactive-statements-reorder-2/input.svelte b/packages/svelte/tests/migrate/samples/reactive-statements-reorder-2/input.svelte new file mode 100644 index 0000000000..ca6d5941c2 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/reactive-statements-reorder-2/input.svelte @@ -0,0 +1,7 @@ + + +{width / mobile} diff --git a/packages/svelte/tests/migrate/samples/reactive-statements-reorder-2/output.svelte b/packages/svelte/tests/migrate/samples/reactive-statements-reorder-2/output.svelte new file mode 100644 index 0000000000..52c57f5669 --- /dev/null +++ b/packages/svelte/tests/migrate/samples/reactive-statements-reorder-2/output.svelte @@ -0,0 +1,11 @@ + + +{width / mobile} \ No newline at end of file From 67bf7a80673ce9e1a6f5f5e7ab70be2b89a05690 Mon Sep 17 00:00:00 2001 From: Yuichiro Yamashita Date: Mon, 8 Jul 2024 18:46:47 +0900 Subject: [PATCH 02/16] fix: correct start of `{:else if}` and `{:else}` (#12043) The modern AST is an opportunity to tweak things. In the old AST, the start of else (if) branches was the content, now it's the opening bracket, which makes more sense. --- .changeset/cold-teachers-turn.md | 5 + packages/svelte/src/compiler/legacy.js | 3 +- .../src/compiler/phases/1-parse/state/tag.js | 2 +- .../samples/if-block-else/input.svelte | 5 + .../samples/if-block-else/output.json | 116 ++++++++++ .../samples/if-block-elseif/input.svelte | 5 + .../samples/if-block-elseif/output.json | 211 ++++++++++++++++++ .../samples/if-block/input.svelte | 1 + .../samples/if-block/output.json | 50 +++++ 9 files changed, 396 insertions(+), 2 deletions(-) create mode 100644 .changeset/cold-teachers-turn.md create mode 100644 packages/svelte/tests/parser-modern/samples/if-block-else/input.svelte create mode 100644 packages/svelte/tests/parser-modern/samples/if-block-else/output.json create mode 100644 packages/svelte/tests/parser-modern/samples/if-block-elseif/input.svelte create mode 100644 packages/svelte/tests/parser-modern/samples/if-block-elseif/output.json create mode 100644 packages/svelte/tests/parser-modern/samples/if-block/input.svelte create mode 100644 packages/svelte/tests/parser-modern/samples/if-block/output.json diff --git a/.changeset/cold-teachers-turn.md b/.changeset/cold-teachers-turn.md new file mode 100644 index 0000000000..e86ffbcdfe --- /dev/null +++ b/.changeset/cold-teachers-turn.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: correct start of `{:else if}` and `{:else}` diff --git a/packages/svelte/src/compiler/legacy.js b/packages/svelte/src/compiler/legacy.js index caa7f8238e..da0ba058d3 100644 --- a/packages/svelte/src/compiler/legacy.js +++ b/packages/svelte/src/compiler/legacy.js @@ -353,11 +353,12 @@ export function convert(source, ast) { }; } + const start = node.elseif ? node.consequent.nodes[0].start : node.start; remove_surrounding_whitespace_nodes(node.consequent.nodes); return { type: 'IfBlock', - start: node.start, + start, end: node.end, expression: node.test, children: node.consequent.nodes.map( diff --git a/packages/svelte/src/compiler/phases/1-parse/state/tag.js b/packages/svelte/src/compiler/phases/1-parse/state/tag.js index baa86783b0..14af03f6f6 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/tag.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/tag.js @@ -345,7 +345,7 @@ function next(parser) { /** @type {ReturnType>} */ const child = parser.append({ - start: parser.index, + start: start - 1, end: -1, type: 'IfBlock', elseif: true, diff --git a/packages/svelte/tests/parser-modern/samples/if-block-else/input.svelte b/packages/svelte/tests/parser-modern/samples/if-block-else/input.svelte new file mode 100644 index 0000000000..28b41c046c --- /dev/null +++ b/packages/svelte/tests/parser-modern/samples/if-block-else/input.svelte @@ -0,0 +1,5 @@ +{#if foo} +

foo

+{:else} +

not foo

+{/if} diff --git a/packages/svelte/tests/parser-modern/samples/if-block-else/output.json b/packages/svelte/tests/parser-modern/samples/if-block-else/output.json new file mode 100644 index 0000000000..1193af156e --- /dev/null +++ b/packages/svelte/tests/parser-modern/samples/if-block-else/output.json @@ -0,0 +1,116 @@ +{ + "css": null, + "js": [], + "start": 0, + "end": 51, + "type": "Root", + "fragment": { + "type": "Fragment", + "nodes": [ + { + "type": "IfBlock", + "elseif": false, + "start": 0, + "end": 51, + "test": { + "type": "Identifier", + "start": 5, + "end": 8, + "loc": { + "start": { + "line": 1, + "column": 5 + }, + "end": { + "line": 1, + "column": 8 + } + }, + "name": "foo" + }, + "consequent": { + "type": "Fragment", + "nodes": [ + { + "type": "Text", + "start": 9, + "end": 11, + "raw": "\n\t", + "data": "\n\t" + }, + { + "type": "RegularElement", + "start": 11, + "end": 21, + "name": "p", + "attributes": [], + "fragment": { + "type": "Fragment", + "nodes": [ + { + "type": "Text", + "start": 14, + "end": 17, + "raw": "foo", + "data": "foo" + } + ], + "transparent": true + } + }, + { + "type": "Text", + "start": 21, + "end": 22, + "raw": "\n", + "data": "\n" + } + ], + "transparent": false + }, + "alternate": { + "type": "Fragment", + "nodes": [ + { + "type": "Text", + "start": 29, + "end": 31, + "raw": "\n\t", + "data": "\n\t" + }, + { + "type": "RegularElement", + "start": 31, + "end": 45, + "name": "p", + "attributes": [], + "fragment": { + "type": "Fragment", + "nodes": [ + { + "type": "Text", + "start": 34, + "end": 41, + "raw": "not foo", + "data": "not foo" + } + ], + "transparent": true + } + }, + { + "type": "Text", + "start": 45, + "end": 46, + "raw": "\n", + "data": "\n" + } + ], + "transparent": false + } + } + ], + "transparent": false + }, + "options": null +} diff --git a/packages/svelte/tests/parser-modern/samples/if-block-elseif/input.svelte b/packages/svelte/tests/parser-modern/samples/if-block-elseif/input.svelte new file mode 100644 index 0000000000..2566aef258 --- /dev/null +++ b/packages/svelte/tests/parser-modern/samples/if-block-elseif/input.svelte @@ -0,0 +1,5 @@ +{#if x > 10} +

x is greater than 10

+{:else if x < 5} +

x is less than 5

+{/if} diff --git a/packages/svelte/tests/parser-modern/samples/if-block-elseif/output.json b/packages/svelte/tests/parser-modern/samples/if-block-elseif/output.json new file mode 100644 index 0000000000..e68b154a55 --- /dev/null +++ b/packages/svelte/tests/parser-modern/samples/if-block-elseif/output.json @@ -0,0 +1,211 @@ +{ + "css": null, + "js": [], + "start": 0, + "end": 89, + "type": "Root", + "fragment": { + "type": "Fragment", + "nodes": [ + { + "type": "IfBlock", + "elseif": false, + "start": 0, + "end": 89, + "test": { + "type": "BinaryExpression", + "start": 5, + "end": 11, + "loc": { + "start": { + "line": 1, + "column": 5 + }, + "end": { + "line": 1, + "column": 11 + } + }, + "left": { + "type": "Identifier", + "start": 5, + "end": 6, + "loc": { + "start": { + "line": 1, + "column": 5 + }, + "end": { + "line": 1, + "column": 6 + } + }, + "name": "x" + }, + "operator": ">", + "right": { + "type": "Literal", + "start": 9, + "end": 11, + "loc": { + "start": { + "line": 1, + "column": 9 + }, + "end": { + "line": 1, + "column": 11 + } + }, + "value": 10, + "raw": "10" + } + }, + "consequent": { + "type": "Fragment", + "nodes": [ + { + "type": "Text", + "start": 12, + "end": 14, + "raw": "\n\t", + "data": "\n\t" + }, + { + "type": "RegularElement", + "start": 14, + "end": 41, + "name": "p", + "attributes": [], + "fragment": { + "type": "Fragment", + "nodes": [ + { + "type": "Text", + "start": 17, + "end": 37, + "raw": "x is greater than 10", + "data": "x is greater than 10" + } + ], + "transparent": true + } + }, + { + "type": "Text", + "start": 41, + "end": 42, + "raw": "\n", + "data": "\n" + } + ], + "transparent": false + }, + "alternate": { + "type": "Fragment", + "nodes": [ + { + "start": 42, + "end": 89, + "type": "IfBlock", + "elseif": true, + "test": { + "type": "BinaryExpression", + "start": 52, + "end": 57, + "loc": { + "start": { + "line": 3, + "column": 10 + }, + "end": { + "line": 3, + "column": 15 + } + }, + "left": { + "type": "Identifier", + "start": 52, + "end": 53, + "loc": { + "start": { + "line": 3, + "column": 10 + }, + "end": { + "line": 3, + "column": 11 + } + }, + "name": "x" + }, + "operator": "<", + "right": { + "type": "Literal", + "start": 56, + "end": 57, + "loc": { + "start": { + "line": 3, + "column": 14 + }, + "end": { + "line": 3, + "column": 15 + } + }, + "value": 5, + "raw": "5" + } + }, + "consequent": { + "type": "Fragment", + "nodes": [ + { + "type": "Text", + "start": 58, + "end": 60, + "raw": "\n\t", + "data": "\n\t" + }, + { + "type": "RegularElement", + "start": 60, + "end": 83, + "name": "p", + "attributes": [], + "fragment": { + "type": "Fragment", + "nodes": [ + { + "type": "Text", + "start": 63, + "end": 79, + "raw": "x is less than 5", + "data": "x is less than 5" + } + ], + "transparent": true + } + }, + { + "type": "Text", + "start": 83, + "end": 84, + "raw": "\n", + "data": "\n" + } + ], + "transparent": false + }, + "alternate": null + } + ], + "transparent": false + } + } + ], + "transparent": false + }, + "options": null +} diff --git a/packages/svelte/tests/parser-modern/samples/if-block/input.svelte b/packages/svelte/tests/parser-modern/samples/if-block/input.svelte new file mode 100644 index 0000000000..5851347a88 --- /dev/null +++ b/packages/svelte/tests/parser-modern/samples/if-block/input.svelte @@ -0,0 +1 @@ +{#if foo}bar{/if} diff --git a/packages/svelte/tests/parser-modern/samples/if-block/output.json b/packages/svelte/tests/parser-modern/samples/if-block/output.json new file mode 100644 index 0000000000..965f2b5614 --- /dev/null +++ b/packages/svelte/tests/parser-modern/samples/if-block/output.json @@ -0,0 +1,50 @@ +{ + "css": null, + "js": [], + "start": 0, + "end": 17, + "type": "Root", + "fragment": { + "type": "Fragment", + "nodes": [ + { + "type": "IfBlock", + "elseif": false, + "start": 0, + "end": 17, + "test": { + "type": "Identifier", + "start": 5, + "end": 8, + "loc": { + "start": { + "line": 1, + "column": 5 + }, + "end": { + "line": 1, + "column": 8 + } + }, + "name": "foo" + }, + "consequent": { + "type": "Fragment", + "nodes": [ + { + "type": "Text", + "start": 9, + "end": 12, + "raw": "bar", + "data": "bar" + } + ], + "transparent": false + }, + "alternate": null + } + ], + "transparent": false + }, + "options": null +} From cf16acda3265a7ae6f13cdb1101ca692cbd90842 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 8 Jul 2024 15:55:00 +0200 Subject: [PATCH 03/16] fix: enable local transitions on `svelte:element` (#12346) fixes #12231 --- .changeset/wild-cows-chew.md | 5 +++++ .../client/dom/blocks/svelte-element.js | 3 ++- .../dynamic-element-transition/_config.js | 21 +++++++++++++++++++ .../dynamic-element-transition/main.svelte | 12 +++++++++++ 4 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 .changeset/wild-cows-chew.md create mode 100644 packages/svelte/tests/runtime-runes/samples/dynamic-element-transition/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/dynamic-element-transition/main.svelte diff --git a/.changeset/wild-cows-chew.md b/.changeset/wild-cows-chew.md new file mode 100644 index 0000000000..55e5163e7b --- /dev/null +++ b/.changeset/wild-cows-chew.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: enable local transitions on `svelte:element` diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js index b216ea506c..f966ac7fa6 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js @@ -14,6 +14,7 @@ import { current_component_context, current_effect } from '../../runtime.js'; import { DEV } from 'esm-env'; import { assign_nodes } from '../template.js'; import { noop } from '../../../shared/utils.js'; +import { EFFECT_TRANSPARENT } from '../../constants.js'; /** * @param {Comment | Element} node @@ -135,5 +136,5 @@ export function element(node, get_tag, is_svg, render_fn, get_namespace, locatio // Inert effects are proactively detached from the effect tree. Returning a noop // teardown function is an easy way to ensure that this is not discarded return noop; - }); + }, EFFECT_TRANSPARENT); } diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-element-transition/_config.js b/packages/svelte/tests/runtime-runes/samples/dynamic-element-transition/_config.js new file mode 100644 index 0000000000..8c0ebf93e3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/dynamic-element-transition/_config.js @@ -0,0 +1,21 @@ +import { test } from '../../test'; +import { flushSync } from 'svelte'; + +export default test({ + async test({ assert, target, raf }) { + const btn = target.querySelector('button'); + + raf.tick(0); + btn?.click(); + flushSync(); + + assert.htmlEqual( + target.innerHTML, + `
DIV
` + ); + + raf.tick(100); + + assert.htmlEqual(target.innerHTML, `
DIV
`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-element-transition/main.svelte b/packages/svelte/tests/runtime-runes/samples/dynamic-element-transition/main.svelte new file mode 100644 index 0000000000..f9be144969 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/dynamic-element-transition/main.svelte @@ -0,0 +1,12 @@ + + + + +{#if show} + DIV +{/if} From 243c4b78b163140c81095483bb1121b1c988baf3 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 8 Jul 2024 16:54:21 +0200 Subject: [PATCH 04/16] fix: ensure `$effect.root` is ignored on the server (#12332) Ignore the contents of the effect root, just return a noop where necessary fixes #12322 --- .changeset/wicked-emus-drive.md | 5 +++++ .../3-transform/server/transform-server.js | 7 ++----- packages/svelte/tests/runtime-legacy/shared.ts | 3 ++- .../samples/effect-root-4/_config.js | 18 ++++++++++++++++++ .../samples/effect-root-4/main.svelte | 10 ++++++++++ .../samples/effect-root/_config.js | 3 +++ .../event-store-no-hoisting/main.svelte | 3 +-- 7 files changed, 41 insertions(+), 8 deletions(-) create mode 100644 .changeset/wicked-emus-drive.md create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-root-4/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-root-4/main.svelte diff --git a/.changeset/wicked-emus-drive.md b/.changeset/wicked-emus-drive.md new file mode 100644 index 0000000000..96629d079d --- /dev/null +++ b/.changeset/wicked-emus-drive.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure `$effect.root` is ignored on the server diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 4c77aa7ffe..d5c9c790b7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -412,11 +412,8 @@ const global_visitors = { } if (rune === '$effect.root') { - const args = /** @type {import('estree').Expression[]} */ ( - node.arguments.map((arg) => context.visit(arg)) - ); - // Just call the function directly - return b.call(args[0]); + // ignore $effect.root() calls, just return a noop which mimics the cleanup function + return b.arrow([], b.block([])); } if (rune === '$state.snapshot') { diff --git a/packages/svelte/tests/runtime-legacy/shared.ts b/packages/svelte/tests/runtime-legacy/shared.ts index ed32d2f0c1..62a85c182f 100644 --- a/packages/svelte/tests/runtime-legacy/shared.ts +++ b/packages/svelte/tests/runtime-legacy/shared.ts @@ -61,7 +61,7 @@ export interface RuntimeTest = Record void | Promise; - test_ssr?: (args: { assert: Assert }) => void | Promise; + test_ssr?: (args: { logs: any[]; assert: Assert }) => void | Promise; accessors?: boolean; immutable?: boolean; intro?: boolean; @@ -285,6 +285,7 @@ async function run_test_variant( if (config.test_ssr) { await config.test_ssr({ + logs, // @ts-expect-error assert: { ...assert, diff --git a/packages/svelte/tests/runtime-runes/samples/effect-root-4/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-root-4/_config.js new file mode 100644 index 0000000000..4aac5eed85 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-root-4/_config.js @@ -0,0 +1,18 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: '', + + async test({ assert, target, logs }) { + const btn = target.querySelector('button'); + + btn?.click(); + flushSync(); + + assert.deepEqual(logs, ['effect1', 'effect2']); + }, + test_ssr({ assert, logs }) { + assert.deepEqual(logs, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-root-4/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-root-4/main.svelte new file mode 100644 index 0000000000..2239445619 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-root-4/main.svelte @@ -0,0 +1,10 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/effect-root/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-root/_config.js index fe2a8bb499..cd072d087f 100644 --- a/packages/svelte/tests/runtime-runes/samples/effect-root/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/effect-root/_config.js @@ -24,5 +24,8 @@ export default test({ }); assert.deepEqual(logs, [0, 1, 'cleanup 1', 'cleanup 2']); + }, + test_ssr({ assert, logs }) { + assert.deepEqual(logs, []); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/event-store-no-hoisting/main.svelte b/packages/svelte/tests/runtime-runes/samples/event-store-no-hoisting/main.svelte index b895081916..e827a9bfe9 100644 --- a/packages/svelte/tests/runtime-runes/samples/event-store-no-hoisting/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/event-store-no-hoisting/main.svelte @@ -5,8 +5,7 @@ function setStore() { store = writable(0, () => { - console.log('start'); - return () => console.log('stop'); + return () => {}; }); } From dba4aa3567f305bc739de419ae36c6034da5cb08 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 8 Jul 2024 17:09:10 +0200 Subject: [PATCH 05/16] chore: align warning and error objects, add frame property (#12326) This aligns warning and error objects to contain the same properties and have their toString methods return the same shape. It's implemented by warnings becoming class objects, too, and sharing the same base class with errors. It also adds back the `frame` property that got lost in the Svelte 4->5 transition. The only difference to Svelte 4 now is a slightly adjusted toString property (which is consistent between warnings and errors now) and a `position` property that contains a tuple of start/end offsets instead of a `pos` property only containing the start offset closes #12151 --- .changeset/small-owls-remain.md | 5 + .../templates/compile-errors.js | 45 +------- .../templates/compile-warnings.js | 30 +++-- packages/svelte/src/compiler/errors.js | 40 +------ packages/svelte/src/compiler/state.js | 11 +- packages/svelte/src/compiler/types/index.d.ts | 14 +-- .../src/compiler/utils/compile_diagnostic.js | 105 ++++++++++++++++++ packages/svelte/src/compiler/warnings.js | 31 +++--- packages/svelte/tests/css/test.ts | 2 + packages/svelte/types/index.d.ts | 50 ++++----- 10 files changed, 191 insertions(+), 142 deletions(-) create mode 100644 .changeset/small-owls-remain.md create mode 100644 packages/svelte/src/compiler/utils/compile_diagnostic.js diff --git a/.changeset/small-owls-remain.md b/.changeset/small-owls-remain.md new file mode 100644 index 0000000000..a63ff64935 --- /dev/null +++ b/.changeset/small-owls-remain.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: align warning and error objects, add frame property diff --git a/packages/svelte/scripts/process-messages/templates/compile-errors.js b/packages/svelte/scripts/process-messages/templates/compile-errors.js index e129eef9f6..349e0b38ef 100644 --- a/packages/svelte/scripts/process-messages/templates/compile-errors.js +++ b/packages/svelte/scripts/process-messages/templates/compile-errors.js @@ -1,54 +1,17 @@ -/** @import { Location } from 'locate-character' */ -import * as state from './state.js'; +import { CompileDiagnostic } from './utils/compile_diagnostic.js'; /** @typedef {{ start?: number, end?: number }} NodeLike */ -export class InternalCompileError extends Error { +export class InternalCompileError extends CompileDiagnostic { name = 'CompileError'; - filename = state.filename; - - /** @type {[number, number] | undefined} */ - position = undefined; - - /** @type {Location | undefined} */ - start = undefined; - - /** @type {Location | undefined} */ - end = undefined; - /** - * * @param {string} code * @param {string} message * @param {[number, number] | undefined} position */ constructor(code, message, position) { - super(message); - - this.code = code; - this.position = position; - - if (position) { - this.start = state.locator(position[0]); - this.end = state.locator(position[1]); - } - } - - toString() { - let out = `${this.name}: ${this.message}`; - - out += `\n(${this.code})`; - - if (this.filename) { - out += `\n${this.filename}`; - - if (this.start) { - out += `${this.start.line}:${this.start.column}`; - } - } - - return out; + super(code, message, position); } } @@ -65,7 +28,7 @@ function e(node, code, message) { throw new InternalCompileError( code, message, - start !== undefined && end !== undefined ? [start, end] : undefined + start !== undefined ? [start, end ?? start] : undefined ); } diff --git a/packages/svelte/scripts/process-messages/templates/compile-warnings.js b/packages/svelte/scripts/process-messages/templates/compile-warnings.js index d6a0cd6a0b..c8bd7993c0 100644 --- a/packages/svelte/scripts/process-messages/templates/compile-warnings.js +++ b/packages/svelte/scripts/process-messages/templates/compile-warnings.js @@ -1,7 +1,21 @@ -import { filename, locator, warnings, ignore_stack, ignore_map } from './state.js'; +import { warnings, ignore_stack, ignore_map } from './state.js'; +import { CompileDiagnostic } from './utils/compile_diagnostic.js'; /** @typedef {{ start?: number, end?: number }} NodeLike */ +export class InternalCompileWarning extends CompileDiagnostic { + name = 'CompileWarning'; + + /** + * @param {string} code + * @param {string} message + * @param {[number, number] | undefined} position + */ + constructor(code, message, position) { + super(code, message, position); + } +} + /** * @param {null | NodeLike} node * @param {string} code @@ -14,13 +28,13 @@ function w(node, code, message) { } if (stack && stack.at(-1)?.has(code)) return; - warnings.push({ - code, - message, - filename, - start: node?.start !== undefined ? locator(node.start) : undefined, - end: node?.end !== undefined ? locator(node.end) : undefined - }); + warnings.push( + new InternalCompileWarning( + code, + message, + node && node.start !== undefined ? [node.start, node.end ?? node.start] : undefined + ) + ); } export const codes = CODES; diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index c26586fb1c..7e0b5dd995 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -1,50 +1,18 @@ /* This file is generated by scripts/process-messages/index.js. Do not edit! */ -/** @import { Location } from 'locate-character' */ -import * as state from './state.js'; +import { CompileDiagnostic } from './utils/compile_diagnostic.js'; /** @typedef {{ start?: number, end?: number }} NodeLike */ -export class InternalCompileError extends Error { +export class InternalCompileError extends CompileDiagnostic { name = 'CompileError'; - filename = state.filename; - /** @type {[number, number] | undefined} */ - position = undefined; - /** @type {Location | undefined} */ - start = undefined; - /** @type {Location | undefined} */ - end = undefined; /** - * * @param {string} code * @param {string} message * @param {[number, number] | undefined} position */ constructor(code, message, position) { - super(message); - this.code = code; - this.position = position; - - if (position) { - this.start = state.locator(position[0]); - this.end = state.locator(position[1]); - } - } - - toString() { - let out = `${this.name}: ${this.message}`; - - out += `\n(${this.code})`; - - if (this.filename) { - out += `\n${this.filename}`; - - if (this.start) { - out += `${this.start.line}:${this.start.column}`; - } - } - - return out; + super(code, message, position); } } @@ -58,7 +26,7 @@ function e(node, code, message) { const start = typeof node === 'number' ? node : node?.start; const end = typeof node === 'number' ? node : node?.end; - throw new InternalCompileError(code, message, start !== undefined && end !== undefined ? [start, end] : undefined); + throw new InternalCompileError(code, message, start !== undefined ? [start, end ?? start] : undefined); } /** diff --git a/packages/svelte/src/compiler/state.js b/packages/svelte/src/compiler/state.js index 1e094d95fd..64b9fe591c 100644 --- a/packages/svelte/src/compiler/state.js +++ b/packages/svelte/src/compiler/state.js @@ -14,6 +14,12 @@ export let warnings = []; */ export let filename; +/** + * The original source code + * @type {string} + */ +export let source; + export let locator = getLocator('', { offsetLine: 1 }); /** @@ -43,10 +49,11 @@ export function pop_ignore() { } /** - * @param {string} source + * @param {string} _source * @param {{ filename?: string, rootDir?: string }} options */ -export function reset(source, options) { +export function reset(_source, options) { + source = _source; const root_dir = options.rootDir?.replace(/\\/g, '/'); filename = options.filename?.replace(/\\/g, '/'); diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 129b5da175..9574491756 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -6,13 +6,12 @@ import type { Identifier, ImportDeclaration } from 'estree'; -import type { Location } from 'locate-character'; import type { SourceMap } from 'magic-string'; import type { Context } from 'zimmerframe'; import type { Scope } from '../phases/scope.js'; import type { Css } from './css.js'; import type { EachBlock, Namespace, SvelteNode, SvelteOptions } from './template.js'; -import type { InternalCompileError } from '../errors.js'; +import type { ICompileDiagnostic } from '../utils/compile_diagnostic.js'; /** The return value of `compile` from `svelte/compiler` */ export interface CompileResult { @@ -51,16 +50,9 @@ export interface CompileResult { ast: any; } -export interface Warning { - start?: Location; - end?: Location; - // TODO there was pos: number in Svelte 4 - do we want to add it back? - code: string; - message: string; - filename?: string; -} +export interface Warning extends ICompileDiagnostic {} -export interface CompileError extends InternalCompileError {} +export interface CompileError extends ICompileDiagnostic {} export type CssHashGetter = (args: { name: string; diff --git a/packages/svelte/src/compiler/utils/compile_diagnostic.js b/packages/svelte/src/compiler/utils/compile_diagnostic.js new file mode 100644 index 0000000000..fa36416a06 --- /dev/null +++ b/packages/svelte/src/compiler/utils/compile_diagnostic.js @@ -0,0 +1,105 @@ +/** @import { Location } from 'locate-character' */ +import * as state from '../state.js'; + +const regex_tabs = /^\t+/; + +/** + * @param {string} str + */ +function tabs_to_spaces(str) { + return str.replace(regex_tabs, (match) => match.split('\t').join(' ')); +} + +/** + * @param {string} source + * @param {number} line + * @param {number} column + */ +function get_code_frame(source, line, column) { + const lines = source.split('\n'); + const frame_start = Math.max(0, line - 2); + const frame_end = Math.min(line + 3, lines.length); + const digits = String(frame_end + 1).length; + return lines + .slice(frame_start, frame_end) + .map((str, i) => { + const is_error_line = frame_start + i === line; + const line_num = String(i + frame_start + 1).padStart(digits, ' '); + if (is_error_line) { + const indicator = + ' '.repeat(digits + 2 + tabs_to_spaces(str.slice(0, column)).length) + '^'; + return `${line_num}: ${tabs_to_spaces(str)}\n${indicator}`; + } + return `${line_num}: ${tabs_to_spaces(str)}`; + }) + .join('\n'); +} + +/** + * @typedef {{ + * code: string; + * message: string; + * filename?: string; + * start?: Location; + * end?: Location; + * position?: [number, number]; + * frame?: string; + * }} ICompileDiagnostic */ + +/** @implements {ICompileDiagnostic} */ +export class CompileDiagnostic extends Error { + name = 'CompileDiagnostic'; + + /** + * @param {string} code + * @param {string} message + * @param {[number, number] | undefined} position + */ + constructor(code, message, position) { + super(message); + this.code = code; + + if (state.filename) { + this.filename = state.filename; + } + + if (position) { + this.position = position; + this.start = state.locator(position[0]); + this.end = state.locator(position[1]); + if (this.start && this.end) { + this.frame = get_code_frame(state.source, this.start.line - 1, this.end.column); + } + } + } + + toString() { + let out = `${this.code}: ${this.message}`; + + if (this.filename) { + out += `\n${this.filename}`; + + if (this.start) { + out += `:${this.start.line}:${this.start.column}`; + } + } + + if (this.frame) { + out += `\n${this.frame}`; + } + + return out; + } + + toJSON() { + return { + code: this.code, + message: this.message, + filename: this.filename, + start: this.start, + end: this.end, + position: this.position, + frame: this.frame + }; + } +} diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 7f40eec02b..2518c6c38a 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -1,14 +1,22 @@ /* This file is generated by scripts/process-messages/index.js. Do not edit! */ -import { - filename, - locator, - warnings, - ignore_stack, - ignore_map -} from './state.js'; +import { warnings, ignore_stack, ignore_map } from './state.js'; +import { CompileDiagnostic } from './utils/compile_diagnostic.js'; /** @typedef {{ start?: number, end?: number }} NodeLike */ +export class InternalCompileWarning extends CompileDiagnostic { + name = 'CompileWarning'; + + /** + * @param {string} code + * @param {string} message + * @param {[number, number] | undefined} position + */ + constructor(code, message, position) { + super(code, message, position); + } +} + /** * @param {null | NodeLike} node * @param {string} code @@ -22,14 +30,7 @@ function w(node, code, message) { } if (stack && stack.at(-1)?.has(code)) return; - - warnings.push({ - code, - message, - filename, - start: node?.start !== undefined ? locator(node.start) : undefined, - end: node?.end !== undefined ? locator(node.end) : undefined - }); + warnings.push(new InternalCompileWarning(code, message, node && node.start !== undefined ? [node.start, node.end ?? node.start] : undefined)); } export const codes = [ diff --git a/packages/svelte/tests/css/test.ts b/packages/svelte/tests/css/test.ts index d17f7e9c6b..b4b25fc479 100644 --- a/packages/svelte/tests/css/test.ts +++ b/packages/svelte/tests/css/test.ts @@ -10,6 +10,8 @@ import type { CompileOptions, Warning } from '#compiler'; function normalize_warning(warning: Warning) { delete warning.filename; + delete warning.position; + delete warning.frame; return warning; } diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 305e1240db..775fd7394b 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -562,9 +562,9 @@ declare module 'svelte/animate' { declare module 'svelte/compiler' { import type { AssignmentExpression, ClassDeclaration, Expression, FunctionDeclaration, Identifier, ImportDeclaration, ArrayExpression, MemberExpression, ObjectExpression, Pattern, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, FunctionExpression, Node, Program, ChainExpression, SimpleCallExpression } from 'estree'; - import type { Location } from 'locate-character'; import type { SourceMap } from 'magic-string'; import type { Context } from 'zimmerframe'; + import type { Location } from 'locate-character'; /** * `compile` converts your `.svelte` source code into a JavaScript module that exports a component * @@ -714,16 +714,9 @@ declare module 'svelte/compiler' { ast: any; } - export interface Warning { - start?: Location; - end?: Location; - // TODO there was pos: number in Svelte 4 - do we want to add it back? - code: string; - message: string; - filename?: string; - } + export interface Warning extends ICompileDiagnostic {} - export interface CompileError extends InternalCompileError {} + export interface CompileError extends ICompileDiagnostic {} type CssHashGetter = (args: { name: string; @@ -1882,18 +1875,15 @@ declare module 'svelte/compiler' { content: Program; attributes: Attribute[]; } - class InternalCompileError extends Error { - - constructor(code: string, message: string, position: [number, number] | undefined); - filename: string | undefined; - - position: [number, number] | undefined; - - start: Location | undefined; - - end: Location | undefined; + type ICompileDiagnostic = { code: string; - } + message: string; + filename?: string; + start?: Location; + end?: Location; + position?: [number, number]; + frame?: string; + }; export {}; } @@ -2539,14 +2529,7 @@ declare module 'svelte/types/compiler/interfaces' { export type CompileOptions = CompileOptions_1; /** @deprecated import this from 'svelte' instead */ export type Warning = Warning_1; - interface Warning_1 { - start?: Location; - end?: Location; - // TODO there was pos: number in Svelte 4 - do we want to add it back? - code: string; - message: string; - filename?: string; - } + interface Warning_1 extends ICompileDiagnostic {} type CssHashGetter = (args: { name: string; @@ -2709,6 +2692,15 @@ declare module 'svelte/types/compiler/interfaces' { * (also see https://github.com/sveltejs/svelte/pull/5652) */ type Namespace = 'html' | 'svg' | 'mathml' | 'foreign'; + type ICompileDiagnostic = { + code: string; + message: string; + filename?: string; + start?: Location; + end?: Location; + position?: [number, number]; + frame?: string; + }; export {}; }declare module '*.svelte' { From 76ddfb3d45a7bc53ac406a03d791db18e30657f2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 8 Jul 2024 11:25:16 -0400 Subject: [PATCH 06/16] Version Packages (next) (#12340) Co-authored-by: github-actions[bot] --- .changeset/pre.json | 7 +++++++ packages/svelte/CHANGELOG.md | 18 ++++++++++++++++++ packages/svelte/package.json | 2 +- packages/svelte/src/version.js | 2 +- 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/.changeset/pre.json b/.changeset/pre.json index b16dc711eb..5012336b7a 100644 --- a/.changeset/pre.json +++ b/.changeset/pre.json @@ -78,6 +78,7 @@ "cold-cheetahs-judge", "cold-lamps-accept", "cold-masks-learn", + "cold-teachers-turn", "cool-actors-tan", "cool-ants-leave", "cool-comics-marry", @@ -133,6 +134,7 @@ "eighty-lizards-notice", "eleven-avocados-walk", "eleven-beers-yell", + "eleven-cows-judge", "eleven-cycles-applaud", "eleven-hounds-pump", "empty-bags-heal", @@ -157,6 +159,7 @@ "fast-donkeys-pay", "fast-penguins-matter", "fast-weeks-clean", + "few-cheetahs-taste", "few-clouds-shop", "few-mugs-fail", "few-teachers-know", @@ -398,6 +401,7 @@ "pink-bikes-agree", "pink-goats-promise", "pink-mayflies-tie", + "plenty-clouds-reply", "plenty-elephants-fry", "plenty-starfishes-dress", "plenty-zoos-fix", @@ -517,6 +521,7 @@ "slow-plums-chew", "slow-wombats-reply", "small-apples-eat", + "small-owls-remain", "small-papayas-laugh", "small-sheep-type", "small-spiders-fail", @@ -658,9 +663,11 @@ "wet-wombats-repeat", "wicked-clouds-exercise", "wicked-doors-train", + "wicked-emus-drive", "wicked-hairs-cheer", "wicked-wasps-allow", "wicked-ways-reply", + "wild-cows-chew", "wild-foxes-wonder", "wild-moose-compare", "wise-apples-care", diff --git a/packages/svelte/CHANGELOG.md b/packages/svelte/CHANGELOG.md index f231e1eb45..0220c2da13 100644 --- a/packages/svelte/CHANGELOG.md +++ b/packages/svelte/CHANGELOG.md @@ -1,5 +1,23 @@ # svelte +## 5.0.0-next.176 + +### Patch Changes + +- fix: correct start of `{:else if}` and `{:else}` ([#12043](https://github.com/sveltejs/svelte/pull/12043)) + +- fix: reverse parent/child order in invalid HTML warning ([#12336](https://github.com/sveltejs/svelte/pull/12336)) + +- fix: reorder reactive statements during migration ([#12329](https://github.com/sveltejs/svelte/pull/12329)) + +- feat: better `` SSR output ([#12339](https://github.com/sveltejs/svelte/pull/12339)) + +- chore: align warning and error objects, add frame property ([#12326](https://github.com/sveltejs/svelte/pull/12326)) + +- fix: ensure `$effect.root` is ignored on the server ([#12332](https://github.com/sveltejs/svelte/pull/12332)) + +- fix: enable local transitions on `svelte:element` ([#12346](https://github.com/sveltejs/svelte/pull/12346)) + ## 5.0.0-next.175 ### Patch Changes diff --git a/packages/svelte/package.json b/packages/svelte/package.json index 2df9ed138b..0b42d49209 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.0.0-next.175", + "version": "5.0.0-next.176", "type": "module", "types": "./types/index.d.ts", "engines": { diff --git a/packages/svelte/src/version.js b/packages/svelte/src/version.js index e2ef71d9cf..047944bb4a 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.0.0-next.175'; +export const VERSION = '5.0.0-next.176'; export const PUBLIC_VERSION = '5'; From 14cbb65d852869802b927a19281004a02ae126b8 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 8 Jul 2024 17:28:06 +0200 Subject: [PATCH 07/16] chore: stricter control flow syntax validation in runes mode (#12342) disallow characters between `{` and `#` / `:` / `@` in runes mode closes #11975 --- .changeset/mighty-paws-smash.md | 5 ++ .../messages/compile-errors/template.md | 4 ++ packages/svelte/src/compiler/errors.js | 10 +++ .../src/compiler/phases/1-parse/state/tag.js | 8 ++- .../compiler/phases/2-analyze/validation.js | 61 +++++++++++++++++++ .../if-block-whitespace-legacy/errors.json | 1 + .../if-block-whitespace-legacy/input.svelte | 8 +++ .../if-block-whitespace-runes/errors.json | 14 +++++ .../if-block-whitespace-runes/input.svelte | 8 +++ 9 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 .changeset/mighty-paws-smash.md create mode 100644 packages/svelte/tests/validator/samples/if-block-whitespace-legacy/errors.json create mode 100644 packages/svelte/tests/validator/samples/if-block-whitespace-legacy/input.svelte create mode 100644 packages/svelte/tests/validator/samples/if-block-whitespace-runes/errors.json create mode 100644 packages/svelte/tests/validator/samples/if-block-whitespace-runes/input.svelte diff --git a/.changeset/mighty-paws-smash.md b/.changeset/mighty-paws-smash.md new file mode 100644 index 0000000000..56f707e10e --- /dev/null +++ b/.changeset/mighty-paws-smash.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: stricter control flow syntax validation in runes mode diff --git a/packages/svelte/messages/compile-errors/template.md b/packages/svelte/messages/compile-errors/template.md index 2bb4197a1d..517e5e730f 100644 --- a/packages/svelte/messages/compile-errors/template.md +++ b/packages/svelte/messages/compile-errors/template.md @@ -88,6 +88,10 @@ > Block was left open +## block_unexpected_character + +> Expected a `%character%` character immediately following the opening bracket + ## block_unexpected_close > Unexpected block closing tag diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index 7e0b5dd995..b092e262db 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -730,6 +730,16 @@ export function block_unclosed(node) { e(node, "block_unclosed", "Block was left open"); } +/** + * Expected a `%character%` character immediately following the opening bracket + * @param {null | number | NodeLike} node + * @param {string} character + * @returns {never} + */ +export function block_unexpected_character(node, character) { + e(node, "block_unexpected_character", `Expected a \`${character}\` character immediately following the opening bracket`); +} + /** * Unexpected block closing tag * @param {null | number | NodeLike} node diff --git a/packages/svelte/src/compiler/phases/1-parse/state/tag.js b/packages/svelte/src/compiler/phases/1-parse/state/tag.js index 14af03f6f6..f5beb7b79a 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/tag.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/tag.js @@ -39,7 +39,8 @@ export default function tag(parser) { /** @param {import('../index.js').Parser} parser */ function open(parser) { - const start = parser.index - 2; + let start = parser.index - 2; + while (parser.template[start] !== '{') start -= 1; if (parser.eat('if')) { parser.require_whitespace(); @@ -343,9 +344,12 @@ function next(parser) { parser.allow_whitespace(); parser.eat('}', true); + let elseif_start = start - 1; + while (parser.template[elseif_start] !== '{') elseif_start -= 1; + /** @type {ReturnType>} */ const child = parser.append({ - start: start - 1, + start: elseif_start, end: -1, type: 'IfBlock', elseif: true, diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index b2097e60ce..882b0a0e46 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -1090,6 +1090,20 @@ function validate_no_const_assignment(node, argument, scope, is_binding) { } } +/** + * Validates that the opening of a control flow block is `{` immediately followed by the expected character. + * In legacy mode whitespace is allowed inbetween. TODO remove once legacy mode is gone and move this into parser instead. + * @param {{start: number; end: number}} node + * @param {import('./types.js').AnalysisState} state + * @param {string} expected + */ +function validate_opening_tag(node, state, expected) { + if (state.analysis.source[node.start + 1] !== expected) { + // avoid a sea of red and only mark the first few characters + e.block_unexpected_character({ start: node.start, end: node.start + 5 }, expected); + } +} + /** * @param {import('estree').AssignmentExpression | import('estree').UpdateExpression} node * @param {import('estree').Pattern | import('estree').Expression} argument @@ -1217,6 +1231,8 @@ export const validation_runes = merge(validation, a11y_validators, { validate_call_expression(node, state.scope, path); }, EachBlock(node, { next, state }) { + validate_opening_tag(node, state, '#'); + const context = node.context; if ( context.type === 'Identifier' && @@ -1226,6 +1242,51 @@ export const validation_runes = merge(validation, a11y_validators, { } next({ ...state }); }, + IfBlock(node, { state, path }) { + const parent = path.at(-1); + const expected = + path.at(-2)?.type === 'IfBlock' && parent?.type === 'Fragment' && parent.nodes.length === 1 + ? ':' + : '#'; + validate_opening_tag(node, state, expected); + }, + AwaitBlock(node, { state }) { + validate_opening_tag(node, state, '#'); + + if (node.value) { + const start = /** @type {number} */ (node.value.start); + const match = state.analysis.source.substring(start - 10, start).match(/{(\s*):then\s+$/); + if (match && match[1] !== '') { + e.block_unexpected_character({ start: start - 10, end: start }, ':'); + } + } + + if (node.error) { + const start = /** @type {number} */ (node.error.start); + const match = state.analysis.source.substring(start - 10, start).match(/{(\s*):catch\s+$/); + if (match && match[1] !== '') { + e.block_unexpected_character({ start: start - 10, end: start }, ':'); + } + } + }, + KeyBlock(node, { state }) { + validate_opening_tag(node, state, '#'); + }, + SnippetBlock(node, { state }) { + validate_opening_tag(node, state, '#'); + }, + ConstTag(node, { state }) { + validate_opening_tag(node, state, '@'); + }, + HtmlTag(node, { state }) { + validate_opening_tag(node, state, '@'); + }, + DebugTag(node, { state }) { + validate_opening_tag(node, state, '@'); + }, + RenderTag(node, { state }) { + validate_opening_tag(node, state, '@'); + }, VariableDeclarator(node, { state }) { ensure_no_module_import_conflict(node, state); diff --git a/packages/svelte/tests/validator/samples/if-block-whitespace-legacy/errors.json b/packages/svelte/tests/validator/samples/if-block-whitespace-legacy/errors.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/packages/svelte/tests/validator/samples/if-block-whitespace-legacy/errors.json @@ -0,0 +1 @@ +[] diff --git a/packages/svelte/tests/validator/samples/if-block-whitespace-legacy/input.svelte b/packages/svelte/tests/validator/samples/if-block-whitespace-legacy/input.svelte new file mode 100644 index 0000000000..8640f21e1a --- /dev/null +++ b/packages/svelte/tests/validator/samples/if-block-whitespace-legacy/input.svelte @@ -0,0 +1,8 @@ + + + +
+ { #if true} +

hi

+ {/if} +
diff --git a/packages/svelte/tests/validator/samples/if-block-whitespace-runes/errors.json b/packages/svelte/tests/validator/samples/if-block-whitespace-runes/errors.json new file mode 100644 index 0000000000..83ff498556 --- /dev/null +++ b/packages/svelte/tests/validator/samples/if-block-whitespace-runes/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "block_unexpected_character", + "message": "Expected a `#` character immediately following the opening bracket", + "start": { + "line": 5, + "column": 1 + }, + "end": { + "line": 5, + "column": 6 + } + } +] diff --git a/packages/svelte/tests/validator/samples/if-block-whitespace-runes/input.svelte b/packages/svelte/tests/validator/samples/if-block-whitespace-runes/input.svelte new file mode 100644 index 0000000000..39b11dd5e1 --- /dev/null +++ b/packages/svelte/tests/validator/samples/if-block-whitespace-runes/input.svelte @@ -0,0 +1,8 @@ + + + +
+ { #if true} +

hi

+ {/if} +
From e8c3729fc94ae948983368a50c4745fc04627496 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 8 Jul 2024 18:27:31 +0200 Subject: [PATCH 08/16] fix: make `$state` component exports settable (#12345) * fix: make `$state` component exports settable fixes #11983 * failing test * fix --------- Co-authored-by: Rich Harris --- .changeset/tidy-lizards-happen.md | 5 +++ .../src/compiler/phases/2-analyze/index.js | 3 ++ .../3-transform/client/transform-client.js | 32 ++++++++++++++++--- .../samples/exports-3/_config.js | 5 ++- .../samples/exports-3/main.svelte | 2 +- .../samples/exports-4/_config.js | 12 +++++++ .../samples/exports-4/main.svelte | 7 ++++ .../samples/exports-4/sub.svelte | 9 ++++++ 8 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 .changeset/tidy-lizards-happen.md create mode 100644 packages/svelte/tests/runtime-runes/samples/exports-4/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/exports-4/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/exports-4/sub.svelte diff --git a/.changeset/tidy-lizards-happen.md b/.changeset/tidy-lizards-happen.md new file mode 100644 index 0000000000..1b280937a0 --- /dev/null +++ b/.changeset/tidy-lizards-happen.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: make `$state` component exports settable diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 4a15f0e46f..aab31869cd 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -1010,6 +1010,9 @@ const runes_scope_tweaker = { name: node.local.name, alias: node.exported.name }); + + const binding = state.scope.get(node.local.name); + if (binding) binding.reassigned = true; }, ExportNamedDeclaration(node, { next, state }) { if (!node.declaration || state.ast_type !== 'instance') { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index d0dd2315a5..a8438c0aca 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -198,14 +198,38 @@ export function client_component(source, analysis, options) { } /** @type {Array} */ - const component_returned_object = analysis.exports.map(({ name, alias }) => { + const component_returned_object = analysis.exports.flatMap(({ name, alias }) => { + const binding = instance_state.scope.get(name); const expression = serialize_get_binding(b.id(name), instance_state); + const getter = b.get(alias ?? name, [b.return(expression)]); + + if (expression.type === 'Identifier') { + if (binding?.declaration_kind === 'let' || binding?.declaration_kind === 'var') { + return [ + getter, + b.set(alias ?? name, [b.stmt(b.assignment('=', expression, b.id('$$value')))]) + ]; + } else if (!options.dev) { + return b.init(alias ?? name, expression); + } + } - if (expression.type === 'Identifier' && !options.dev) { - return b.init(alias ?? name, expression); + if (binding?.kind === 'state' || binding?.kind === 'frozen_state') { + return [ + getter, + b.set(alias ?? name, [ + b.stmt( + b.call( + '$.set', + b.id(name), + b.call(binding.kind === 'state' ? '$.proxy' : '$.freeze', b.id('$$value')) + ) + ) + ]) + ]; } - return b.get(alias ?? name, [b.return(expression)]); + return getter; }); const properties = [...analysis.instance.scope.declarations].filter( diff --git a/packages/svelte/tests/runtime-runes/samples/exports-3/_config.js b/packages/svelte/tests/runtime-runes/samples/exports-3/_config.js index cf22671d9f..807fed2aa6 100644 --- a/packages/svelte/tests/runtime-runes/samples/exports-3/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/exports-3/_config.js @@ -4,10 +4,9 @@ import { test } from '../../test'; export default test({ test({ assert, target }) { assert.htmlEqual(target.innerHTML, `0 0 `); - const [btn] = target.querySelectorAll('button'); + const btn = target.querySelector('button'); - btn?.click(); - flushSync(); + flushSync(() => btn?.click()); assert.htmlEqual(target.innerHTML, '1 2 '); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/exports-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/exports-3/main.svelte index b1cabb399f..a68a40f300 100644 --- a/packages/svelte/tests/runtime-runes/samples/exports-3/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/exports-3/main.svelte @@ -1,5 +1,5 @@ diff --git a/packages/svelte/tests/runtime-runes/samples/exports-4/_config.js b/packages/svelte/tests/runtime-runes/samples/exports-4/_config.js new file mode 100644 index 0000000000..807fed2aa6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/exports-4/_config.js @@ -0,0 +1,12 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + test({ assert, target }) { + assert.htmlEqual(target.innerHTML, `0 0 `); + const btn = target.querySelector('button'); + + flushSync(() => btn?.click()); + assert.htmlEqual(target.innerHTML, '1 2 '); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/exports-4/main.svelte b/packages/svelte/tests/runtime-runes/samples/exports-4/main.svelte new file mode 100644 index 0000000000..85adcffbe3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/exports-4/main.svelte @@ -0,0 +1,7 @@ + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/exports-4/sub.svelte b/packages/svelte/tests/runtime-runes/samples/exports-4/sub.svelte new file mode 100644 index 0000000000..75b0091235 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/exports-4/sub.svelte @@ -0,0 +1,9 @@ + + +{count} +{doubled} From ba93e5fce31c7b608430b86ed3b29c8b45815cf1 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 8 Jul 2024 19:27:48 +0200 Subject: [PATCH 09/16] breaking: play transitions on `mount` by default (#12351) * breaking: play transitions on `mount` by default closes #11280 * only prevent transitions when the component is invalidated --------- Co-authored-by: Rich Harris --- .changeset/hip-garlics-tap.md | 5 +++++ .../svelte/src/internal/client/dev/hmr.js | 13 ++++++++---- packages/svelte/src/internal/client/render.js | 10 +++++---- packages/svelte/src/legacy/legacy-client.js | 2 +- .../mount-intro-transition/Component.svelte | 5 +++++ .../samples/mount-intro-transition/_config.js | 21 +++++++++++++++++++ .../mount-intro-transition/main.svelte | 21 +++++++++++++++++++ packages/svelte/types/index.d.ts | 3 ++- .../03-appendix/02-breaking-changes.md | 4 ++++ 9 files changed, 74 insertions(+), 10 deletions(-) create mode 100644 .changeset/hip-garlics-tap.md create mode 100644 packages/svelte/tests/runtime-runes/samples/mount-intro-transition/Component.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/mount-intro-transition/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/mount-intro-transition/main.svelte diff --git a/.changeset/hip-garlics-tap.md b/.changeset/hip-garlics-tap.md new file mode 100644 index 0000000000..0913e45d87 --- /dev/null +++ b/.changeset/hip-garlics-tap.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +breaking: play transitions on `mount` by default diff --git a/packages/svelte/src/internal/client/dev/hmr.js b/packages/svelte/src/internal/client/dev/hmr.js index 20f205ade8..0d0edd4ddc 100644 --- a/packages/svelte/src/internal/client/dev/hmr.js +++ b/packages/svelte/src/internal/client/dev/hmr.js @@ -1,9 +1,7 @@ /** @import { Source, Effect } from '#client' */ -import { empty } from '../dom/operations.js'; import { block, branch, destroy_effect } from '../reactivity/effects.js'; import { set_should_intro } from '../render.js'; import { get } from '../runtime.js'; -import { check_target } from './legacy.js'; /** * @template {(anchor: Comment, props: any) => any} Component @@ -20,6 +18,8 @@ export function hmr(source) { /** @type {Effect} */ let effect; + let ran = false; + block(() => { const component = get(source); @@ -30,7 +30,9 @@ export function hmr(source) { } effect = branch(() => { - set_should_intro(false); + // when the component is invalidated, replace it without transitions + if (ran) set_should_intro(false); + // preserve getters/setters Object.defineProperties( instance, @@ -39,10 +41,13 @@ export function hmr(source) { new.target ? new component(anchor, props) : component(anchor, props) ) ); - set_should_intro(true); + + if (ran) set_should_intro(true); }); }); + ran = true; + return instance; }; } diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index e01f8657c7..5820b227e1 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -24,8 +24,8 @@ export const root_event_handles = new Set(); /** * This is normally true — block effects should run their intro transitions — - * but is false during hydration and mounting (unless `options.intro` is `true`) - * and when creating the children of a `` that just changed tag + * but is false during hydration (unless `options.intro` is `true`) and + * when creating the children of a `` that just changed tag */ export let should_intro = true; @@ -66,7 +66,8 @@ export function slot(anchor, slot_fn, slot_props, fallback_fn) { } /** - * Mounts a component to the given target and returns the exports and potentially the props (if compiled with `accessors: true`) of the component + * Mounts a component to the given target and returns the exports and potentially the props (if compiled with `accessors: true`) of the component. + * Transitions will play during the initial render unless the `intro` option is set to `false`. * * @template {Record} Props * @template {Record} Exports @@ -126,6 +127,7 @@ export function hydrate(component, options) { validate_component(component); } + options.intro = options.intro ?? false; const target = options.target; const previous_hydrate_nodes = hydrate_nodes; @@ -190,7 +192,7 @@ export function hydrate(component, options) { * }} options * @returns {Exports} */ -function _mount(Component, { target, anchor, props = {}, events, context, intro = false }) { +function _mount(Component, { target, anchor, props = {}, events, context, intro = true }) { init_operations(); const registered_events = new Set(); diff --git a/packages/svelte/src/legacy/legacy-client.js b/packages/svelte/src/legacy/legacy-client.js index d2eba5be9c..b5f499f37d 100644 --- a/packages/svelte/src/legacy/legacy-client.js +++ b/packages/svelte/src/legacy/legacy-client.js @@ -77,7 +77,7 @@ class Svelte4Component { target: options.target, props, context: options.context, - intro: options.intro, + intro: options.intro ?? false, recover: options.recover }); diff --git a/packages/svelte/tests/runtime-runes/samples/mount-intro-transition/Component.svelte b/packages/svelte/tests/runtime-runes/samples/mount-intro-transition/Component.svelte new file mode 100644 index 0000000000..f9c91fb840 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/mount-intro-transition/Component.svelte @@ -0,0 +1,5 @@ + + +
DIV
diff --git a/packages/svelte/tests/runtime-runes/samples/mount-intro-transition/_config.js b/packages/svelte/tests/runtime-runes/samples/mount-intro-transition/_config.js new file mode 100644 index 0000000000..114f6f0cbf --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/mount-intro-transition/_config.js @@ -0,0 +1,21 @@ +import { flushSync } from 'svelte'; +import { ok, test } from '../../test'; + +export default test({ + async test({ assert, target, raf }) { + const [btn1, btn2] = target.querySelectorAll('button'); + const div = target.querySelector('div'); + ok(div); + + btn1.click(); + flushSync(); + assert.htmlEqual(div.innerHTML, `
DIV
`); + + raf.tick(100); + assert.htmlEqual(div.innerHTML, `
DIV
`); + + btn2.click(); + flushSync(); + assert.htmlEqual(div.innerHTML, `
DIV
`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/mount-intro-transition/main.svelte b/packages/svelte/tests/runtime-runes/samples/mount-intro-transition/main.svelte new file mode 100644 index 0000000000..83021fa537 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/mount-intro-transition/main.svelte @@ -0,0 +1,21 @@ + + +
+ + + diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 775fd7394b..c63e6366ba 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -366,7 +366,8 @@ declare module 'svelte' { /** Anything except a function */ type NotFunction = T extends Function ? never : T; /** - * Mounts a component to the given target and returns the exports and potentially the props (if compiled with `accessors: true`) of the component + * Mounts a component to the given target and returns the exports and potentially the props (if compiled with `accessors: true`) of the component. + * Transitions will play during the initial render unless the `intro` option is set to `false`. * * */ export function mount, Exports extends Record>(component: ComponentType> | Component, options: {} extends Props ? { diff --git a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md index 5b08103d80..8de2bfbfe4 100644 --- a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md +++ b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md @@ -281,3 +281,7 @@ In Svelte 4, `` is valid code. This makes little sens ``` Note that whereas Svelte 4 would treat `` (for example) identically to `` for the purposes of determining which `bind:` directives could be applied, Svelte 5 does not. + +### `mount` plays transitions by default + +The `mount` function used to render a component tree plays transitions by default unless the `intro` option is set to `false`. This is different from legacy class components which, when manually instantiated, didn't play transitions by default. From 63f3ee4ffd3131030ee82979f78aba24153c90fe Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 8 Jul 2024 21:06:57 +0200 Subject: [PATCH 10/16] fix: make `` `