From ff080cbbdc6d520f40b04960a3a84bc1d7753912 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 21 Jul 2024 16:38:16 -0400 Subject: [PATCH] fix: improve validation error that occurs when using `{@render ...}` to render default slotted content (#12521) * add invalid_default_snippet error message * fix: improve validation error that occurs when using `{@render ...}` to render default slotted content * cheeky hack to keep treeshakeability until we can nuke this validation altogether --- .changeset/forty-bikes-buy.md | 5 ++ .../svelte/messages/shared-errors/errors.md | 4 ++ .../3-transform/client/visitors/template.js | 46 +++++++++--------- .../3-transform/server/transform-server.js | 47 +++++++++---------- packages/svelte/src/internal/client/index.js | 1 + packages/svelte/src/internal/server/index.js | 1 + packages/svelte/src/internal/shared/errors.js | 16 +++++++ .../svelte/src/internal/shared/validate.js | 11 ++--- .../samples/snippet-slot-let-error/_config.js | 2 +- .../_config.js | 8 ++++ .../inner.svelte | 5 ++ .../main.svelte | 7 +++ 12 files changed, 97 insertions(+), 56 deletions(-) create mode 100644 .changeset/forty-bikes-buy.md create mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/inner.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/main.svelte diff --git a/.changeset/forty-bikes-buy.md b/.changeset/forty-bikes-buy.md new file mode 100644 index 0000000000..503c10451b --- /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/packages/svelte/messages/shared-errors/errors.md b/packages/svelte/messages/shared-errors/errors.md index d693b35e05..8748182a81 100644 --- a/packages/svelte/messages/shared-errors/errors.md +++ b/packages/svelte/messages/shared-errors/errors.md @@ -1,3 +1,7 @@ +## invalid_default_snippet + +> Cannot use `{@render children(...)}` if the parent component uses `let:` directives. Consider using a named snippet instead + ## lifecycle_outside_component > `%name%(...)` can only be used during component initialisation 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 645895144f..02a7699f0b 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_default_snippet'))); + } } else { serialized_slots.push(b.init(slot_name, slot_fn)); } @@ -1865,13 +1869,7 @@ 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 - ); + snippet_function = b.call('$.validate_snippet', snippet_function); } if (node.metadata.dynamic) { 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 04115a850e..1d0695b5f0 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,25 +963,28 @@ 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'), - context.state.options.dev ? b.call('$.add_snippet_symbol', 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('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'), + context.state.options.dev ? b.call('$.add_snippet_symbol', 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_default_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)); } } @@ -1211,13 +1214,7 @@ const template_visitors = { 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 - ) + ? b.call('$.validate_snippet', expression) : expression; const snippet_args = raw_args.map((arg) => { diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 5901ba1938..859e85f22c 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -164,6 +164,7 @@ export { export { snapshot } from '../shared/clone.js'; export { noop } from '../shared/utils.js'; export { + invalid_default_snippet, validate_component, validate_dynamic_element_tag, validate_snippet, diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index a1a2d3febc..6300924ab2 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -556,6 +556,7 @@ export { snapshot } from '../shared/clone.js'; export { add_snippet_symbol, + invalid_default_snippet, validate_component, validate_dynamic_element_tag, validate_snippet, diff --git a/packages/svelte/src/internal/shared/errors.js b/packages/svelte/src/internal/shared/errors.js index 880b37bc71..8f0ec8eea4 100644 --- a/packages/svelte/src/internal/shared/errors.js +++ b/packages/svelte/src/internal/shared/errors.js @@ -2,6 +2,22 @@ import { DEV } from 'esm-env'; +/** + * 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%(...)` can only be used during component initialisation * @param {string} name diff --git a/packages/svelte/src/internal/shared/validate.js b/packages/svelte/src/internal/shared/validate.js index e08f3ddab1..a8acc167c8 100644 --- a/packages/svelte/src/internal/shared/validate.js +++ b/packages/svelte/src/internal/shared/validate.js @@ -6,10 +6,13 @@ import * as e from './errors.js'; const snippet_symbol = Symbol.for('svelte.snippet'); +export const invalid_default_snippet = add_snippet_symbol(e.invalid_default_snippet); + /** * @param {any} fn * @returns {import('svelte').Snippet} */ +/*@__NO_SIDE_EFFECTS__*/ export function add_snippet_symbol(fn) { fn[snippet_symbol] = true; return fn; @@ -18,13 +21,9 @@ export function add_snippet_symbol(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) - ) { +export function validate_snippet(snippet_fn) { + if (snippet_fn && snippet_fn[snippet_symbol] !== true) { e.render_tag_invalid_argument(); } 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 2fbcf3eb10..e8adfb38f2 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' }); diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/_config.js b/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/_config.js new file mode 100644 index 0000000000..e8adfb38f2 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/_config.js @@ -0,0 +1,8 @@ +import { test } from '../../test'; + +export default test({ + compileOptions: { + dev: true + }, + runtime_error: 'invalid_default_snippet' +}); diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/inner.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/inner.svelte new file mode 100644 index 0000000000..016a1ef319 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/inner.svelte @@ -0,0 +1,5 @@ + + +{@render x(true)} diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/main.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/main.svelte new file mode 100644 index 0000000000..6bac8819c6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/snippet-slot-let-renamed-children-error/main.svelte @@ -0,0 +1,7 @@ + + + + {foo} +