From 3b88b88886bdf8fef064140e91938e162e80813d Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 5 Sep 2024 18:07:36 +0100 Subject: [PATCH] fix: prevent nullish snippet for rendering empty content (#13083) * fix: prevent nullish snippet for rendering empty content * fix: prevent nullish snippet for rendering empty content * lint * fix message * alternative approach * tweak * feedback --- .changeset/tame-frogs-shave.md | 5 +++++ packages/svelte/messages/client-errors/errors.md | 4 ++++ .../3-transform/client/visitors/RenderTag.js | 5 +++++ .../src/internal/client/dom/blocks/snippet.js | 11 ++++++++--- packages/svelte/src/internal/client/errors.js | 16 ++++++++++++++++ .../samples/snippet-undefined/_config.js | 13 +++++++++++++ .../samples/snippet-undefined/main.svelte | 13 +++++++++++++ 7 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 .changeset/tame-frogs-shave.md create mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-undefined/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/snippet-undefined/main.svelte diff --git a/.changeset/tame-frogs-shave.md b/.changeset/tame-frogs-shave.md new file mode 100644 index 0000000000..af0728a4a9 --- /dev/null +++ b/.changeset/tame-frogs-shave.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: prevent nullish snippet for rendering empty content diff --git a/packages/svelte/messages/client-errors/errors.md b/packages/svelte/messages/client-errors/errors.md index 4e8af2e6b4..9bd7e8a654 100644 --- a/packages/svelte/messages/client-errors/errors.md +++ b/packages/svelte/messages/client-errors/errors.md @@ -48,6 +48,10 @@ > Failed to hydrate the application +## invalid_snippet + +> Could not `{@render}` snippet due to the expression being `null` or `undefined`. Consider using optional chaining `{@render snippet?.()}` + ## lifecycle_legacy_only > `%name%(...)` cannot be used in runes mode diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RenderTag.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RenderTag.js index cfb50a13db..7da987f6cc 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RenderTag.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RenderTag.js @@ -30,6 +30,11 @@ export function RenderTag(node, context) { let snippet_function = /** @type {Expression} */ (context.visit(callee)); if (node.metadata.dynamic) { + // If we have a chain expression then ensure a nullish snippet function gets turned into an empty one + if (node.expression.type === 'ChainExpression') { + snippet_function = b.logical('??', snippet_function, b.id('$.noop')); + } + context.state.init.push( b.stmt(b.call('$.snippet', context.state.node, b.thunk(snippet_function), ...args)) ); diff --git a/packages/svelte/src/internal/client/dom/blocks/snippet.js b/packages/svelte/src/internal/client/dom/blocks/snippet.js index c184f72ddc..cec57f83b7 100644 --- a/packages/svelte/src/internal/client/dom/blocks/snippet.js +++ b/packages/svelte/src/internal/client/dom/blocks/snippet.js @@ -11,8 +11,10 @@ import { hydrate_next, hydrate_node, hydrating } from '../hydration.js'; import { create_fragment_from_html } from '../reconciler.js'; import { assign_nodes } from '../template.js'; import * as w from '../../warnings.js'; +import * as e from '../../errors.js'; import { DEV } from 'esm-env'; import { get_first_child, get_next_sibling } from '../operations.js'; +import { noop } from '../../../shared/utils.js'; /** * @template {(node: TemplateNode, ...args: any[]) => void} SnippetFn @@ -25,7 +27,8 @@ export function snippet(node, get_snippet, ...args) { var anchor = node; /** @type {SnippetFn | null | undefined} */ - var snippet; + // @ts-ignore + var snippet = noop; /** @type {Effect | null} */ var snippet_effect; @@ -38,9 +41,11 @@ export function snippet(node, get_snippet, ...args) { snippet_effect = null; } - if (snippet) { - snippet_effect = branch(() => /** @type {SnippetFn} */ (snippet)(anchor, ...args)); + if (DEV && snippet == null) { + e.invalid_snippet(); } + + snippet_effect = branch(() => /** @type {SnippetFn} */ (snippet)(anchor, ...args)); }, EFFECT_TRANSPARENT); if (hydrating) { diff --git a/packages/svelte/src/internal/client/errors.js b/packages/svelte/src/internal/client/errors.js index 8b537ed7ba..26d38f0aba 100644 --- a/packages/svelte/src/internal/client/errors.js +++ b/packages/svelte/src/internal/client/errors.js @@ -210,6 +210,22 @@ export function hydration_failed() { } } +/** + * Could not `{@render}` snippet due to the expression being `null` or `undefined`. Consider using optional chaining `{@render snippet?.()}` + * @returns {never} + */ +export function invalid_snippet() { + if (DEV) { + const error = new Error(`invalid_snippet\nCould not \`{@render}\` snippet due to the expression being \`null\` or \`undefined\`. Consider using optional chaining \`{@render snippet?.()}\``); + + error.name = 'Svelte error'; + throw error; + } else { + // TODO print a link to the documentation + throw new Error("invalid_snippet"); + } +} + /** * `%name%(...)` cannot be used in runes mode * @param {string} name diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-undefined/_config.js b/packages/svelte/tests/runtime-runes/samples/snippet-undefined/_config.js new file mode 100644 index 0000000000..4a13e208b5 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/snippet-undefined/_config.js @@ -0,0 +1,13 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + test({ assert, target }) { + const btn = target.querySelector('button'); + + assert.throws(() => { + btn?.click(); + flushSync(); + }, /invalid_snippet/); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-undefined/main.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-undefined/main.svelte new file mode 100644 index 0000000000..09607fec0d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/snippet-undefined/main.svelte @@ -0,0 +1,13 @@ + + +{#snippet counter()} + Test +{/snippet} + +{@render state.value()} + +