From 67a6753f0b83427e00e26cf2e6d5cfaeabd69cd7 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Sat, 15 Mar 2025 13:32:46 -0700 Subject: [PATCH 01/10] init --- .changeset/bright-jeans-compare.md | 5 +++++ .../98-reference/.generated/shared-errors.md | 6 ++++++ .../svelte/messages/shared-errors/errors.md | 4 ++++ .../3-transform/client/visitors/SnippetBlock.js | 17 ++++++++++++++--- .../3-transform/server/visitors/SnippetBlock.js | 5 ++++- .../src/internal/client/dev/validation.js | 15 +++++++++++++++ packages/svelte/src/internal/client/index.js | 1 + packages/svelte/src/internal/server/dev.js | 10 ++++++++++ packages/svelte/src/internal/server/index.js | 2 +- packages/svelte/src/internal/shared/errors.js | 15 +++++++++++++++ 10 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 .changeset/bright-jeans-compare.md create mode 100644 packages/svelte/src/internal/client/dev/validation.js diff --git a/.changeset/bright-jeans-compare.md b/.changeset/bright-jeans-compare.md new file mode 100644 index 000000000000..206fecdecfbf --- /dev/null +++ b/.changeset/bright-jeans-compare.md @@ -0,0 +1,5 @@ +--- +'svelte': minor +--- + +fix: add snippet argument validation in dev diff --git a/documentation/docs/98-reference/.generated/shared-errors.md b/documentation/docs/98-reference/.generated/shared-errors.md index 0102aafcbca1..09652c7d0cb7 100644 --- a/documentation/docs/98-reference/.generated/shared-errors.md +++ b/documentation/docs/98-reference/.generated/shared-errors.md @@ -30,6 +30,12 @@ This error would be thrown in a setup like this: Here, `List.svelte` is using `{@render children(item)` which means it expects `Parent.svelte` to use snippets. Instead, `Parent.svelte` uses the deprecated `let:` directive. This combination of APIs is incompatible, hence the error. +### invalid_snippet_arguments + +``` +A snippet function was passed invalid arguments. A snippet function should only be called via `{@render ...}` +``` + ### lifecycle_outside_component ``` diff --git a/packages/svelte/messages/shared-errors/errors.md b/packages/svelte/messages/shared-errors/errors.md index 8b4c61303a07..6367ce726e4e 100644 --- a/packages/svelte/messages/shared-errors/errors.md +++ b/packages/svelte/messages/shared-errors/errors.md @@ -26,6 +26,10 @@ This error would be thrown in a setup like this: Here, `List.svelte` is using `{@render children(item)` which means it expects `Parent.svelte` to use snippets. Instead, `Parent.svelte` uses the deprecated `let:` directive. This combination of APIs is incompatible, hence the error. +## invalid_snippet_arguments + +> A snippet function was passed invalid arguments. A snippet function should only be called via `{@render ...}` + ## lifecycle_outside_component > `%name%(...)` can only be used during component initialisation diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js index 7a0d6981b52a..7eb043aa5d11 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js @@ -1,4 +1,4 @@ -/** @import { BlockStatement, Expression, Identifier, Pattern, Statement } from 'estree' */ +/** @import { AssignmentPattern, BlockStatement, Expression, Identifier, Statement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types' */ import { dev } from '../../../../state.js'; @@ -12,7 +12,7 @@ import { get_value } from './shared/declarations.js'; */ export function SnippetBlock(node, context) { // TODO hoist where possible - /** @type {Pattern[]} */ + /** @type {(Identifier | AssignmentPattern)[]} */ const args = [b.id('$$anchor')]; /** @type {BlockStatement} */ @@ -66,7 +66,18 @@ export function SnippetBlock(node, context) { } } } - + if (dev) { + declarations.unshift( + b.stmt( + b.call( + '$.validate_snippet_args', + .../** @type {Identifier[]} */ ( + args.map((arg) => (arg?.type === 'Identifier' ? arg : arg?.left)) + ) + ) + ) + ); + } body = b.block([ ...declarations, .../** @type {BlockStatement} */ (context.visit(node.body, child_state)).body diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SnippetBlock.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SnippetBlock.js index eb839179271a..cae3e7d79c94 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/SnippetBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/SnippetBlock.js @@ -1,6 +1,7 @@ /** @import { BlockStatement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types.js' */ +import { dev } from '../../../../state.js'; import * as b from '../../../../utils/builders.js'; /** @@ -13,7 +14,9 @@ export function SnippetBlock(node, context) { [b.id('$$payload'), ...node.parameters], /** @type {BlockStatement} */ (context.visit(node.body)) ); - + if (dev) { + fn.body.body.unshift(b.stmt(b.call('$.validate_snippet_args', b.id('$$payload')))); + } // @ts-expect-error - TODO remove this hack once $$render_inner for legacy bindings is gone fn.___snippet = true; diff --git a/packages/svelte/src/internal/client/dev/validation.js b/packages/svelte/src/internal/client/dev/validation.js new file mode 100644 index 000000000000..bfb81d81da2e --- /dev/null +++ b/packages/svelte/src/internal/client/dev/validation.js @@ -0,0 +1,15 @@ +import { invalid_snippet_arguments } from '../../shared/errors'; +/** + * @param {Node} anchor + * @param {...(()=>any)[]} args + */ +export function validate_snippet_args(anchor, ...args) { + if (typeof anchor !== 'object' || !(anchor instanceof Node)) { + invalid_snippet_arguments(); + } + for (let arg of args) { + if (typeof arg !== 'function') { + invalid_snippet_arguments(); + } + } +} diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 31da00dbb448..e833934634e1 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -16,6 +16,7 @@ export { export { check_target, legacy_api } from './dev/legacy.js'; export { trace } from './dev/tracing.js'; export { inspect } from './dev/inspect.js'; +export { validate_snippet_args } from './dev/validation.js'; export { await_block as await } from './dom/blocks/await.js'; export { if_block as if } from './dom/blocks/if.js'; export { key_block as key } from './dom/blocks/key.js'; diff --git a/packages/svelte/src/internal/server/dev.js b/packages/svelte/src/internal/server/dev.js index ecf4e67429ac..2a51d4a7c5c6 100644 --- a/packages/svelte/src/internal/server/dev.js +++ b/packages/svelte/src/internal/server/dev.js @@ -5,6 +5,7 @@ import { is_tag_valid_with_parent } from '../../html-tree-validation.js'; import { current_component } from './context.js'; +import { invalid_snippet_arguments } from '../shared/errors'; /** * @typedef {{ @@ -98,3 +99,12 @@ export function push_element(payload, tag, line, column) { export function pop_element() { parent = /** @type {Element} */ (parent).parent; } + +/** + * @param {Payload} payload + */ +export function validate_snippet_args(payload) { + if (typeof payload !== 'object' || !('out' in payload)) { + invalid_snippet_arguments(); + } +} diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 6098b496c5ac..a5c78f8eb600 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -541,7 +541,7 @@ export { html } from './blocks/html.js'; export { push, pop } from './context.js'; -export { push_element, pop_element } from './dev.js'; +export { push_element, pop_element, validate_snippet_args } from './dev.js'; export { snapshot } from '../shared/clone.js'; diff --git a/packages/svelte/src/internal/shared/errors.js b/packages/svelte/src/internal/shared/errors.js index 26d6822cdb29..7d31d6e91049 100644 --- a/packages/svelte/src/internal/shared/errors.js +++ b/packages/svelte/src/internal/shared/errors.js @@ -17,6 +17,21 @@ export function invalid_default_snippet() { } } +/** + * A snippet function was passed invalid arguments. A snippet function should only be called via `{@render ...}` + * @returns {never} + */ +export function invalid_snippet_arguments() { + if (DEV) { + const error = new Error(`invalid_snippet_arguments\nA snippet function was passed invalid arguments. A snippet function should only be called via \`{@render ...}\`\nhttps://svelte.dev/e/invalid_snippet_arguments`); + + error.name = 'Svelte error'; + throw error; + } else { + throw new Error(`https://svelte.dev/e/invalid_snippet_arguments`); + } +} + /** * `%name%(...)` can only be used during component initialisation * @param {string} name From 926331f5d706fcd452759df314db33090e54ff96 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Sat, 15 Mar 2025 13:36:29 -0700 Subject: [PATCH 02/10] fix --- packages/svelte/src/internal/client/dev/validation.js | 2 +- packages/svelte/src/internal/server/dev.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/dev/validation.js b/packages/svelte/src/internal/client/dev/validation.js index bfb81d81da2e..e41e4c46283d 100644 --- a/packages/svelte/src/internal/client/dev/validation.js +++ b/packages/svelte/src/internal/client/dev/validation.js @@ -1,4 +1,4 @@ -import { invalid_snippet_arguments } from '../../shared/errors'; +import { invalid_snippet_arguments } from '../../shared/errors.js'; /** * @param {Node} anchor * @param {...(()=>any)[]} args diff --git a/packages/svelte/src/internal/server/dev.js b/packages/svelte/src/internal/server/dev.js index 2a51d4a7c5c6..73c5beb62057 100644 --- a/packages/svelte/src/internal/server/dev.js +++ b/packages/svelte/src/internal/server/dev.js @@ -5,7 +5,7 @@ import { is_tag_valid_with_parent } from '../../html-tree-validation.js'; import { current_component } from './context.js'; -import { invalid_snippet_arguments } from '../shared/errors'; +import { invalid_snippet_arguments } from '../shared/errors.js'; /** * @typedef {{ From 6f64812ea14ee40c94c326353fbd0165f38f8c10 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Mon, 17 Mar 2025 17:39:58 -0700 Subject: [PATCH 03/10] make `Payload` a class --- .../src/internal/server/blocks/snippet.js | 2 +- packages/svelte/src/internal/server/dev.js | 5 +-- packages/svelte/src/internal/server/index.js | 31 +++++++++++++------ .../svelte/src/internal/server/types.d.ts | 13 -------- 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/packages/svelte/src/internal/server/blocks/snippet.js b/packages/svelte/src/internal/server/blocks/snippet.js index 3c5e8607905b..466de1926adc 100644 --- a/packages/svelte/src/internal/server/blocks/snippet.js +++ b/packages/svelte/src/internal/server/blocks/snippet.js @@ -1,5 +1,5 @@ /** @import { Snippet } from 'svelte' */ -/** @import { Payload } from '#server' */ +/** @import { Payload } from '../index' */ /** @import { Getters } from '#shared' */ /** diff --git a/packages/svelte/src/internal/server/dev.js b/packages/svelte/src/internal/server/dev.js index 73c5beb62057..2df8e1133310 100644 --- a/packages/svelte/src/internal/server/dev.js +++ b/packages/svelte/src/internal/server/dev.js @@ -1,4 +1,4 @@ -/** @import { Component, Payload } from '#server' */ +/** @import { Component } from '#server' */ import { FILENAME } from '../../constants.js'; import { is_tag_valid_with_ancestor, @@ -6,6 +6,7 @@ import { } from '../../html-tree-validation.js'; import { current_component } from './context.js'; import { invalid_snippet_arguments } from '../shared/errors.js'; +import { Payload } from './index.js'; /** * @typedef {{ @@ -104,7 +105,7 @@ export function pop_element() { * @param {Payload} payload */ export function validate_snippet_args(payload) { - if (typeof payload !== 'object' || !('out' in payload)) { + if (typeof payload !== 'object' || !(payload instanceof Payload)) { invalid_snippet_arguments(); } } diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index a5c78f8eb600..c6248c619d14 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -1,5 +1,5 @@ /** @import { ComponentType, SvelteComponent } from 'svelte' */ -/** @import { Component, Payload, RenderOutput } from '#server' */ +/** @import { Component, RenderOutput } from '#server' */ /** @import { Store } from '#shared' */ export { FILENAME, HMR } from '../../constants.js'; import { attr, clsx, to_class, to_style } from '../shared/attributes.js'; @@ -96,6 +96,24 @@ function props_id_generator(prefix) { return () => `${prefix}s${uid++}`; } +class Payload { + out = ''; + /**@type {Set<{ hash: string; code: string }>} */ + css = new Set(); + uid = () => ''; + head = { + /**@type {Set<{ hash: string; code: string }>} */ + css: new Set(), + title: '', + out: '', + uid: () => '' + }; + constructor(idPrefix = '') { + this.uid = props_id_generator(idPrefix); + this.head.uid = this.uid; + } +} + /** * Only available on the server and when compiling with the `server` option. * Takes a component and returns an object with `body` and `head` properties on it, which you can use to populate the HTML when server-rendering your app. @@ -105,14 +123,7 @@ function props_id_generator(prefix) { * @returns {RenderOutput} */ export function render(component, options = {}) { - const uid = props_id_generator(options.idPrefix ? options.idPrefix + '-' : ''); - /** @type {Payload} */ - const payload = { - out: '', - css: new Set(), - head: { title: '', out: '', css: new Set(), uid }, - uid - }; + const payload = new Payload(options.idPrefix ? options.idPrefix + '' : ''); const prev_on_destroy = on_destroy; on_destroy = []; @@ -535,7 +546,7 @@ export function props_id(payload) { return uid; } -export { attr, clsx }; +export { attr, clsx, Payload }; export { html } from './blocks/html.js'; diff --git a/packages/svelte/src/internal/server/types.d.ts b/packages/svelte/src/internal/server/types.d.ts index 2fffdbbdf0bb..6b0fc146c419 100644 --- a/packages/svelte/src/internal/server/types.d.ts +++ b/packages/svelte/src/internal/server/types.d.ts @@ -11,19 +11,6 @@ export interface Component { function?: any; } -export interface Payload { - out: string; - css: Set<{ hash: string; code: string }>; - head: { - title: string; - out: string; - uid: () => string; - css: Set<{ hash: string; code: string }>; - }; - /** Function that generates a unique ID */ - uid: () => string; -} - export interface RenderOutput { /** HTML that goes into the `
` */ head: string; From af766dfa40c179d67cc567794dcf5f1e74732f6e Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Mon, 17 Mar 2025 17:42:24 -0700 Subject: [PATCH 04/10] doh --- packages/svelte/src/internal/server/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index c6248c619d14..633fc4805b2b 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -123,7 +123,7 @@ class Payload { * @returns {RenderOutput} */ export function render(component, options = {}) { - const payload = new Payload(options.idPrefix ? options.idPrefix + '' : ''); + const payload = new Payload(options.idPrefix ? options.idPrefix + '-' : ''); const prev_on_destroy = on_destroy; on_destroy = []; From 805fe77a3dfdc217f0b8a723d932e8428ddccb6e Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Mon, 17 Mar 2025 17:45:57 -0700 Subject: [PATCH 05/10] lint --- packages/svelte/src/internal/server/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 633fc4805b2b..00a7977c3456 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -108,8 +108,8 @@ class Payload { out: '', uid: () => '' }; - constructor(idPrefix = '') { - this.uid = props_id_generator(idPrefix); + constructor(id_prefix = '') { + this.uid = props_id_generator(id_prefix); this.head.uid = this.uid; } } From 31b4bb6bb30d347408f6fcf19b2c0ed117c4ddf1 Mon Sep 17 00:00:00 2001 From: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com> Date: Tue, 1 Apr 2025 08:14:42 -0700 Subject: [PATCH 06/10] tweak changeset --- .changeset/bright-jeans-compare.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/bright-jeans-compare.md b/.changeset/bright-jeans-compare.md index 206fecdecfbf..eaec658e0343 100644 --- a/.changeset/bright-jeans-compare.md +++ b/.changeset/bright-jeans-compare.md @@ -1,5 +1,5 @@ --- -'svelte': minor +'svelte': patch --- fix: add snippet argument validation in dev From eaca5268ab40412a2bfc894c97fc7c53f2a8780f Mon Sep 17 00:00:00 2001 From: Rich Harris