From 37b6fd48a25aa5bcaca3160a8d1f39741d389e5f Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Thu, 11 Jan 2024 15:12:26 -0700 Subject: [PATCH] chore: Remove rest params --- .../src/compiler/phases/1-parse/state/tag.js | 8 +++ .../3-transform/client/visitors/template.js | 28 +--------- .../svelte/src/internal/client/runtime.js | 26 --------- .../src/internal/client/runtime.test.ts | 24 +------- packages/svelte/src/internal/index.js | 1 - packages/svelte/src/main/public.d.ts | 22 +++++--- .../_config.js | 21 ------- .../main.svelte | 15 ----- .../samples/snippet-argument-rest/_config.js | 21 ------- .../samples/snippet-argument-rest/main.svelte | 15 ----- .../samples/snippet-spread-args/main.svelte | 4 +- packages/svelte/types/index.d.ts | 56 ++++++++++++------- .../routes/docs/content/01-api/03-snippets.md | 7 +-- 13 files changed, 66 insertions(+), 182 deletions(-) delete mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-argument-rest-destructured/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-argument-rest-destructured/main.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-argument-rest/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-argument-rest/main.svelte 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 e5163aad7d..b32e15c7f6 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/tag.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/tag.js @@ -298,6 +298,14 @@ function open(parser) { error(snippet_expression, 'TODO', 'expected a snippet name'); } + if (snippet_expression.params.at(-1)?.type === 'RestElement') { + error( + snippet_expression, + 'TODO', + 'snippets do not support rest parameters; use an array instead' + ); + } + // slice the `{#` off the beginning since it's already been eaten parser.eat(raw_snippet_declaration.slice(2), true); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index cda8d0d3c8..ee7f8ec369 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -2501,25 +2501,9 @@ export const template_visitors = { } let arg_alias = `$$arg${i}`; + args.push(b.id(arg_alias)); - if (argument.type === 'RestElement') { - args.push(b.rest(b.id(arg_alias))); - - if (argument.argument.type === 'Identifier') { - declarations.push( - b.let(argument.argument.name, b.call('$.proxy_rest_array', b.id(arg_alias))) - ); - continue; - } - - const new_arg_alias = `$$proxied_arg${i}`; - declarations.push(b.let(new_arg_alias, b.call('$.proxy_rest_array', b.id(arg_alias)))); - arg_alias = new_arg_alias; - } else { - args.push(b.id(arg_alias)); - } - - const paths = extract_paths(argument.type === 'RestElement' ? argument.argument : argument); + const paths = extract_paths(argument); for (const path of paths) { const name = /** @type {import('estree').Identifier} */ (path.node).name; @@ -2529,13 +2513,7 @@ export const template_visitors = { path.node, b.thunk( /** @type {import('estree').Expression} */ ( - context.visit( - path.expression?.( - argument.type === 'RestElement' - ? b.id(arg_alias) - : b.call('$.maybe_call', b.id(arg_alias)) - ) - ) + context.visit(path.expression?.(b.call('$.maybe_call', b.id(arg_alias)))) ) ) ) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 075c5a41fa..2329d29e69 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -2028,32 +2028,6 @@ export function thunkspread(iterable) { return thunks; } -/** - * This is meant to proxy the `...rest` parameter to a snippet function. Basically, - * this array will be full of functions that need to be invoked to unwrap their value. - * We have no way of forcing that invocation in all circumstances -- for example, if - * a user passes the rest array to a function that then accesses it via `rest[0]`, we - * would need to transform that into `rest[0]()`. That's effectively what this proxy does. - * - * @template {unknown[]} T - * @param {T} items - * @returns {T} - */ -export function proxy_rest_array(items) { - return new Proxy(items, { - get(target, property) { - // @ts-expect-error -- It thinks arrays can't have properties that aren't numeric - if (typeof property === 'symbol') return target[property]; - if (!isNaN(parseInt(property))) { - // @ts-expect-error -- It thinks arrays can't have properties that aren't numeric - return target[property]?.(); - } - // @ts-expect-error -- It thinks arrays can't have properties that aren't numeric - return target[property]; - } - }); -} - /** * @template {Function | undefined} T * @param {T} fn diff --git a/packages/svelte/src/internal/client/runtime.test.ts b/packages/svelte/src/internal/client/runtime.test.ts index ecd19102cc..302d4ffb98 100644 --- a/packages/svelte/src/internal/client/runtime.test.ts +++ b/packages/svelte/src/internal/client/runtime.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { proxy_rest_array, thunkspread } from '..'; +import { thunkspread } from '..'; describe('thunkspread', () => { it('makes all of its arguments callable', () => { @@ -19,25 +19,3 @@ describe('thunkspread', () => { expect(thunks.map((thunk) => thunk())).toEqual([...items()]); }); }); - -describe('proxy_rest_array', () => { - it('calls its items on access', () => { - const items = [() => 1, () => 2, () => 3]; - const proxied_items = proxy_rest_array(items); - expect(proxied_items[1]).toBe(2); - }); - - it('returns undefined for keys with no item', () => { - const items = [() => 1, () => 2, () => 3]; - const proxied_items = proxy_rest_array(items); - expect(proxied_items[4]).toBe(undefined); - }); - - it('works with array methods', () => { - const items = [() => 1, () => 2, () => 3]; - const proxied_items = proxy_rest_array(items); - expect(proxied_items.map((item) => item)).toEqual([1, 2, 3]); - // @ts-expect-error - This is a weird case for sure - expect(proxied_items.find((item) => item === 1)).toBe(1); - }); -}); diff --git a/packages/svelte/src/internal/index.js b/packages/svelte/src/internal/index.js index a5f789ee71..306bd88e8a 100644 --- a/packages/svelte/src/internal/index.js +++ b/packages/svelte/src/internal/index.js @@ -37,7 +37,6 @@ export { user_root_effect, inspect, unwrap, - proxy_rest_array, thunkspread, maybe_call, freeze diff --git a/packages/svelte/src/main/public.d.ts b/packages/svelte/src/main/public.d.ts index 75415ab3bc..dfb4f6b38a 100644 --- a/packages/svelte/src/main/public.d.ts +++ b/packages/svelte/src/main/public.d.ts @@ -195,14 +195,20 @@ declare const SnippetReturn: unique symbol; * ``` * You can only call a snippet through the `{@render ...}` tag. */ -export interface Snippet { - ( - this: void, - ...args: T - ): typeof SnippetReturn & { - _: 'functions passed to {@render ...} tags must use the `Snippet` type imported from "svelte"'; - }; -} +export type Snippet = + // this conditional allows tuples but not arrays. Arrays would indicate a + // rest parameter type, which is not supported. If rest parameters are added + // in the future, the condition can be removed. + number extends T['length'] + ? never + : { + ( + this: void, + ...args: T + ): typeof SnippetReturn & { + _: 'functions passed to {@render ...} tags must use the `Snippet` type imported from "svelte"'; + }; + }; interface DispatchOptions { cancelable?: boolean; diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest-destructured/_config.js b/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest-destructured/_config.js deleted file mode 100644 index 6c0f9cbb31..0000000000 --- a/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest-destructured/_config.js +++ /dev/null @@ -1,21 +0,0 @@ -import { test } from '../../test'; - -export default test({ - html: ` -

clicks: 0, doubled: 0, tripled: 0

- - `, - - async test({ assert, target }) { - const btn = target.querySelector('button'); - - await btn?.click(); - assert.htmlEqual( - target.innerHTML, - ` -

clicks: 1, doubled: 2, tripled: 3

- - ` - ); - } -}); diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest-destructured/main.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest-destructured/main.svelte deleted file mode 100644 index 263f399a05..0000000000 --- a/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest-destructured/main.svelte +++ /dev/null @@ -1,15 +0,0 @@ - - -{#snippet foo(n: number, ...[doubled, { tripled }]: number[])} -

clicks: {n}, doubled: {doubled}, tripled: {tripled}

-{/snippet} - -{@render foo(count, doubled, {tripled})} - - diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest/_config.js b/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest/_config.js deleted file mode 100644 index 6c0f9cbb31..0000000000 --- a/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest/_config.js +++ /dev/null @@ -1,21 +0,0 @@ -import { test } from '../../test'; - -export default test({ - html: ` -

clicks: 0, doubled: 0, tripled: 0

- - `, - - async test({ assert, target }) { - const btn = target.querySelector('button'); - - await btn?.click(); - assert.htmlEqual( - target.innerHTML, - ` -

clicks: 1, doubled: 2, tripled: 3

- - ` - ); - } -}); diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest/main.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest/main.svelte deleted file mode 100644 index 05ac668a2e..0000000000 --- a/packages/svelte/tests/runtime-runes/samples/snippet-argument-rest/main.svelte +++ /dev/null @@ -1,15 +0,0 @@ - - -{#snippet foo(n: number, ...rest: number[])} -

clicks: {n}, doubled: {rest[0]}, tripled: {rest[1]}

-{/snippet} - -{@render foo(count, doubled, tripled)} - - diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-spread-args/main.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-spread-args/main.svelte index b2f2ae13e2..06c6942c6c 100644 --- a/packages/svelte/tests/runtime-runes/samples/snippet-spread-args/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/snippet-spread-args/main.svelte @@ -27,8 +27,8 @@ let whatever_comes_after_that = derivedBox(count, 5); -{#snippet foo(n: number, ...[doubled, { tripled }, ...rest]: number[])} -

clicks: {n.value}, doubled: {doubled.value}, tripled: {tripled.value}, quadrupled: {rest[0].value}, something else: {rest[1].value}

+{#snippet foo(n, doubled, { tripled }, quadrupled, whatever)} +

clicks: {n.value}, doubled: {doubled.value}, tripled: {tripled.value}, quadrupled: {quadrupled.value}, something else: {whatever.value}

{/snippet} {@render foo(...[count, doubled, {tripled}, quadrupled, whatever_comes_after_that])} diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index fcd4b24dbe..98461ce731 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -196,14 +196,20 @@ declare module 'svelte' { * ``` * You can only call a snippet through the `{@render ...}` tag. */ - export interface Snippet { - ( - this: void, - ...args: T - ): typeof SnippetReturn & { - _: 'functions passed to {@render ...} tags must use the `Snippet` type imported from "svelte"'; - }; - } + export type Snippet = + // this conditional allows tuples but not arrays. Arrays would indicate a + // rest parameter type, which is not supported. If rest parameters are added + // in the future, the condition can be removed. + number extends T['length'] + ? never + : { + ( + this: void, + ...args: T + ): typeof SnippetReturn & { + _: 'functions passed to {@render ...} tags must use the `Snippet` type imported from "svelte"'; + }; + }; interface DispatchOptions { cancelable?: boolean; @@ -324,7 +330,9 @@ declare module 'svelte' { new (options: ComponentConstructorOptions | undefined; + children?: ((this: void) => unique symbol & { + _: "functions passed to {@render ...} tags must use the `Snippet` type imported from \"svelte\""; + }) | undefined; })>): SvelteComponent; }, options: { target: Node; @@ -347,7 +355,9 @@ declare module 'svelte' { new (options: ComponentConstructorOptions | undefined; + children?: ((this: void) => unique symbol & { + _: "functions passed to {@render ...} tags must use the `Snippet` type imported from \"svelte\""; + }) | undefined; })>): SvelteComponent; }, options: { target: Node; @@ -1725,7 +1735,9 @@ declare module 'svelte/legacy' { } ? {} : Slots extends { default: any; } ? { - children?: Snippet<[]> | undefined; + children?: ((this: void) => unique symbol & { + _: "functions passed to {@render ...} tags must use the `Snippet` type imported from \"svelte\""; + }) | undefined; } : {})>): SvelteComponent; } & Exports; // This should contain all the public interfaces (not all of them are actually importable, check current Svelte for which ones are). @@ -1851,14 +1863,20 @@ declare module 'svelte/legacy' { * ``` * You can only call a snippet through the `{@render ...}` tag. */ - interface Snippet { - ( - this: void, - ...args: T - ): typeof SnippetReturn & { - _: 'functions passed to {@render ...} tags must use the `Snippet` type imported from "svelte"'; - }; - } + type Snippet = + // this conditional allows tuples but not arrays. Arrays would indicate a + // rest parameter type, which is not supported. If rest parameters are added + // in the future, the condition can be removed. + number extends T['length'] + ? never + : { + ( + this: void, + ...args: T + ): typeof SnippetReturn & { + _: 'functions passed to {@render ...} tags must use the `Snippet` type imported from "svelte"'; + }; + }; } declare module 'svelte/motion' { diff --git a/sites/svelte-5-preview/src/routes/docs/content/01-api/03-snippets.md b/sites/svelte-5-preview/src/routes/docs/content/01-api/03-snippets.md index c9291f6273..6cc64c8977 100644 --- a/sites/svelte-5-preview/src/routes/docs/content/01-api/03-snippets.md +++ b/sites/svelte-5-preview/src/routes/docs/content/01-api/03-snippets.md @@ -58,7 +58,7 @@ Snippets, and _render tags_, are a way to create reusable chunks of markup insid {/each} ``` -A snippet behaves pretty much like a regular function declaration: It can have multiple parameters, those parameters can be destructured, and they can have default values. You can also use `...rest` params. ([demo](/#H4sIAAAAAAAAE2WO0YrCQAxFfyXGhVoo9L3Wot9hF6xt1IHpTJikggzz78tI2YX1MTecc2_Em7Ek2JwjumEmbPDEjBXqi_MhT7JKWKH4JYw5aWUMhrXrXa-WFCLMJDLcCQ5wMVoIOK8gHu6BBgX1IETw8ssFEhzgi4Nn2ZX73rX1n8rFrTjDTAoPstbv8pjqQ_3hLFPe0XL3piBmLG0grmDatDV3vYv1ak_vrmMgN1FYq4rBmpGKrPr_ufpr8buiTFjh7CdzMzRho2Gh9J1-AFhYxBNCAQAA)): +A snippet behaves pretty much like a regular function declaration: It can have multiple parameters, those parameters can be destructured, and they can have default values. However, you cannot use `...rest` params. ([demo](/#H4sIAAAAAAAAE2WO0YrCQAxFfyXGhVoo9L3Wot9hF6xt1IHpTJikggzz78tI2YX1MTecc2_Em7Ek2JwjumEmbPDEjBXqi_MhT7JKWKH4JYw5aWUMhrXrXa-WFCLMJDLcCQ5wMVoIOK8gHu6BBgX1IETw8ssFEhzgi4Nn2ZX73rX1n8rFrTjDTAoPstbv8pjqQ_3hLFPe0XL3piBmLG0grmDatDV3vYv1ak_vrmMgN1FYq4rBmpGKrPr_ufpr8buiTFjh7CdzMzRho2Gh9J1-AFhYxBNCAQAA)): ```svelte {#snippet figure({ src, caption, width, height })} @@ -244,7 +244,6 @@ type SnippetWithOneArg = Snippet<[argOne: number]>; type SnippetWithMultipleArgs = Snippet< [argOne: number, argTwo: string] >; -type SnippetWithAnyNumberOfArgs = Snippet; ``` And here are the snippet declarations matching those cases (note: this example uses TypeScript): @@ -261,8 +260,4 @@ And here are the snippet declarations matching those cases (note: this example u {#snippet withMultipleArgs(argOne: number, argTwo: string)} {/snippet} - -{#snippet withAnyNumberOfArgs(...rest: number[])} - -{/snippet} ```