Skip to content

feat: Variadic snippets #9988

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 88 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
88 commits
Select commit Hold shift + click to select a range
7690371
give this another try
elliott-with-the-longest-name-on-github Dec 23, 2023
14805f9
fix: lint
elliott-with-the-longest-name-on-github Dec 23, 2023
3756ce5
fix: Forgot to save
elliott-with-the-longest-name-on-github Dec 23, 2023
243ba9c
feat: it works boiiii
elliott-with-the-longest-name-on-github Dec 23, 2023
cb4daea
look, ok, it did work, i just needed to update the snapshots
elliott-with-the-longest-name-on-github Dec 23, 2023
8382eb0
bruh
elliott-with-the-longest-name-on-github Dec 23, 2023
5bab2db
changeset
elliott-with-the-longest-name-on-github Dec 23, 2023
5beaf36
feat: ok I think the client snippet block finally works
elliott-with-the-longest-name-on-github Dec 24, 2023
33a39bf
feat: current tests pass; I'm sure I'm missing stuff for new things
elliott-with-the-longest-name-on-github Dec 24, 2023
c1bb239
fix: snapshot
elliott-with-the-longest-name-on-github Dec 24, 2023
3a726a9
feat: I think non-destructured rest should work now?
elliott-with-the-longest-name-on-github Dec 26, 2023
8555681
chore: duplicated computation
elliott-with-the-longest-name-on-github Dec 26, 2023
6d364aa
feat: Tests (passing and failing
elliott-with-the-longest-name-on-github Dec 26, 2023
3da243f
feat: it's... alive?
elliott-with-the-longest-name-on-github Dec 26, 2023
49893ec
chore: Clean up my messes
elliott-with-the-longest-name-on-github Dec 26, 2023
398bb67
chore: devtime stuff
elliott-with-the-longest-name-on-github Dec 26, 2023
bb63c85
fix: fmt
elliott-with-the-longest-name-on-github Dec 26, 2023
c0d62c4
chore: see if this fixes repl
elliott-with-the-longest-name-on-github Dec 27, 2023
42e7bf4
chore: make naming more offensive
elliott-with-the-longest-name-on-github Dec 27, 2023
959abf6
Merge branch 'main' into elliott/variadic-snippets
elliott-with-the-longest-name-on-github Dec 28, 2023
83218b1
fix: Don't throw on missing keys, return undefined as it usually would
elliott-with-the-longest-name-on-github Dec 28, 2023
381fd7b
Update packages/svelte/src/compiler/phases/1-parse/state/tag.js
elliott-with-the-longest-name-on-github Jan 2, 2024
36601ef
Update packages/svelte/src/compiler/phases/1-parse/state/tag.js
elliott-with-the-longest-name-on-github Jan 2, 2024
c4509b2
Merge remote-tracking branch 'origin' into elliott/variadic-snippets
elliott-with-the-longest-name-on-github Jan 5, 2024
cfee435
fix: Hopefully default param values now work
elliott-with-the-longest-name-on-github Jan 5, 2024
7158321
Merge branch 'elliott/variadic-snippets' of github.com:sveltejs/svelt…
elliott-with-the-longest-name-on-github Jan 5, 2024
e6ddbba
dumb
elliott-with-the-longest-name-on-github Jan 5, 2024
99ed291
types
elliott-with-the-longest-name-on-github Jan 5, 2024
bc917fe
feat: Test it
elliott-with-the-longest-name-on-github Jan 5, 2024
1065145
fix: Turns out javascript parameters are optional
elliott-with-the-longest-name-on-github Jan 5, 2024
d7dac4e
feat: The Final Solution
elliott-with-the-longest-name-on-github Jan 6, 2024
79536e4
document function
elliott-with-the-longest-name-on-github Jan 6, 2024
1dc662a
feat: Better bracket matching, unit tests
elliott-with-the-longest-name-on-github Jan 9, 2024
31b78f0
feat: exclude test files from publish
elliott-with-the-longest-name-on-github Jan 9, 2024
a7be93c
feat: More unit tests
elliott-with-the-longest-name-on-github Jan 9, 2024
eb26b84
feat: Use more efficient parsing for @const
elliott-with-the-longest-name-on-github Jan 9, 2024
486370e
Merge remote-tracking branch 'origin' into elliott/variadic-snippets
elliott-with-the-longest-name-on-github Jan 9, 2024
88b06c7
Update .changeset/curvy-cups-cough.md
elliott-with-the-longest-name-on-github Jan 9, 2024
83973ed
Update packages/svelte/package.json
elliott-with-the-longest-name-on-github Jan 9, 2024
55a57b4
Update packages/svelte/src/compiler/phases/1-parse/utils/bracket.js
elliott-with-the-longest-name-on-github Jan 9, 2024
6e97647
fix: changesets
elliott-with-the-longest-name-on-github Jan 9, 2024
330a3c0
chore: additional comments
elliott-with-the-longest-name-on-github Jan 9, 2024
9a688cc
fix: kill foreach
elliott-with-the-longest-name-on-github Jan 9, 2024
3b6f5b8
fix: foreach again
elliott-with-the-longest-name-on-github Jan 9, 2024
33e5886
Merge remote-tracking branch 'origin' into elliott/variadic-snippets
elliott-with-the-longest-name-on-github Jan 9, 2024
4c5ecd2
feat: Docs
elliott-with-the-longest-name-on-github Jan 9, 2024
82694ac
Revert "fix: kill foreach"
elliott-with-the-longest-name-on-github Jan 9, 2024
11086f3
fix: My own stupidity
elliott-with-the-longest-name-on-github Jan 9, 2024
f9ac0cf
fix: style
elliott-with-the-longest-name-on-github Jan 9, 2024
4630670
fix - maybe
elliott-with-the-longest-name-on-github Jan 10, 2024
2584931
Update sites/svelte-5-preview/src/routes/docs/content/01-api/03-snipp…
dummdidumm Jan 10, 2024
a6a7a74
Update tag.js
elliott-with-the-longest-name-on-github Jan 11, 2024
7b32431
Update .changeset/curvy-cups-cough.md
elliott-with-the-longest-name-on-github Jan 11, 2024
37b6fd4
chore: Remove rest params
elliott-with-the-longest-name-on-github Jan 11, 2024
a802c92
Merge branch 'elliott/variadic-snippets' of github.com:sveltejs/svelt…
elliott-with-the-longest-name-on-github Jan 11, 2024
63b0374
Delete .changeset/eighty-rivers-wash.md
dummdidumm Jan 12, 2024
39d6ed0
fix: Honestly idk why it was broken but it's fixed now
elliott-with-the-longest-name-on-github Jan 15, 2024
304e36c
Merge branch 'elliott/variadic-snippets' of github.com:sveltejs/svelt…
elliott-with-the-longest-name-on-github Jan 15, 2024
348b312
Merge branch 'main' into elliott/variadic-snippets
elliott-with-the-longest-name-on-github Jan 15, 2024
d1f4ed9
fix: var name lol
elliott-with-the-longest-name-on-github Jan 15, 2024
979170c
Merge branch 'elliott/variadic-snippets' of github.com:sveltejs/svelt…
elliott-with-the-longest-name-on-github Jan 15, 2024
3f9a246
fix: typegen
elliott-with-the-longest-name-on-github Jan 15, 2024
4c84313
fix: idk
elliott-with-the-longest-name-on-github Jan 15, 2024
ab851d5
fix: It looks like a bunch of unformatted shit came in through main??…
elliott-with-the-longest-name-on-github Jan 15, 2024
e0104f0
Revert "fix: It looks like a bunch of unformatted shit came in throug…
elliott-with-the-longest-name-on-github Jan 15, 2024
5104475
fix: format again
elliott-with-the-longest-name-on-github Jan 15, 2024
80b52a7
this is getting ridiculous
elliott-with-the-longest-name-on-github Jan 15, 2024
7c8564b
Merge remote-tracking branch 'origin' into elliott/variadic-snippets
elliott-with-the-longest-name-on-github Jan 15, 2024
d9803f1
Update tag.js
elliott-with-the-longest-name-on-github Jan 26, 2024
f05699d
merge main
Rich-Harris Jan 29, 2024
6f7a21c
Merge branch 'main' into pr/9988
Rich-Harris Jan 30, 2024
a74e2c5
fix errors
Rich-Harris Jan 30, 2024
0cd01f4
simplify a bit
Rich-Harris Jan 30, 2024
8a6d7e4
use read_context
Rich-Harris Jan 30, 2024
dd5926f
use read_context for const as well
Rich-Harris Jan 30, 2024
d34f6ef
remove unused code
Rich-Harris Jan 30, 2024
75576b9
unused import
Rich-Harris Jan 30, 2024
0fb39db
unused export
Rich-Harris Jan 30, 2024
0b015a2
remove spread args. sorry elliott
Rich-Harris Jan 30, 2024
569bc1f
tidy up SnippetBlock interface
Rich-Harris Jan 30, 2024
e55d8d1
fix test
Rich-Harris Jan 31, 2024
1390466
simplify
Rich-Harris Jan 31, 2024
9f25d8c
tweak
Rich-Harris Jan 31, 2024
d258b82
revert example, so that it matches the surrounding text
Rich-Harris Jan 31, 2024
3198e20
move PropsWithChildren back to public.d.ts
Rich-Harris Jan 31, 2024
0107dc3
update typing docs, so that it flows from previous example
Rich-Harris Jan 31, 2024
069ad1d
temporarily revert const parsing changes, to get prettier working aga…
Rich-Harris Jan 31, 2024
84c6eda
oops
Rich-Harris Jan 31, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/curvy-cups-cough.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

breaking: snippets can now take multiple arguments, support default parameters. Because of this, the type signature has changed
2 changes: 1 addition & 1 deletion packages/svelte/elements.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export type ToggleEventHandler<T extends EventTarget> = EventHandler<ToggleEvent
export interface DOMAttributes<T extends EventTarget> {
// Implicit children prop every element has
// Add this here so that libraries doing `$props<HTMLButtonAttributes>()` don't need a separate interface
children?: import('svelte').Snippet<void>;
children?: import('svelte').Snippet;

// Clipboard Events
'on:copy'?: ClipboardEventHandler<T> | undefined | null;
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
},
"files": [
"src",
"!src/**/*.test.*",
"types",
"compiler.cjs",
"*.d.ts",
Expand Down
5 changes: 4 additions & 1 deletion packages/svelte/src/compiler/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ const parse = {
'duplicate-script-element': () =>
`A component can have a single top-level <script> element and/or a single top-level <script context="module"> element`,
'invalid-render-expression': () => 'expected an identifier followed by (...)',
'invalid-render-arguments': () => 'expected at most one argument'
'invalid-render-arguments': () => 'expected at most one argument',
'invalid-render-spread-argument': () => 'cannot use spread arguments in {@render ...} tags',
'invalid-snippet-rest-parameter': () =>
'snippets do not support rest parameters; use an array instead'
};

/** @satisfies {Errors} */
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/compiler/legacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ export function convert(source, ast) {
start: node.start,
end: node.end,
expression: node.expression,
context: node.context,
parameters: node.parameters,
children: node.body.nodes.map((child) => visit(child))
};
},
Expand Down
46 changes: 31 additions & 15 deletions packages/svelte/src/compiler/phases/1-parse/read/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,24 +98,40 @@ export default function read_context(parser) {
* @returns {any}
*/
function read_type_annotation(parser) {
const index = parser.index;
const start = parser.index;
parser.allow_whitespace();

if (parser.eat(':')) {
// we need to trick Acorn into parsing the type annotation
const insert = '_ as ';
let a = parser.index - insert.length;
const template = ' '.repeat(a) + insert + parser.template.slice(parser.index);
let expression = parse_expression_at(template, parser.ts, a);
if (!parser.eat(':')) {
parser.index = start;
return undefined;
}

// `array as item: string, index` becomes `string, index`, which is mistaken as a sequence expression - fix that
if (expression.type === 'SequenceExpression') {
expression = expression.expressions[0];
}
// we need to trick Acorn into parsing the type annotation
const insert = '_ as ';
let a = parser.index - insert.length;
const template =
parser.template.slice(0, a).replace(/[^\n]/g, ' ') +
insert +
parser.template.slice(parser.index);
let expression = parse_expression_at(template, parser.ts, a);

parser.index = /** @type {number} */ (expression.end);
return /** @type {any} */ (expression).typeAnnotation;
} else {
parser.index = index;
// `foo: bar = baz` gets mangled — fix it
if (expression.type === 'AssignmentExpression') {
let b = expression.right.start;
while (template[b] !== '=') b -= 1;
expression = parse_expression_at(template.slice(0, b), parser.ts, a);
}

// `array as item: string, index` becomes `string, index`, which is mistaken as a sequence expression - fix that
if (expression.type === 'SequenceExpression') {
expression = expression.expressions[0];
}

parser.index = /** @type {number} */ (expression.end);
return {
type: 'TSTypeAnnotation',
start,
end: parser.index,
typeAnnotation: /** @type {any} */ (expression).typeAnnotation
};
}
31 changes: 23 additions & 8 deletions packages/svelte/src/compiler/phases/1-parse/state/tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,28 @@ function open(parser) {

parser.allow_whitespace();

const context = parser.match(')') ? null : read_context(parser);
/** @type {import('estree').Pattern[]} */
const parameters = [];

while (!parser.match(')')) {
let pattern = read_context(parser);

parser.allow_whitespace();
if (parser.eat('=')) {
parser.allow_whitespace();
pattern = {
type: 'AssignmentPattern',
left: pattern,
right: read_expression(parser)
};
}

parameters.push(pattern);

if (!parser.eat(',')) break;
parser.allow_whitespace();
}

parser.allow_whitespace();
parser.eat(')', true);

parser.allow_whitespace();
Expand All @@ -294,7 +313,7 @@ function open(parser) {
end: name_end,
name
},
context,
parameters,
body: create_fragment()
})
);
Expand Down Expand Up @@ -589,10 +608,6 @@ function special(parser) {
error(expression, 'invalid-render-expression');
}

if (expression.arguments.length > 1) {
error(expression.arguments[1], 'invalid-render-arguments');
}

parser.allow_whitespace();
parser.eat('}', true);

Expand All @@ -602,7 +617,7 @@ function special(parser) {
start,
end: parser.index,
expression: expression.callee,
argument: expression.arguments[0] ?? null
arguments: expression.arguments
})
);
}
Expand Down
7 changes: 7 additions & 0 deletions packages/svelte/src/compiler/phases/2-analyze/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,13 @@ const validation = {
parent_element: node.name
});
},
RenderTag(node, context) {
for (const arg of node.arguments) {
if (arg.type === 'SpreadElement') {
error(arg, 'invalid-render-spread-argument');
}
}
},
SvelteHead(node) {
const attribute = node.attributes[0];
if (attribute) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1498,13 +1498,7 @@ function process_children(nodes, parent, { visit, state }) {
);
const assignment = serialize_template_literal(sequence, visit, state)[1];
const init = b.stmt(b.assignment('=', b.member(text_id, b.id('nodeValue')), assignment));
const singular = b.stmt(
b.call(
'$.text_effect',
text_id,
b.thunk(serialize_template_literal(sequence, visit, state)[1])
)
);
const singular = b.stmt(b.call('$.text_effect', text_id, b.thunk(assignment)));

if (contains_call_expression && !within_bound_contenteditable) {
state.update_effects.push(singular);
Expand Down Expand Up @@ -1776,8 +1770,8 @@ export const template_visitors = {

/** @type {import('estree').Expression[]} */
const args = [context.state.node];
if (node.argument) {
args.push(b.thunk(/** @type {import('estree').Expression} */ (context.visit(node.argument))));
for (const arg of node.arguments) {
args.push(b.thunk(/** @type {import('estree').Expression} */ (context.visit(arg))));
}

let snippet_function = /** @type {import('estree').Expression} */ (
Expand Down Expand Up @@ -2472,61 +2466,67 @@ export const template_visitors = {
},
SnippetBlock(node, context) {
// TODO hoist where possible
/** @type {import('estree').Pattern[]} */
const args = [b.id('$$anchor')];

/** @type {import('estree').BlockStatement} */
let body;

if (node.context) {
const id = node.context.type === 'Identifier' ? node.context : b.id('$$context');
args.push(id);
/** @type {import('estree').Statement[]} */
const declarations = [];

for (let i = 0; i < node.parameters.length; i++) {
const argument = node.parameters[i];

/** @type {import('estree').Statement[]} */
const declarations = [];
if (!argument) continue;

// some of this is duplicated with EachBlock — TODO dedupe?
if (node.context.type === 'Identifier') {
if (argument.type === 'Identifier') {
args.push({
type: 'AssignmentPattern',
left: argument,
right: b.id('$.noop')
});
const binding = /** @type {import('#compiler').Binding} */ (
context.state.scope.get(id.name)
context.state.scope.get(argument.name)
);
binding.expression = b.call(id);
} else {
const paths = extract_paths(node.context);
binding.expression = b.call(argument);
continue;
}

for (const path of paths) {
const name = /** @type {import('estree').Identifier} */ (path.node).name;
const binding = /** @type {import('#compiler').Binding} */ (
context.state.scope.get(name)
);
declarations.push(
b.let(
path.node,
b.thunk(
/** @type {import('estree').Expression} */ (
context.visit(path.expression?.(b.call('$$context')))
)
let arg_alias = `$$arg${i}`;
args.push(b.id(arg_alias));

const paths = extract_paths(argument);

for (const path of paths) {
const name = /** @type {import('estree').Identifier} */ (path.node).name;
const binding = /** @type {import('#compiler').Binding} */ (context.state.scope.get(name));
declarations.push(
b.let(
path.node,
b.thunk(
/** @type {import('estree').Expression} */ (
context.visit(path.expression?.(b.maybe_call(b.id(arg_alias))))
)
)
);

// we need to eagerly evaluate the expression in order to hit any
// 'Cannot access x before initialization' errors
if (context.state.options.dev) {
declarations.push(b.stmt(b.call(name)));
}
)
);

binding.expression = b.call(name);
// we need to eagerly evaluate the expression in order to hit any
// 'Cannot access x before initialization' errors
if (context.state.options.dev) {
declarations.push(b.stmt(b.call(name)));
}
}

body = b.block([
...declarations,
.../** @type {import('estree').BlockStatement} */ (context.visit(node.body)).body
]);
} else {
body = /** @type {import('estree').BlockStatement} */ (context.visit(node.body));
binding.expression = b.call(name);
}
}

body = b.block([
...declarations,
.../** @type {import('estree').BlockStatement} */ (context.visit(node.body)).body
]);

const path = context.path;
// If we're top-level, then we can create a function for the snippet so that it can be referenced
// in the props declaration (default value pattern).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1134,21 +1134,14 @@ const template_visitors = {
const snippet_function = state.options.dev
? b.call('$.validate_snippet', expression)
: expression;
if (node.argument) {
state.template.push(
t_statement(
b.stmt(
b.call(
snippet_function,
b.id('$$payload'),
/** @type {import('estree').Expression} */ (context.visit(node.argument))
)
)
)
);
} else {
state.template.push(t_statement(b.stmt(b.call(snippet_function, b.id('$$payload')))));
}

const snippet_args = node.arguments.map((arg) => {
return /** @type {import('estree').Expression} */ (context.visit(arg));
});

state.template.push(
t_statement(b.stmt(b.call(snippet_function, b.id('$$payload'), ...snippet_args)))
);

state.template.push(t_expression(anchor_id));
},
Expand Down Expand Up @@ -1451,19 +1444,14 @@ const template_visitors = {
},
SnippetBlock(node, context) {
// TODO hoist where possible
/** @type {import('estree').Pattern[]} */
const args = [b.id('$$payload')];
if (node.context) {
args.push(node.context);
}

context.state.init.push(
b.function_declaration(
node.expression,
args,
[b.id('$$payload'), ...node.parameters],
/** @type {import('estree').BlockStatement} */ (context.visit(node.body))
)
);

if (context.state.options.dev) {
context.state.init.push(b.stmt(b.call('$.add_snippet_symbol', node.expression)));
}
Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/compiler/phases/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -592,8 +592,8 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
const child_scope = state.scope.child();
scopes.set(node, child_scope);

if (node.context) {
for (const id of extract_identifiers(node.context)) {
for (const param of node.parameters) {
for (const id of extract_identifiers(param)) {
child_scope.declare(id, 'each', 'let');
}
}
Expand Down
Loading