From 3a032887d5bf0f39551bee4a6c761fedd0cda0b7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 19 Jul 2024 09:00:04 -0400 Subject: [PATCH 01/14] use types rather than runtime validation to prevent incorrect snippet/component usage --- .../svelte/messages/shared-errors/errors.md | 8 ---- .../3-transform/server/transform-server.js | 12 +---- packages/svelte/src/index.d.ts | 15 ++++-- .../src/internal/client/dom/blocks/snippet.js | 46 +++++++++---------- .../src/internal/server/blocks/snippet.js | 7 ++- packages/svelte/src/internal/server/index.js | 1 - .../svelte/src/internal/shared/validate.js | 22 +-------- .../samples/mount-snippet-error/_config.js | 12 ----- .../samples/mount-snippet-error/main.svelte | 14 ------ .../snippet-validation-error-1/_config.js | 8 ---- .../snippet-validation-error-1/main.svelte | 7 --- .../snippet-validation-error-2/_config.js | 8 ---- .../snippet-validation-error-2/main.svelte | 5 -- 13 files changed, 38 insertions(+), 127 deletions(-) delete mode 100644 packages/svelte/tests/runtime-runes/samples/mount-snippet-error/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/mount-snippet-error/main.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-validation-error-1/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-validation-error-1/main.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-validation-error-2/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-validation-error-2/main.svelte diff --git a/packages/svelte/messages/shared-errors/errors.md b/packages/svelte/messages/shared-errors/errors.md index d693b35e05c6..1d17b0f6d47d 100644 --- a/packages/svelte/messages/shared-errors/errors.md +++ b/packages/svelte/messages/shared-errors/errors.md @@ -2,14 +2,6 @@ > `%name%(...)` can only be used during component initialisation -## render_tag_invalid_argument - -> The argument to `{@render ...}` must be a snippet function, not a component or a slot with a `let:` directive or some other kind of function. If you want to dynamically render one snippet or another, use `$derived` and pass its result to `{@render ...}` - -## snippet_used_as_component - -> A snippet must be rendered with `{@render ...}` - ## store_invalid_shape > `%name%` is not a store with a `subscribe` method 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 04115a850e5c..0f5a1f25222d 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 @@ -969,13 +969,7 @@ function serialize_inline_component(node, expression, context) { lets.length === 0 && children.default.every((node) => node.type !== 'SvelteFragment') ) { - push_prop( - b.prop( - 'init', - b.id('children'), - context.state.options.dev ? b.call('$.add_snippet_symbol', slot_fn) : slot_fn - ) - ); + push_prop(b.prop('init', b.id('children'), slot_fn)); // We additionally add the default slot as a boolean, so that the slot render function on the other // side knows it should get the content to render from $$props.children serialized_slots.push(b.init('default', b.true)); @@ -1501,10 +1495,6 @@ const template_visitors = { fn.___snippet = true; // TODO hoist where possible context.state.init.push(fn); - - if (context.state.options.dev) { - context.state.init.push(b.stmt(b.call('$.add_snippet_symbol', node.expression))); - } }, Component(node, context) { serialize_inline_component(node, b.id(node.name), context); diff --git a/packages/svelte/src/index.d.ts b/packages/svelte/src/index.d.ts index 511bb6c02ec0..f56b4c66e04f 100644 --- a/packages/svelte/src/index.d.ts +++ b/packages/svelte/src/index.d.ts @@ -1,5 +1,6 @@ // This should contain all the public interfaces (not all of them are actually importable, check current Svelte for which ones are). +import type { Getters } from '#shared'; import './ambient.js'; /** @@ -104,6 +105,10 @@ export class SvelteComponent< $set(props: Partial): void; } +declare const brand: unique symbol; +type Brand = { [brand]: B }; +type Branded = T & Brand; + /** * Can be used to create strongly typed Svelte components. * @@ -136,7 +141,8 @@ export interface Component< * @param props The props passed to the component. */ ( - internal: unknown, + this: void, + internal: Branded<{}, 'ComponentInternals'>, props: Props ): { /** @@ -271,13 +277,12 @@ declare const SnippetReturn: unique symbol; export interface Snippet { ( this: void, + internal: Branded<{}, 'SnippetInternals'>, // 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. - ...args: number extends Parameters['length'] ? never : Parameters - ): typeof SnippetReturn & { - _: 'functions passed to {@render ...} tags must use the `Snippet` type imported from "svelte"'; - }; + ...args: number extends Parameters['length'] ? never : Getters + ): void; } interface DispatchOptions { diff --git a/packages/svelte/src/internal/client/dom/blocks/snippet.js b/packages/svelte/src/internal/client/dom/blocks/snippet.js index a920f6db3eda..2df4487536b9 100644 --- a/packages/svelte/src/internal/client/dom/blocks/snippet.js +++ b/packages/svelte/src/internal/client/dom/blocks/snippet.js @@ -1,7 +1,6 @@ /** @import { Snippet } from 'svelte' */ /** @import { Effect, TemplateNode } from '#client' */ /** @import { Getters } from '#shared' */ -import { add_snippet_symbol } from '../../../shared/validate.js'; import { EFFECT_TRANSPARENT } from '../../constants.js'; import { branch, block, destroy_effect, teardown } from '../../reactivity/effects.js'; import { @@ -50,10 +49,11 @@ export function snippet(node, get_snippet, ...args) { * In development, wrap the snippet function so that it passes validation, and so that the * correct component context is set for ownership checks * @param {any} component - * @param {(node: TemplateNode, ...args: any[]) => void} fn + * @param {Snippet} fn + * @returns {Snippet} */ export function wrap_snippet(component, fn) { - return add_snippet_symbol((/** @type {TemplateNode} */ node, /** @type {any[]} */ ...args) => { + return (node, ...args) => { var previous_component_function = dev_current_component_function; set_dev_current_component_function(component); @@ -62,7 +62,7 @@ export function wrap_snippet(component, fn) { } finally { set_dev_current_component_function(previous_component_function); } - }); + }; } /** @@ -75,29 +75,27 @@ export function wrap_snippet(component, fn) { * @returns {Snippet} */ export function createRawSnippet(fn) { - return add_snippet_symbol( - (/** @type {TemplateNode} */ anchor, /** @type {Getters} */ ...params) => { - var snippet = fn(...params); + return (anchor, ...params) => { + var snippet = fn(...params); - /** @type {Element} */ - var element; + /** @type {Element} */ + var element; - if (hydrating) { - element = /** @type {Element} */ (hydrate_node); - hydrate_next(); - } else { - var html = snippet.render().trim(); - var fragment = create_fragment_from_html(html); - element = /** @type {Element} */ (fragment.firstChild); - anchor.before(element); - } + if (hydrating) { + element = /** @type {Element} */ (hydrate_node); + hydrate_next(); + } else { + var html = snippet.render().trim(); + var fragment = create_fragment_from_html(html); + element = /** @type {Element} */ (fragment.firstChild); + /** @type {TemplateNode} */ (/** @type {unknown} */ (anchor)).before(element); + } - const result = snippet.setup?.(element); - assign_nodes(element, element); + const result = snippet.setup?.(element); + assign_nodes(element, element); - if (typeof result === 'function') { - teardown(result); - } + if (typeof result === 'function') { + teardown(result); } - ); + }; } diff --git a/packages/svelte/src/internal/server/blocks/snippet.js b/packages/svelte/src/internal/server/blocks/snippet.js index b9f72063f4d7..884f6baa82f4 100644 --- a/packages/svelte/src/internal/server/blocks/snippet.js +++ b/packages/svelte/src/internal/server/blocks/snippet.js @@ -1,7 +1,6 @@ /** @import { Snippet } from 'svelte' */ /** @import { Payload } from '#server' */ /** @import { Getters } from '#shared' */ -import { add_snippet_symbol } from '../../shared/validate.js'; /** * Create a snippet programmatically @@ -13,10 +12,10 @@ import { add_snippet_symbol } from '../../shared/validate.js'; * @returns {Snippet} */ export function createRawSnippet(fn) { - return add_snippet_symbol((/** @type {Payload} */ payload, /** @type {Params} */ ...args) => { + return (payload, ...args) => { var getters = /** @type {Getters} */ (args.map((value) => () => value)); - payload.out += fn(...getters) + /** @type {Payload} */ (/** @type {unknown} */ (payload)).out += fn(...getters) .render() .trim(); - }); + }; } diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index a1a2d3febcc1..74c66631dfe5 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -555,7 +555,6 @@ export { push_element, pop_element } from './dev.js'; export { snapshot } from '../shared/clone.js'; export { - add_snippet_symbol, validate_component, validate_dynamic_element_tag, validate_snippet, diff --git a/packages/svelte/src/internal/shared/validate.js b/packages/svelte/src/internal/shared/validate.js index e08f3ddab1fa..817c11751fc8 100644 --- a/packages/svelte/src/internal/shared/validate.js +++ b/packages/svelte/src/internal/shared/validate.js @@ -4,27 +4,13 @@ import { is_void } from '../../constants.js'; import * as w from './warnings.js'; import * as e from './errors.js'; -const snippet_symbol = Symbol.for('svelte.snippet'); - -/** - * @param {any} fn - * @returns {import('svelte').Snippet} - */ -export function add_snippet_symbol(fn) { - fn[snippet_symbol] = true; - return fn; -} - /** * Validate that the function handed to `{@render ...}` is a snippet function, and not some other kind of function. * @param {any} snippet_fn * @param {Record | undefined} $$props Only passed if render tag receives arguments and is for the children prop */ export function validate_snippet(snippet_fn, $$props) { - if ( - ($$props?.$$slots?.default && typeof $$props.$$slots.default !== 'boolean') || - (snippet_fn && snippet_fn[snippet_symbol] !== true) - ) { + if ($$props?.$$slots?.default && typeof $$props.$$slots.default !== 'boolean') { e.render_tag_invalid_argument(); } @@ -36,11 +22,7 @@ export function validate_snippet(snippet_fn, $$props) { * @param {any} component_fn */ export function validate_component(component_fn) { - if (component_fn?.[snippet_symbol] === true) { - e.snippet_used_as_component(); - } - - return component_fn; + return component_fn; // TODO get rid } /** diff --git a/packages/svelte/tests/runtime-runes/samples/mount-snippet-error/_config.js b/packages/svelte/tests/runtime-runes/samples/mount-snippet-error/_config.js deleted file mode 100644 index 3ba2b358ea52..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/mount-snippet-error/_config.js +++ /dev/null @@ -1,12 +0,0 @@ -import { test } from '../../test'; - -export default test({ - compileOptions: { - dev: true - }, - async test({ assert, target }) { - const div = target.querySelector('div'); - assert.htmlEqual(div?.innerHTML || '', ''); - }, - runtime_error: 'snippet_used_as_component\nA snippet must be rendered with `{@render ...}`' -}); diff --git a/packages/svelte/tests/runtime-runes/samples/mount-snippet-error/main.svelte b/packages/svelte/tests/runtime-runes/samples/mount-snippet-error/main.svelte deleted file mode 100644 index 075fbba24ee8..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/mount-snippet-error/main.svelte +++ /dev/null @@ -1,14 +0,0 @@ - - -
-{#snippet foo()} - shouldnt be rendered -{/snippet} \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-1/_config.js b/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-1/_config.js deleted file mode 100644 index 288edc26a614..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-1/_config.js +++ /dev/null @@ -1,8 +0,0 @@ -import { test } from '../../test'; - -export default test({ - compileOptions: { - dev: true - }, - error: 'render_tag_invalid_argument' -}); diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-1/main.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-1/main.svelte deleted file mode 100644 index d07df452da8b..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-1/main.svelte +++ /dev/null @@ -1,7 +0,0 @@ - - -{@render not_a_snippet()} diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-2/_config.js b/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-2/_config.js deleted file mode 100644 index b549144bf51c..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-2/_config.js +++ /dev/null @@ -1,8 +0,0 @@ -import { test } from '../../test'; - -export default test({ - compileOptions: { - dev: true - }, - error: 'snippet_used_as_component\nA snippet must be rendered with `{@render ...}`' -}); diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-2/main.svelte deleted file mode 100644 index e24e9549e32e..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/snippet-validation-error-2/main.svelte +++ /dev/null @@ -1,5 +0,0 @@ -{#snippet Foo()} -

hello

-{/snippet} - - From cd3e287b06164477c1abbdd846bc2defd998533d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 19 Jul 2024 09:02:14 -0400 Subject: [PATCH 02/14] expose internals --- packages/svelte/src/index.d.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/index.d.ts b/packages/svelte/src/index.d.ts index f56b4c66e04f..bf5d5e419e12 100644 --- a/packages/svelte/src/index.d.ts +++ b/packages/svelte/src/index.d.ts @@ -109,6 +109,11 @@ declare const brand: unique symbol; type Brand = { [brand]: B }; type Branded = T & Brand; +/** + * Internal implementation details that vary between environments + */ +export type ComponentInternals = Branded<{}, 'ComponentInternals'>; + /** * Can be used to create strongly typed Svelte components. * @@ -142,7 +147,7 @@ export interface Component< */ ( this: void, - internal: Branded<{}, 'ComponentInternals'>, + internals: ComponentInternals, props: Props ): { /** @@ -260,7 +265,10 @@ export type ComponentType = (new element?: typeof HTMLElement; }; -declare const SnippetReturn: unique symbol; +/** + * Internal implementation details that vary between environments + */ +export type SnippetInternals = Branded<{}, 'SnippetInternals'>; // Use an interface instead of a type, makes for better intellisense info because the type is named in more situations. /** @@ -277,7 +285,7 @@ declare const SnippetReturn: unique symbol; export interface Snippet { ( this: void, - internal: Branded<{}, 'SnippetInternals'>, + internal: SnippetInternals, // 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. From 5569e625e6afb0b912caa812eb9f316307728156 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 19 Jul 2024 14:22:23 -0400 Subject: [PATCH 03/14] remove unused message --- packages/svelte/messages/shared-errors/errors.md | 4 ++++ packages/svelte/src/internal/shared/errors.js | 16 ---------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/packages/svelte/messages/shared-errors/errors.md b/packages/svelte/messages/shared-errors/errors.md index 1d17b0f6d47d..252e6b4a9110 100644 --- a/packages/svelte/messages/shared-errors/errors.md +++ b/packages/svelte/messages/shared-errors/errors.md @@ -2,6 +2,10 @@ > `%name%(...)` can only be used during component initialisation +## render_tag_invalid_argument + +> The argument to `{@render ...}` must be a snippet function, not a component or a slot with a `let:` directive or some other kind of function. If you want to dynamically render one snippet or another, use `$derived` and pass its result to `{@render ...}` + ## store_invalid_shape > `%name%` is not a store with a `subscribe` method diff --git a/packages/svelte/src/internal/shared/errors.js b/packages/svelte/src/internal/shared/errors.js index 880b37bc71c2..109a9fe02d3f 100644 --- a/packages/svelte/src/internal/shared/errors.js +++ b/packages/svelte/src/internal/shared/errors.js @@ -35,22 +35,6 @@ export function render_tag_invalid_argument() { } } -/** - * A snippet must be rendered with `{@render ...}` - * @returns {never} - */ -export function snippet_used_as_component() { - if (DEV) { - const error = new Error(`snippet_used_as_component\nA snippet must be rendered with \`{@render ...}\``); - - error.name = 'Svelte error'; - throw error; - } else { - // TODO print a link to the documentation - throw new Error("snippet_used_as_component"); - } -} - /** * `%name%` is not a store with a `subscribe` method * @param {string} name From deae40476a623cebcb513e6a792e367a0d318835 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 19 Jul 2024 14:22:30 -0400 Subject: [PATCH 04/14] regenerate types --- packages/svelte/types/index.d.ts | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 86f33588a67b..0f8845095ad5 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -101,6 +101,15 @@ declare module 'svelte' { $set(props: Partial): void; } + const brand: unique symbol; + type Brand = { [brand]: B }; + type Branded = T & Brand; + + /** + * Internal implementation details that vary between environments + */ + export type ComponentInternals = Branded<{}, 'ComponentInternals'>; + /** * Can be used to create strongly typed Svelte components. * @@ -133,7 +142,8 @@ declare module 'svelte' { * @param props The props passed to the component. */ ( - internal: unknown, + this: void, + internals: ComponentInternals, props: Props ): { /** @@ -251,7 +261,10 @@ declare module 'svelte' { element?: typeof HTMLElement; }; - const SnippetReturn: unique symbol; + /** + * Internal implementation details that vary between environments + */ + export type SnippetInternals = Branded<{}, 'SnippetInternals'>; // Use an interface instead of a type, makes for better intellisense info because the type is named in more situations. /** @@ -268,13 +281,12 @@ declare module 'svelte' { export interface Snippet { ( this: void, + internal: SnippetInternals, // 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. - ...args: number extends Parameters['length'] ? never : Parameters - ): typeof SnippetReturn & { - _: 'functions passed to {@render ...} tags must use the `Snippet` type imported from "svelte"'; - }; + ...args: number extends Parameters['length'] ? never : Getters + ): void; } interface DispatchOptions { @@ -293,6 +305,9 @@ declare module 'svelte' { : [type: Type, parameter: EventMap[Type], options?: DispatchOptions] ): boolean; } + type Getters = { + [K in keyof T]: () => T[K]; + }; /** * The `onMount` function schedules a callback to run as soon as the component has been mounted to the DOM. * It must be called during the component's initialisation (but doesn't need to live *inside* the component; @@ -457,9 +472,6 @@ declare module 'svelte' { * https://svelte.dev/docs/svelte#getallcontexts * */ export function getAllContexts = Map>(): T; - type Getters = { - [K in keyof T]: () => T[K]; - }; export {}; } From e76c7c7360810c3a91b6031c2c1abfd4eef12c29 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 19 Jul 2024 16:03:54 -0400 Subject: [PATCH 05/14] replace render_tag_invalid_argument with invalid_default_snippet --- .../svelte/messages/shared-errors/errors.md | 8 ++-- .../3-transform/client/visitors/template.js | 47 +++++++++---------- .../3-transform/server/transform-server.js | 38 +++++++-------- packages/svelte/src/internal/client/index.js | 2 +- packages/svelte/src/internal/server/index.js | 2 +- packages/svelte/src/internal/shared/errors.js | 16 +++++++ .../svelte/src/internal/shared/validate.js | 4 ++ .../samples/snippet-slot-let-error/_config.js | 2 +- 8 files changed, 64 insertions(+), 55 deletions(-) diff --git a/packages/svelte/messages/shared-errors/errors.md b/packages/svelte/messages/shared-errors/errors.md index 252e6b4a9110..ef030c8a1c91 100644 --- a/packages/svelte/messages/shared-errors/errors.md +++ b/packages/svelte/messages/shared-errors/errors.md @@ -1,10 +1,10 @@ -## lifecycle_outside_component +## invalid_default_snippet -> `%name%(...)` can only be used during component initialisation +> Cannot use `{@render children(...)}` if the parent component uses `let:` directives. Consider using a named snippet instead -## render_tag_invalid_argument +## lifecycle_outside_component -> The argument to `{@render ...}` must be a snippet function, not a component or a slot with a `let:` directive or some other kind of function. If you want to dynamically render one snippet or another, use `$derived` and pass its result to `{@render ...}` +> `%name%(...)` can only be used during component initialisation ## store_invalid_shape 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 645895144f38..ef7834296544 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 @@ -884,23 +884,27 @@ function serialize_inline_component(node, component_name, context, anchor = cont ]) ); - if ( - slot_name === 'default' && - !has_children_prop && - lets.length === 0 && - children.default.every((node) => node.type !== 'SvelteFragment') - ) { - push_prop( - b.init( - 'children', - context.state.options.dev - ? b.call('$.wrap_snippet', b.id(context.state.analysis.name), slot_fn) - : slot_fn - ) - ); - // We additionally add the default slot as a boolean, so that the slot render function on the other - // side knows it should get the content to render from $$props.children - serialized_slots.push(b.init(slot_name, b.true)); + if (slot_name === 'default' && !has_children_prop) { + if (lets.length === 0 && children.default.every((node) => node.type !== 'SvelteFragment')) { + // create `children` prop... + push_prop( + b.init( + 'children', + context.state.options.dev + ? b.call('$.wrap_snippet', b.id(context.state.analysis.name), slot_fn) + : slot_fn + ) + ); + + // and `$$slots.default: true` so that `` on the child works + serialized_slots.push(b.init(slot_name, b.true)); + } else { + // create `$$slots.default`... + serialized_slots.push(b.init(slot_name, slot_fn)); + + // and a `children` prop that errors + push_prop(b.init('children', b.id('$.invalid_children_snippet'))); + } } else { serialized_slots.push(b.init(slot_name, slot_fn)); } @@ -1864,15 +1868,6 @@ export const template_visitors = { } let snippet_function = /** @type {Expression} */ (context.visit(callee)); - if (context.state.options.dev) { - snippet_function = b.call( - '$.validate_snippet', - snippet_function, - args.length && callee.type === 'Identifier' && callee.name === 'children' - ? b.id('$$props') - : undefined - ); - } if (node.metadata.dynamic) { context.state.init.push( 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 0f5a1f25222d..690254fe6bc6 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 @@ -963,19 +963,22 @@ function serialize_inline_component(node, expression, context) { ]) ); - if ( - slot_name === 'default' && - !has_children_prop && - lets.length === 0 && - children.default.every((node) => node.type !== 'SvelteFragment') - ) { - push_prop(b.prop('init', b.id('children'), slot_fn)); - // We additionally add the default slot as a boolean, so that the slot render function on the other - // side knows it should get the content to render from $$props.children - serialized_slots.push(b.init('default', b.true)); + if (slot_name === 'default' && !has_children_prop) { + if (lets.length === 0 && children.default.every((node) => node.type !== 'SvelteFragment')) { + // create `children` prop... + push_prop(b.prop('init', b.id('children'), slot_fn)); + + // and `$$slots.default: true` so that `` on the child works + serialized_slots.push(b.init(slot_name, b.true)); + } else { + // create `$$slots.default`... + serialized_slots.push(b.init(slot_name, slot_fn)); + + // and a `children` prop that errors + push_prop(b.init('children', b.id('$.invalid_children_snippet'))); + } } else { - const slot = b.prop('init', b.literal(slot_name), slot_fn); - serialized_slots.push(slot); + serialized_slots.push(b.init(slot_name, slot_fn)); } } @@ -1203,16 +1206,7 @@ const template_visitors = { const callee = unwrap_optional(node.expression).callee; const raw_args = unwrap_optional(node.expression).arguments; - const expression = /** @type {import('estree').Expression} */ (context.visit(callee)); - const snippet_function = context.state.options.dev - ? b.call( - '$.validate_snippet', - expression, - raw_args.length && callee.type === 'Identifier' && callee.name === 'children' - ? b.id('$$props') - : undefined - ) - : expression; + const snippet_function = /** @type {import('estree').Expression} */ (context.visit(callee)); const snippet_args = raw_args.map((arg) => { return /** @type {import('estree').Expression} */ (context.visit(arg)); diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 5901ba1938a8..8f4740069172 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -164,9 +164,9 @@ export { export { snapshot } from '../shared/clone.js'; export { noop } from '../shared/utils.js'; export { + invalid_children_snippet, validate_component, validate_dynamic_element_tag, - validate_snippet, validate_store, validate_void_dynamic_element } from '../shared/validate.js'; diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 74c66631dfe5..5cc1818a448c 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -555,9 +555,9 @@ export { push_element, pop_element } from './dev.js'; export { snapshot } from '../shared/clone.js'; export { + invalid_children_snippet, validate_component, validate_dynamic_element_tag, - validate_snippet, validate_void_dynamic_element } from '../shared/validate.js'; diff --git a/packages/svelte/src/internal/shared/errors.js b/packages/svelte/src/internal/shared/errors.js index 109a9fe02d3f..6d83f03c7256 100644 --- a/packages/svelte/src/internal/shared/errors.js +++ b/packages/svelte/src/internal/shared/errors.js @@ -35,6 +35,22 @@ export function render_tag_invalid_argument() { } } +/** + * Cannot use `{@render children(...)}` if the parent component uses `let:` directives. Consider using a named snippet instead + * @returns {never} + */ +export function invalid_default_snippet() { + if (DEV) { + const error = new Error(`invalid_default_snippet\nCannot use \`{@render children(...)}\` if the parent component uses \`let:\` directives. Consider using a named snippet instead`); + + error.name = 'Svelte error'; + throw error; + } else { + // TODO print a link to the documentation + throw new Error("invalid_default_snippet"); + } +} + /** * `%name%` is not a store with a `subscribe` method * @param {string} name diff --git a/packages/svelte/src/internal/shared/validate.js b/packages/svelte/src/internal/shared/validate.js index 817c11751fc8..05716ab7ca1c 100644 --- a/packages/svelte/src/internal/shared/validate.js +++ b/packages/svelte/src/internal/shared/validate.js @@ -17,6 +17,10 @@ export function validate_snippet(snippet_fn, $$props) { return snippet_fn; } +export function invalid_children_snippet() { + e.invalid_default_snippet(); +} + /** * Validate that the function behind `` isn't a snippet. * @param {any} component_fn diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-error/_config.js b/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-error/_config.js index 2fbcf3eb10fb..e8adfb38f2d5 100644 --- a/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-error/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-error/_config.js @@ -4,5 +4,5 @@ export default test({ compileOptions: { dev: true }, - runtime_error: 'render_tag_invalid_argument' + runtime_error: 'invalid_default_snippet' }); From 46a1fc80e4cb913c86dfaa4e4dca1eb9c4aa015b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 19 Jul 2024 16:05:30 -0400 Subject: [PATCH 06/14] remove unused code --- packages/svelte/src/internal/shared/validate.js | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/packages/svelte/src/internal/shared/validate.js b/packages/svelte/src/internal/shared/validate.js index 05716ab7ca1c..e9758f25cbc0 100644 --- a/packages/svelte/src/internal/shared/validate.js +++ b/packages/svelte/src/internal/shared/validate.js @@ -4,19 +4,6 @@ import { is_void } from '../../constants.js'; import * as w from './warnings.js'; import * as e from './errors.js'; -/** - * Validate that the function handed to `{@render ...}` is a snippet function, and not some other kind of function. - * @param {any} snippet_fn - * @param {Record | undefined} $$props Only passed if render tag receives arguments and is for the children prop - */ -export function validate_snippet(snippet_fn, $$props) { - if ($$props?.$$slots?.default && typeof $$props.$$slots.default !== 'boolean') { - e.render_tag_invalid_argument(); - } - - return snippet_fn; -} - export function invalid_children_snippet() { e.invalid_default_snippet(); } From 6dba6adbf9dbf78a6ee0a5e779b6b1d729e1384c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 19 Jul 2024 16:07:18 -0400 Subject: [PATCH 07/14] simplify --- .../compiler/phases/3-transform/client/visitors/template.js | 2 +- .../compiler/phases/3-transform/server/transform-server.js | 2 +- packages/svelte/src/internal/client/index.js | 2 +- packages/svelte/src/internal/server/index.js | 2 +- packages/svelte/src/internal/shared/validate.js | 4 +--- 5 files changed, 5 insertions(+), 7 deletions(-) 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 ef7834296544..0d17ea99936e 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 @@ -903,7 +903,7 @@ function serialize_inline_component(node, component_name, context, anchor = cont serialized_slots.push(b.init(slot_name, slot_fn)); // and a `children` prop that errors - push_prop(b.init('children', b.id('$.invalid_children_snippet'))); + push_prop(b.init('children', b.id('$.invalid_default_snippet'))); } } else { serialized_slots.push(b.init(slot_name, slot_fn)); 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 690254fe6bc6..b6048e1fec1d 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 @@ -975,7 +975,7 @@ function serialize_inline_component(node, expression, context) { serialized_slots.push(b.init(slot_name, slot_fn)); // and a `children` prop that errors - push_prop(b.init('children', b.id('$.invalid_children_snippet'))); + push_prop(b.init('children', b.id('$.invalid_default_snippet'))); } } else { serialized_slots.push(b.init(slot_name, slot_fn)); diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 8f4740069172..10cb4e9195a9 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -164,7 +164,7 @@ export { export { snapshot } from '../shared/clone.js'; export { noop } from '../shared/utils.js'; export { - invalid_children_snippet, + invalid_default_snippet, validate_component, validate_dynamic_element_tag, validate_store, diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 5cc1818a448c..3a099767745c 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -555,7 +555,7 @@ export { push_element, pop_element } from './dev.js'; export { snapshot } from '../shared/clone.js'; export { - invalid_children_snippet, + invalid_default_snippet, validate_component, validate_dynamic_element_tag, validate_void_dynamic_element diff --git a/packages/svelte/src/internal/shared/validate.js b/packages/svelte/src/internal/shared/validate.js index e9758f25cbc0..0547c4280109 100644 --- a/packages/svelte/src/internal/shared/validate.js +++ b/packages/svelte/src/internal/shared/validate.js @@ -4,9 +4,7 @@ import { is_void } from '../../constants.js'; import * as w from './warnings.js'; import * as e from './errors.js'; -export function invalid_children_snippet() { - e.invalid_default_snippet(); -} +export { invalid_default_snippet } from './errors.js'; /** * Validate that the function behind `` isn't a snippet. From dd926cde27982d4532aadf4ea06a6012143d4bc4 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 19 Jul 2024 16:33:26 -0400 Subject: [PATCH 08/14] changesets --- .changeset/forty-bikes-buy.md | 5 +++++ .changeset/moody-lions-watch.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/forty-bikes-buy.md create mode 100644 .changeset/moody-lions-watch.md diff --git a/.changeset/forty-bikes-buy.md b/.changeset/forty-bikes-buy.md new file mode 100644 index 000000000000..503c10451b4f --- /dev/null +++ b/.changeset/forty-bikes-buy.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: improve validation error that occurs when using `{@render ...}` to render default slotted content diff --git a/.changeset/moody-lions-watch.md b/.changeset/moody-lions-watch.md new file mode 100644 index 000000000000..ef15dc742eba --- /dev/null +++ b/.changeset/moody-lions-watch.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: update component/snippet types to more accurately reflect implementation From 38e8fb680ac0994d2c0118d26fefc07a9b96a6e5 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 19 Jul 2024 17:25:34 -0400 Subject: [PATCH 09/14] regenerate types --- packages/svelte/src/internal/shared/errors.js | 34 +++++-------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/packages/svelte/src/internal/shared/errors.js b/packages/svelte/src/internal/shared/errors.js index 6d83f03c7256..0d72800fb764 100644 --- a/packages/svelte/src/internal/shared/errors.js +++ b/packages/svelte/src/internal/shared/errors.js @@ -3,51 +3,35 @@ import { DEV } from 'esm-env'; /** - * `%name%(...)` can only be used during component initialisation - * @param {string} name - * @returns {never} - */ -export function lifecycle_outside_component(name) { - if (DEV) { - const error = new Error(`lifecycle_outside_component\n\`${name}(...)\` can only be used during component initialisation`); - - error.name = 'Svelte error'; - throw error; - } else { - // TODO print a link to the documentation - throw new Error("lifecycle_outside_component"); - } -} - -/** - * The argument to `{@render ...}` must be a snippet function, not a component or a slot with a `let:` directive or some other kind of function. If you want to dynamically render one snippet or another, use `$derived` and pass its result to `{@render ...}` + * Cannot use `{@render children(...)}` if the parent component uses `let:` directives. Consider using a named snippet instead * @returns {never} */ -export function render_tag_invalid_argument() { +export function invalid_default_snippet() { if (DEV) { - const error = new Error(`render_tag_invalid_argument\nThe argument to \`{@render ...}\` must be a snippet function, not a component or a slot with a \`let:\` directive or some other kind of function. If you want to dynamically render one snippet or another, use \`$derived\` and pass its result to \`{@render ...}\``); + const error = new Error(`invalid_default_snippet\nCannot use \`{@render children(...)}\` if the parent component uses \`let:\` directives. Consider using a named snippet instead`); error.name = 'Svelte error'; throw error; } else { // TODO print a link to the documentation - throw new Error("render_tag_invalid_argument"); + throw new Error("invalid_default_snippet"); } } /** - * Cannot use `{@render children(...)}` if the parent component uses `let:` directives. Consider using a named snippet instead + * `%name%(...)` can only be used during component initialisation + * @param {string} name * @returns {never} */ -export function invalid_default_snippet() { +export function lifecycle_outside_component(name) { if (DEV) { - const error = new Error(`invalid_default_snippet\nCannot use \`{@render children(...)}\` if the parent component uses \`let:\` directives. Consider using a named snippet instead`); + const error = new Error(`lifecycle_outside_component\n\`${name}(...)\` can only be used during component initialisation`); error.name = 'Svelte error'; throw error; } else { // TODO print a link to the documentation - throw new Error("invalid_default_snippet"); + throw new Error("lifecycle_outside_component"); } } From 3ebc8bd8220d57517d232a5df80699a3c2b554e8 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 19 Jul 2024 17:42:22 -0400 Subject: [PATCH 10/14] update type tests --- packages/svelte/tests/types/component.ts | 5 ++-- packages/svelte/tests/types/snippet.ts | 37 +++++++----------------- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/packages/svelte/tests/types/component.ts b/packages/svelte/tests/types/component.ts index fc806537969c..eccf08548ec1 100644 --- a/packages/svelte/tests/types/component.ts +++ b/packages/svelte/tests/types/component.ts @@ -6,7 +6,8 @@ import { type ComponentType, mount, hydrate, - type Component + type Component, + type ComponentInternals } from 'svelte'; import { render } from 'svelte/server'; @@ -338,5 +339,5 @@ render(functionComponent, { // but should always pass in tsc (because it will never know about this fact) import Foo from './doesntexist.svelte'; -Foo(null, { a: true }); +Foo(null as unknown as ComponentInternals, { a: true }); const f: Foo = new Foo({ target: document.body, props: { a: true } }); diff --git a/packages/svelte/tests/types/snippet.ts b/packages/svelte/tests/types/snippet.ts index edc5aba12378..9aeef3330bd6 100644 --- a/packages/svelte/tests/types/snippet.ts +++ b/packages/svelte/tests/types/snippet.ts @@ -1,40 +1,25 @@ import type { Snippet } from 'svelte'; -const return_type: ReturnType = null as any; - // @ts-expect-error const a: Snippet<{ text: string }> = () => {}; // @ts-expect-error -const b: Snippet = (a, b) => { - return return_type; -}; -// @ts-expect-error -const c: Snippet = (a: string) => { - return return_type; -}; +const b: Snippet = (a, b) => {}; // @ts-expect-error -const d: Snippet = (a: string, b: number) => { - return return_type; -}; +const c: Snippet = (a: string) => {}; // @ts-expect-error -const e: Snippet = (a: string) => { - return return_type; -}; +const d: Snippet = (a: string, b: number) => {}; // @ts-expect-error +const e: Snippet = (a: string) => {}; const f: Snippet = (a) => { + // @ts-expect-error a?.x; - return return_type; }; -const g: Snippet<[boolean]> = (a) => { +const g: Snippet<[boolean]> = (internals, a) => { // @ts-expect-error - a === ''; - a === true; - return return_type; -}; -const h: Snippet<[{ a: true }]> = (a) => { - a.a === true; - return return_type; + a() === ''; + a() === true; }; -const i: Snippet = () => { - return return_type; +const h: Snippet<[{ a: true }]> = (internals, a) => { + a().a === true; }; +const i: Snippet = () => {}; From 748a13bd899837a021d50a6f128eb3c5b8229621 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 21 Jul 2024 23:58:08 -0400 Subject: [PATCH 11/14] unused --- .../phases/3-transform/client/visitors/template.js | 8 +------- .../phases/3-transform/server/transform-server.js | 2 +- packages/svelte/src/internal/client/index.js | 1 - packages/svelte/src/internal/client/render.js | 9 --------- packages/svelte/src/internal/server/index.js | 1 - packages/svelte/src/internal/shared/validate.js | 8 -------- 6 files changed, 2 insertions(+), 27 deletions(-) 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 52693cb2eb55..76329a09bb1d 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 @@ -928,13 +928,7 @@ function serialize_inline_component(node, component_name, context, anchor = cont /** @param {Expression} node_id */ let fn = (node_id) => { - return b.call( - context.state.options.dev - ? b.call('$.validate_component', b.id(component_name)) - : component_name, - node_id, - props_expression - ); + return b.call(component_name, node_id, props_expression); }; if (bind_this !== null) { 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 b6048e1fec1d..0a5d05a6ed85 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 @@ -998,7 +998,7 @@ function serialize_inline_component(node, expression, context) { /** @type {import('estree').Statement} */ let statement = b.stmt( (node.type === 'SvelteComponent' ? b.maybe_call : b.call)( - context.state.options.dev ? b.call('$.validate_component', expression) : expression, + expression, b.id('$$payload'), props_expression ) diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 10cb4e9195a9..6af6e0a4a667 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -165,7 +165,6 @@ export { snapshot } from '../shared/clone.js'; export { noop } from '../shared/utils.js'; export { invalid_default_snippet, - validate_component, validate_dynamic_element_tag, validate_store, validate_void_dynamic_element diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 8510d49dab43..21cc096325af 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -24,7 +24,6 @@ import { import { reset_head_anchor } from './dom/blocks/svelte-head.js'; import * as w from './warnings.js'; import * as e from './errors.js'; -import { validate_component } from '../shared/validate.js'; import { assign_nodes } from './dom/template.js'; /** @@ -79,10 +78,6 @@ export function set_text(text, value) { * @returns {Exports} */ export function mount(component, options) { - if (DEV) { - validate_component(component); - } - const anchor = options.anchor ?? options.target.appendChild(empty()); // Don't flush previous effects to ensure order of outer effects stays consistent return flush_sync(() => _mount(component, { ...options, anchor }), false); @@ -112,10 +107,6 @@ export function mount(component, options) { * @returns {Exports} */ export function hydrate(component, options) { - if (DEV) { - validate_component(component); - } - options.intro = options.intro ?? false; const target = options.target; const was_hydrating = hydrating; diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 3a099767745c..b962e0e65ac6 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -556,7 +556,6 @@ export { snapshot } from '../shared/clone.js'; export { invalid_default_snippet, - validate_component, validate_dynamic_element_tag, validate_void_dynamic_element } from '../shared/validate.js'; diff --git a/packages/svelte/src/internal/shared/validate.js b/packages/svelte/src/internal/shared/validate.js index 0547c4280109..d0b5f323377b 100644 --- a/packages/svelte/src/internal/shared/validate.js +++ b/packages/svelte/src/internal/shared/validate.js @@ -6,14 +6,6 @@ import * as e from './errors.js'; export { invalid_default_snippet } from './errors.js'; -/** - * Validate that the function behind `` isn't a snippet. - * @param {any} component_fn - */ -export function validate_component(component_fn) { - return component_fn; // TODO get rid -} - /** * @param {() => string} tag_fn * @returns {void} From 106a4219a9b476fabc506c4fd635cbc6dab02619 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 22 Jul 2024 11:32:33 -0400 Subject: [PATCH 12/14] revert snippet type changes --- packages/svelte/src/index.d.ts | 12 +++++------- .../svelte/src/internal/client/dom/blocks/snippet.js | 10 +++++----- .../svelte/src/internal/server/blocks/snippet.js | 5 +++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/svelte/src/index.d.ts b/packages/svelte/src/index.d.ts index bf5d5e419e12..5a052e3c1253 100644 --- a/packages/svelte/src/index.d.ts +++ b/packages/svelte/src/index.d.ts @@ -265,10 +265,7 @@ export type ComponentType = (new element?: typeof HTMLElement; }; -/** - * Internal implementation details that vary between environments - */ -export type SnippetInternals = Branded<{}, 'SnippetInternals'>; +declare const SnippetReturn: unique symbol; // Use an interface instead of a type, makes for better intellisense info because the type is named in more situations. /** @@ -285,12 +282,13 @@ export type SnippetInternals = Branded<{}, 'SnippetInternals'>; export interface Snippet { ( this: void, - internal: SnippetInternals, // 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. - ...args: number extends Parameters['length'] ? never : Getters - ): void; + ...args: number extends Parameters['length'] ? never : Parameters + ): typeof SnippetReturn & { + _: 'functions passed to {@render ...} tags must use the `Snippet` type imported from "svelte"'; + }; } interface DispatchOptions { diff --git a/packages/svelte/src/internal/client/dom/blocks/snippet.js b/packages/svelte/src/internal/client/dom/blocks/snippet.js index 846fbd671aa8..6a6203d16ee2 100644 --- a/packages/svelte/src/internal/client/dom/blocks/snippet.js +++ b/packages/svelte/src/internal/client/dom/blocks/snippet.js @@ -51,11 +51,10 @@ export function snippet(node, get_snippet, ...args) { * In development, wrap the snippet function so that it passes validation, and so that the * correct component context is set for ownership checks * @param {any} component - * @param {Snippet} fn - * @returns {Snippet} + * @param {(node: TemplateNode, ...args: any[]) => void} fn */ export function wrap_snippet(component, fn) { - return (node, ...args) => { + return (/** @type {TemplateNode} */ node, /** @type {any[]} */ ...args) => { var previous_component_function = dev_current_component_function; set_dev_current_component_function(component); @@ -77,7 +76,8 @@ export function wrap_snippet(component, fn) { * @returns {Snippet} */ export function createRawSnippet(fn) { - return (anchor, ...params) => { + // @ts-expect-error the types are a lie + return (/** @type {TemplateNode} */ anchor, /** @type {Getters} */ ...params) => { var snippet = fn(...params); /** @type {Element} */ @@ -95,7 +95,7 @@ export function createRawSnippet(fn) { w.invalid_raw_snippet_render(); } - /** @type {TemplateNode} */ (/** @type {unknown} */ (anchor)).before(element); + anchor.before(element); } const result = snippet.setup?.(element); diff --git a/packages/svelte/src/internal/server/blocks/snippet.js b/packages/svelte/src/internal/server/blocks/snippet.js index 884f6baa82f4..d9469be511a0 100644 --- a/packages/svelte/src/internal/server/blocks/snippet.js +++ b/packages/svelte/src/internal/server/blocks/snippet.js @@ -12,9 +12,10 @@ * @returns {Snippet} */ export function createRawSnippet(fn) { - return (payload, ...args) => { + // @ts-expect-error the types are a lie + return (/** @type {Payload} */ payload, /** @type {Params} */ ...args) => { var getters = /** @type {Getters} */ (args.map((value) => () => value)); - /** @type {Payload} */ (/** @type {unknown} */ (payload)).out += fn(...getters) + payload.out += fn(...getters) .render() .trim(); }; From cc1357bfd647b178faac120fd77eaaa99ea7354b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 22 Jul 2024 12:37:25 -0400 Subject: [PATCH 13/14] tidy up --- packages/svelte/tests/types/snippet.ts | 37 ++++++++++++++++++-------- packages/svelte/types/index.d.ts | 12 ++++----- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/packages/svelte/tests/types/snippet.ts b/packages/svelte/tests/types/snippet.ts index 9aeef3330bd6..edc5aba12378 100644 --- a/packages/svelte/tests/types/snippet.ts +++ b/packages/svelte/tests/types/snippet.ts @@ -1,25 +1,40 @@ import type { Snippet } from 'svelte'; +const return_type: ReturnType = null as any; + // @ts-expect-error const a: Snippet<{ text: string }> = () => {}; // @ts-expect-error -const b: Snippet = (a, b) => {}; +const b: Snippet = (a, b) => { + return return_type; +}; +// @ts-expect-error +const c: Snippet = (a: string) => { + return return_type; +}; // @ts-expect-error -const c: Snippet = (a: string) => {}; +const d: Snippet = (a: string, b: number) => { + return return_type; +}; // @ts-expect-error -const d: Snippet = (a: string, b: number) => {}; +const e: Snippet = (a: string) => { + return return_type; +}; // @ts-expect-error -const e: Snippet = (a: string) => {}; const f: Snippet = (a) => { - // @ts-expect-error a?.x; + return return_type; }; -const g: Snippet<[boolean]> = (internals, a) => { +const g: Snippet<[boolean]> = (a) => { // @ts-expect-error - a() === ''; - a() === true; + a === ''; + a === true; + return return_type; +}; +const h: Snippet<[{ a: true }]> = (a) => { + a.a === true; + return return_type; }; -const h: Snippet<[{ a: true }]> = (internals, a) => { - a().a === true; +const i: Snippet = () => { + return return_type; }; -const i: Snippet = () => {}; diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 0f8845095ad5..ba71b15d0508 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -261,10 +261,7 @@ declare module 'svelte' { element?: typeof HTMLElement; }; - /** - * Internal implementation details that vary between environments - */ - export type SnippetInternals = Branded<{}, 'SnippetInternals'>; + const SnippetReturn: unique symbol; // Use an interface instead of a type, makes for better intellisense info because the type is named in more situations. /** @@ -281,12 +278,13 @@ declare module 'svelte' { export interface Snippet { ( this: void, - internal: SnippetInternals, // 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. - ...args: number extends Parameters['length'] ? never : Getters - ): void; + ...args: number extends Parameters['length'] ? never : Parameters + ): typeof SnippetReturn & { + _: 'functions passed to {@render ...} tags must use the `Snippet` type imported from "svelte"'; + }; } interface DispatchOptions { From c27d8decf068fe02c93bdda16a7734273b21b850 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 22 Jul 2024 12:48:26 -0400 Subject: [PATCH 14/14] Update .changeset/moody-lions-watch.md --- .changeset/moody-lions-watch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/moody-lions-watch.md b/.changeset/moody-lions-watch.md index ef15dc742eba..8efd727883a2 100644 --- a/.changeset/moody-lions-watch.md +++ b/.changeset/moody-lions-watch.md @@ -2,4 +2,4 @@ 'svelte': patch --- -fix: update component/snippet types to more accurately reflect implementation +fix: remove runtime validation of components/snippets, rely on types instead