From ea29bad2fc2aab08602f62a25373610012399b4b Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Sat, 12 Apr 2025 15:25:52 +0200 Subject: [PATCH 1/4] fix: correctly validate `undefined` snippet params with default value --- .changeset/angry-mayflies-matter.md | 5 +++++ .../3-transform/client/visitors/SnippetBlock.js | 11 ++++++++++- packages/svelte/src/internal/client/dev/validation.js | 8 +++++--- .../validate-undefined-snippet-default-arg/_config.js | 5 +++++ .../main.svelte | 5 +++++ 5 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 .changeset/angry-mayflies-matter.md create mode 100644 packages/svelte/tests/runtime-runes/samples/validate-undefined-snippet-default-arg/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/validate-undefined-snippet-default-arg/main.svelte diff --git a/.changeset/angry-mayflies-matter.md b/.changeset/angry-mayflies-matter.md new file mode 100644 index 000000000000..289fbf87b67a --- /dev/null +++ b/.changeset/angry-mayflies-matter.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: correctly validate `undefined` snippet params with default value diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js index 7eb043aa5d11..01d24310d4d7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js @@ -24,6 +24,8 @@ export function SnippetBlock(node, context) { const transform = { ...context.state.transform }; const child_state = { ...context.state, transform }; + let fallback_arr = []; + for (let i = 0; i < node.parameters.length; i++) { const argument = node.parameters[i]; @@ -49,6 +51,9 @@ export function SnippetBlock(node, context) { for (const path of paths) { const name = /** @type {Identifier} */ (path.node).name; const needs_derived = path.has_default_value; // to ensure that default value is only called once + if (needs_derived) { + fallback_arr.push(i); + } const fn = b.thunk( /** @type {Expression} */ (context.visit(path.expression?.(b.maybe_call(b.id(arg_alias))))) ); @@ -67,12 +72,16 @@ export function SnippetBlock(node, context) { } } if (dev) { + const [anchor, ...validations_args] = args; declarations.unshift( b.stmt( b.call( '$.validate_snippet_args', .../** @type {Identifier[]} */ ( - args.map((arg) => (arg?.type === 'Identifier' ? arg : arg?.left)) + [anchor, b.array(fallback_arr.map((i) => b.literal(i))), ...validations_args].map( + (arg) => + arg?.type === 'Identifier' || arg.type === 'ArrayExpression' ? arg : arg?.left + ) ) ) ) diff --git a/packages/svelte/src/internal/client/dev/validation.js b/packages/svelte/src/internal/client/dev/validation.js index e41e4c46283d..3c1039f13f41 100644 --- a/packages/svelte/src/internal/client/dev/validation.js +++ b/packages/svelte/src/internal/client/dev/validation.js @@ -1,14 +1,16 @@ import { invalid_snippet_arguments } from '../../shared/errors.js'; /** * @param {Node} anchor + * @param {number[]} with_fallback_idx * @param {...(()=>any)[]} args */ -export function validate_snippet_args(anchor, ...args) { +export function validate_snippet_args(anchor, with_fallback_idx, ...args) { if (typeof anchor !== 'object' || !(anchor instanceof Node)) { invalid_snippet_arguments(); } - for (let arg of args) { - if (typeof arg !== 'function') { + for (let i = 0; i < args.length; i++) { + const arg = args[i]; + if (typeof arg !== 'function' && !(with_fallback_idx.includes(i) && arg === undefined)) { invalid_snippet_arguments(); } } diff --git a/packages/svelte/tests/runtime-runes/samples/validate-undefined-snippet-default-arg/_config.js b/packages/svelte/tests/runtime-runes/samples/validate-undefined-snippet-default-arg/_config.js new file mode 100644 index 000000000000..bddb75e67785 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/validate-undefined-snippet-default-arg/_config.js @@ -0,0 +1,5 @@ +import { test } from '../../test'; + +export default test({ + html: `

default

` +}); diff --git a/packages/svelte/tests/runtime-runes/samples/validate-undefined-snippet-default-arg/main.svelte b/packages/svelte/tests/runtime-runes/samples/validate-undefined-snippet-default-arg/main.svelte new file mode 100644 index 000000000000..3f00eba46bc2 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/validate-undefined-snippet-default-arg/main.svelte @@ -0,0 +1,5 @@ +{#snippet test(param = "default")} +

{param}

+{/snippet} + +{@render test()} \ No newline at end of file From 1fed4dd12668428095002343e290a9c406c3c087 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 10:32:03 -0400 Subject: [PATCH 2/4] use arguments --- .../client/visitors/SnippetBlock.js | 30 +++++-------------- .../src/internal/client/dev/validation.js | 8 ++--- 2 files changed, 11 insertions(+), 27 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js index 01d24310d4d7..c1b067d64496 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js @@ -21,6 +21,10 @@ export function SnippetBlock(node, context) { /** @type {Statement[]} */ const declarations = []; + if (dev) { + declarations.push(b.stmt(b.call('$.validate_snippet_args', b.spread(b.id('arguments'))))); + } + const transform = { ...context.state.transform }; const child_state = { ...context.state, transform }; @@ -71,33 +75,15 @@ export function SnippetBlock(node, context) { } } } - if (dev) { - const [anchor, ...validations_args] = args; - declarations.unshift( - b.stmt( - b.call( - '$.validate_snippet_args', - .../** @type {Identifier[]} */ ( - [anchor, b.array(fallback_arr.map((i) => b.literal(i))), ...validations_args].map( - (arg) => - arg?.type === 'Identifier' || arg.type === 'ArrayExpression' ? arg : arg?.left - ) - ) - ) - ) - ); - } body = b.block([ ...declarations, .../** @type {BlockStatement} */ (context.visit(node.body, child_state)).body ]); - /** @type {Expression} */ - let snippet = b.arrow(args, body); - - if (dev) { - snippet = b.call('$.wrap_snippet', b.id(context.state.analysis.name), snippet); - } + // in dev we use a FunctionExpression (not arrow function) so we can use `arguments` + let snippet = dev + ? b.call('$.wrap_snippet', b.id(context.state.analysis.name), b.function(null, args, body)) + : b.arrow(args, body); const declaration = b.const(node.expression, snippet); diff --git a/packages/svelte/src/internal/client/dev/validation.js b/packages/svelte/src/internal/client/dev/validation.js index 3c1039f13f41..e41e4c46283d 100644 --- a/packages/svelte/src/internal/client/dev/validation.js +++ b/packages/svelte/src/internal/client/dev/validation.js @@ -1,16 +1,14 @@ import { invalid_snippet_arguments } from '../../shared/errors.js'; /** * @param {Node} anchor - * @param {number[]} with_fallback_idx * @param {...(()=>any)[]} args */ -export function validate_snippet_args(anchor, with_fallback_idx, ...args) { +export function validate_snippet_args(anchor, ...args) { if (typeof anchor !== 'object' || !(anchor instanceof Node)) { invalid_snippet_arguments(); } - for (let i = 0; i < args.length; i++) { - const arg = args[i]; - if (typeof arg !== 'function' && !(with_fallback_idx.includes(i) && arg === undefined)) { + for (let arg of args) { + if (typeof arg !== 'function') { invalid_snippet_arguments(); } } From 57a59e25fd09d28abfc04b08ad67813a116d3e09 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 10:33:13 -0400 Subject: [PATCH 3/4] unused --- .../phases/3-transform/client/visitors/SnippetBlock.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js index c1b067d64496..69cfb8a29de6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js @@ -28,8 +28,6 @@ export function SnippetBlock(node, context) { const transform = { ...context.state.transform }; const child_state = { ...context.state, transform }; - let fallback_arr = []; - for (let i = 0; i < node.parameters.length; i++) { const argument = node.parameters[i]; @@ -55,9 +53,6 @@ export function SnippetBlock(node, context) { for (const path of paths) { const name = /** @type {Identifier} */ (path.node).name; const needs_derived = path.has_default_value; // to ensure that default value is only called once - if (needs_derived) { - fallback_arr.push(i); - } const fn = b.thunk( /** @type {Expression} */ (context.visit(path.expression?.(b.maybe_call(b.id(arg_alias))))) ); @@ -75,6 +70,7 @@ export function SnippetBlock(node, context) { } } } + body = b.block([ ...declarations, .../** @type {BlockStatement} */ (context.visit(node.body, child_state)).body From da3686476d3615e33cf7067654202b1384608435 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Apr 2025 10:35:07 -0400 Subject: [PATCH 4/4] drive-by --- .../phases/3-transform/client/visitors/SnippetBlock.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js index 69cfb8a29de6..f28f8c8a596c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js @@ -34,12 +34,7 @@ export function SnippetBlock(node, context) { if (!argument) continue; if (argument.type === 'Identifier') { - args.push({ - type: 'AssignmentPattern', - left: argument, - right: b.id('$.noop') - }); - + args.push(b.assignment_pattern(argument, b.id('$.noop'))); transform[argument.name] = { read: b.call }; continue;