Skip to content

fix: remove runtime validation of components/snippets, rely on types instead #12507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jul 23, 2024
Merged
5 changes: 5 additions & 0 deletions .changeset/moody-lions-watch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: remove runtime validation of components/snippets, rely on types instead
8 changes: 0 additions & 8 deletions packages/svelte/messages/shared-errors/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/** @import { BlockStatement, CallExpression, Expression, ExpressionStatement, Identifier, Literal, MemberExpression, ObjectExpression, Pattern, Property, Statement, Super, TemplateElement, TemplateLiteral } from 'estree' */
/** @import { BindDirective } from '#compiler' */
/** @import { ComponentClientTransformState } from '../types' */
import {
extract_identifiers,
extract_paths,
Expand Down Expand Up @@ -929,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) {
Expand Down Expand Up @@ -1868,9 +1861,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);
}

if (node.metadata.dynamic) {
context.state.init.push(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -966,13 +966,7 @@ function serialize_inline_component(node, expression, context) {
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
)
);
push_prop(b.prop('init', b.id('children'), slot_fn));

// and `$$slots.default: true` so that `<slot>` on the child works
serialized_slots.push(b.init(slot_name, b.true));
Expand Down Expand Up @@ -1004,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
)
Expand Down Expand Up @@ -1212,10 +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)
: 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));
Expand Down Expand Up @@ -1498,10 +1489,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);
Expand Down
13 changes: 12 additions & 1 deletion packages/svelte/src/index.d.ts
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand Down Expand Up @@ -104,6 +105,15 @@ export class SvelteComponent<
$set(props: Partial<Props>): void;
}

declare const brand: unique symbol;
type Brand<B> = { [brand]: B };
type Branded<T, B> = T & Brand<B>;

/**
* Internal implementation details that vary between environments
*/
export type ComponentInternals = Branded<{}, 'ComponentInternals'>;

/**
* Can be used to create strongly typed Svelte components.
*
Expand Down Expand Up @@ -136,7 +146,8 @@ export interface Component<
* @param props The props passed to the component.
*/
(
internal: unknown,
this: void,
internals: ComponentInternals,
props: Props
): {
/**
Expand Down
54 changes: 27 additions & 27 deletions packages/svelte/src/internal/client/dom/blocks/snippet.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -55,7 +54,7 @@ export function snippet(node, get_snippet, ...args) {
* @param {(node: TemplateNode, ...args: any[]) => void} fn
*/
export function wrap_snippet(component, fn) {
return add_snippet_symbol((/** @type {TemplateNode} */ node, /** @type {any[]} */ ...args) => {
return (/** @type {TemplateNode} */ node, /** @type {any[]} */ ...args) => {
var previous_component_function = dev_current_component_function;
set_dev_current_component_function(component);

Expand All @@ -64,7 +63,7 @@ export function wrap_snippet(component, fn) {
} finally {
set_dev_current_component_function(previous_component_function);
}
});
};
}

/**
Expand All @@ -77,32 +76,33 @@ export function wrap_snippet(component, fn) {
* @returns {Snippet<Params>}
*/
export function createRawSnippet(fn) {
return add_snippet_symbol(
(/** @type {TemplateNode} */ anchor, /** @type {Getters<Params>} */ ...params) => {
var snippet = fn(...params);

/** @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);
if (DEV && (element.nextSibling !== null || element.nodeType !== 1)) {
w.invalid_raw_snippet_render();
}
anchor.before(element);
}
// @ts-expect-error the types are a lie
return (/** @type {TemplateNode} */ anchor, /** @type {Getters<Params>} */ ...params) => {
var snippet = fn(...params);

/** @type {Element} */
var element;

const result = snippet.setup?.(element);
assign_nodes(element, 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);

if (typeof result === 'function') {
teardown(result);
if (DEV && (element.nextSibling !== null || element.nodeType !== 3)) {
w.invalid_raw_snippet_render();
}

anchor.before(element);
}

const result = snippet.setup?.(element);
assign_nodes(element, element);

if (typeof result === 'function') {
teardown(result);
}
);
};
}
2 changes: 0 additions & 2 deletions packages/svelte/src/internal/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,7 @@ export { snapshot } from '../shared/clone.js';
export { noop } from '../shared/utils.js';
export {
invalid_default_snippet,
validate_component,
validate_dynamic_element_tag,
validate_snippet,
validate_store,
validate_void_dynamic_element
} from '../shared/validate.js';
Expand Down
9 changes: 0 additions & 9 deletions packages/svelte/src/internal/client/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions packages/svelte/src/internal/server/blocks/snippet.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -13,10 +12,11 @@ import { add_snippet_symbol } from '../../shared/validate.js';
* @returns {Snippet<Params>}
*/
export function createRawSnippet(fn) {
return add_snippet_symbol((/** @type {Payload} */ payload, /** @type {Params} */ ...args) => {
// @ts-expect-error the types are a lie
return (/** @type {Payload} */ payload, /** @type {Params} */ ...args) => {
var getters = /** @type {Getters<Params>} */ (args.map((value) => () => value));
payload.out += fn(...getters)
.render()
.trim();
});
};
}
3 changes: 0 additions & 3 deletions packages/svelte/src/internal/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -555,11 +555,8 @@ export { push_element, pop_element } from './dev.js';
export { snapshot } from '../shared/clone.js';

export {
add_snippet_symbol,
invalid_default_snippet,
validate_component,
validate_dynamic_element_tag,
validate_snippet,
validate_void_dynamic_element
} from '../shared/validate.js';

Expand Down
32 changes: 0 additions & 32 deletions packages/svelte/src/internal/shared/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,38 +35,6 @@ export function lifecycle_outside_component(name) {
}
}

/**
* 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 ...}`
* @returns {never}
*/
export function render_tag_invalid_argument() {
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 ...}\``);

error.name = 'Svelte error';
throw error;
} else {
// TODO print a link to the documentation
throw new Error("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
Expand Down
38 changes: 1 addition & 37 deletions packages/svelte/src/internal/shared/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,43 +4,7 @@ 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');

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;
}

/**
* Validate that the function handed to `{@render ...}` is a snippet function, and not some other kind of function.
* @param {any} snippet_fn
*/
export function validate_snippet(snippet_fn) {
if (snippet_fn && snippet_fn[snippet_symbol] !== true) {
e.render_tag_invalid_argument();
}

return snippet_fn;
}

/**
* Validate that the function behind `<Component />` isn't a snippet.
* @param {any} component_fn
*/
export function validate_component(component_fn) {
if (component_fn?.[snippet_symbol] === true) {
e.snippet_used_as_component();
}

return component_fn;
}
export { invalid_default_snippet } from './errors.js';

/**
* @param {() => string} tag_fn
Expand Down

This file was deleted.

Loading
Loading