From 965c12f633453cbb77d0e0941570c7aa5ee7de77 Mon Sep 17 00:00:00 2001 From: Paolo Ricciuti Date: Fri, 21 Jun 2024 22:34:25 +0200 Subject: [PATCH] fix: allow multiple optional parameters with defaults (#12070) * fix: allow multiple optional parameters with defaults * Apply suggestions from code review * partial fix * feat: parse as a whole function * couple of fixes * work around acorn-typescript quirks * add the harder test * Update .changeset/ten-geese-share.md --------- Co-authored-by: Rich Harris Co-authored-by: Rich Harris --- .changeset/ten-geese-share.md | 5 ++ .../src/compiler/phases/1-parse/acorn.js | 11 +++++ .../src/compiler/phases/1-parse/state/tag.js | 47 +++++++------------ .../samples/snippets/output.json | 18 +++++-- .../typescript-in-event-handler/output.json | 5 +- .../_config.js | 5 ++ .../main.svelte | 9 ++++ 7 files changed, 65 insertions(+), 35 deletions(-) create mode 100644 .changeset/ten-geese-share.md create mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-optional-arguments-defaults/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-optional-arguments-defaults/main.svelte diff --git a/.changeset/ten-geese-share.md b/.changeset/ten-geese-share.md new file mode 100644 index 0000000000..4b924c9958 --- /dev/null +++ b/.changeset/ten-geese-share.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: allow multiple optional parameters with defaults in snippets diff --git a/packages/svelte/src/compiler/phases/1-parse/acorn.js b/packages/svelte/src/compiler/phases/1-parse/acorn.js index 5749cb0d6f..0da21fe3ce 100644 --- a/packages/svelte/src/compiler/phases/1-parse/acorn.js +++ b/packages/svelte/src/compiler/phases/1-parse/acorn.js @@ -1,6 +1,7 @@ import * as acorn from 'acorn'; import { walk } from 'zimmerframe'; import { tsPlugin } from 'acorn-typescript'; +import { locator } from '../../state.js'; const ParserWithTS = acorn.Parser.extend(tsPlugin({ allowSatisfies: true })); @@ -127,6 +128,16 @@ function amend(source, node) { // @ts-expect-error delete node.loc.end.index; + if (typeof node.loc?.end === 'number') { + const loc = locator(node.loc.end); + if (loc) { + node.loc.end = { + line: loc.line, + column: loc.column + }; + } + } + if (/** @type {any} */ (node).typeAnnotation && node.end === undefined) { // i think there might be a bug in acorn-typescript that prevents // `end` from being assigned when there's a type annotation 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 e113469421..85fde1d486 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/tag.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/tag.js @@ -3,6 +3,7 @@ import read_expression from '../read/expression.js'; import * as e from '../../../errors.js'; import { create_fragment } from '../utils/create.js'; import { walk } from 'zimmerframe'; +import { parse_expression_at } from '../acorn.js'; const regex_whitespace_with_closing_curly_brace = /^\s*}/; @@ -268,39 +269,28 @@ function open(parser) { e.expected_identifier(parser.index); } - parser.eat('(', true); - - parser.allow_whitespace(); - - /** @type {import('estree').Pattern[]} */ - const parameters = []; + let slice_end = parser.index; - while (!parser.match(')')) { - let pattern = read_pattern(parser, true); - - parser.allow_whitespace(); - if (parser.eat('=')) { - parser.allow_whitespace(); - const right = read_expression(parser); - pattern = { - type: 'AssignmentPattern', - left: pattern, - right: right, - start: pattern.start, - end: right.end - }; - } + parser.eat('(', true); - parameters.push(pattern); + let parentheses = 1; + let params = ''; - if (!parser.eat(',')) break; - parser.allow_whitespace(); + while (!parser.match(')') || parentheses !== 1) { + if (parser.match('(')) parentheses++; + if (parser.match(')')) parentheses--; + params += parser.read(/^./); } - parser.eat(')', true); + let function_expression = /** @type {import('estree').ArrowFunctionExpression} */ ( + parse_expression_at( + parser.template.slice(0, slice_end).replace(/\S/g, ' ') + `(${params}) => {}`, + parser.ts, + 0 + ) + ); - parser.allow_whitespace(); - parser.eat('}', true); + parser.index += 2; /** @type {ReturnType>} */ const block = parser.append({ @@ -313,10 +303,9 @@ function open(parser) { end: name_end, name }, - parameters, + parameters: function_expression.params, body: create_fragment() }); - parser.stack.push(block); parser.fragments.push(block.body); diff --git a/packages/svelte/tests/parser-modern/samples/snippets/output.json b/packages/svelte/tests/parser-modern/samples/snippets/output.json index ac9aa58067..67d30db1b0 100644 --- a/packages/svelte/tests/parser-modern/samples/snippets/output.json +++ b/packages/svelte/tests/parser-modern/samples/snippets/output.json @@ -32,20 +32,28 @@ "loc": { "start": { "line": 3, - "column": 14, - "character": 43 + "column": 14 }, "end": { "line": 3, - "column": 25, - "character": 54 + "column": 25 } }, - "end": 54, + "end": 46, "typeAnnotation": { "type": "TSTypeAnnotation", "start": 46, "end": 54, + "loc": { + "start": { + "line": 3, + "column": 17 + }, + "end": { + "line": 3, + "column": 25 + } + }, "typeAnnotation": { "type": "TSStringKeyword", "start": 48, diff --git a/packages/svelte/tests/parser-modern/samples/typescript-in-event-handler/output.json b/packages/svelte/tests/parser-modern/samples/typescript-in-event-handler/output.json index 6f219c7311..c3442092a8 100644 --- a/packages/svelte/tests/parser-modern/samples/typescript-in-event-handler/output.json +++ b/packages/svelte/tests/parser-modern/samples/typescript-in-event-handler/output.json @@ -54,7 +54,10 @@ "line": 6, "column": 12 }, - "end": 87 + "end": { + "line": 6, + "column": 25 + } }, "name": "e", "typeAnnotation": { diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-optional-arguments-defaults/_config.js b/packages/svelte/tests/runtime-runes/samples/snippet-optional-arguments-defaults/_config.js new file mode 100644 index 0000000000..e651cb2eb6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/snippet-optional-arguments-defaults/_config.js @@ -0,0 +1,5 @@ +import { test } from '../../test'; + +export default test({ + html: '013/023' +}); diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-optional-arguments-defaults/main.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-optional-arguments-defaults/main.svelte new file mode 100644 index 0000000000..235381d57c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/snippet-optional-arguments-defaults/main.svelte @@ -0,0 +1,9 @@ +{#snippet one(a, b = 1, c = (2, 3))} + {a}{b}{c} +{/snippet} + +{#snippet two(a, b = (1, 2), c = 3)} + {a}{b}{c} +{/snippet} + +{@render one(0)}/{@render two(0)}