Skip to content

fix: add symbol to components to properly validate them #12452

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

Closed
wants to merge 8 commits into from
5 changes: 5 additions & 0 deletions .changeset/gold-pumpkins-destroy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: add symbol to components to properly validate them
4 changes: 0 additions & 4 deletions packages/svelte/messages/client-errors/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,3 @@
## state_unsafe_mutation

> Updating state inside a derived is forbidden. If the value should not be reactive, declare it without `$state`

## svelte_component_invalid_this_value

> The `this={...}` property of a `<svelte:component>` must be a Svelte component, if defined
8 changes: 8 additions & 0 deletions packages/svelte/messages/shared-errors/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

> `%name%(...)` can only be used during component initialisation

## not_a_svelte_component

> You are trying to render something that is not a Svelte component

## 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 ...}`
Expand All @@ -14,6 +18,10 @@

> `%name%` is not a store with a `subscribe` method

## svelte_component_invalid_this_value

> The `this={...}` property of a `<svelte:component>` must be a Svelte component, if defined

## svelte_element_invalid_this_value

> The `this` prop on `<svelte:element>` must be a string, if defined
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,14 @@ export function client_component(source, analysis, options) {
b.id('$$original')
)
),
// mark the HMR'd component as IS_COMPONENT as well
b.stmt(
b.assignment(
'=',
b.member(b.id(analysis.name), b.id('$.IS_COMPONENT'), true),
b.literal(true)
)
),
b.stmt(b.call('import.meta.hot.accept', b.arrow([b.id('module')], b.block(accept_fn_body))))
]);

Expand All @@ -473,7 +481,16 @@ export function client_component(source, analysis, options) {
)
);
}

// add `App[$.IS_COMPONENT] = true` to easily check if it's actually a component
body.unshift(
b.stmt(
b.assignment(
'=',
b.member(b.id(analysis.name), b.id('$.IS_COMPONENT'), true),
b.literal(true)
)
)
);
body.unshift(b.stmt(b.call(b.id('$.mark_module_start'))));
body.push(b.stmt(b.call(b.id('$.mark_module_end'), b.id(analysis.name))));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,11 @@ function serialize_inline_component(node, component_name, context, anchor = cont
let fn = (node_id) => {
return b.call(
context.state.options.dev
? b.call('$.validate_component', b.id(component_name))
? b.call(
'$.validate_component',
b.id(component_name),
node.type === 'SvelteComponent' ? b.literal(true) : b.literal(false)
)
: component_name,
node_id,
props_expression
Expand Down Expand Up @@ -954,14 +958,7 @@ function serialize_inline_component(node, component_name, context, anchor = cont
b.thunk(/** @type {Expression} */ (context.visit(node.expression))),
b.arrow(
[b.id('$$anchor'), b.id(component_name)],
b.block([
...binding_initializers,
b.stmt(
context.state.options.dev
? b.call('$.validate_dynamic_component', b.thunk(prev(b.id('$$anchor'))))
: prev(b.id('$$anchor'))
)
])
b.block([...binding_initializers, b.stmt(prev(b.id('$$anchor')))])
)
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,13 @@ 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,
context.state.options.dev
? b.call(
'$.validate_component',
expression,
node.type === 'SvelteComponent' ? b.literal(true) : b.literal(false)
)
: expression,
b.id('$$payload'),
props_expression
)
Expand Down Expand Up @@ -2259,6 +2265,15 @@ export function server_component(analysis, options) {
)
)
);
body.unshift(
b.stmt(
b.assignment(
'=',
b.member(b.id(analysis.name), b.id('$.IS_COMPONENT'), true),
b.literal(true)
)
)
);
}

return {
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const UNINITIALIZED = Symbol();
// Dev-time component properties
export const FILENAME = Symbol('filename');
export const ORIGINAL = Symbol('original');
export const IS_COMPONENT = Symbol('is_component');

/** List of elements that require raw contents and should not have SSR comments put in them */
export const RawTextElements = ['textarea', 'script', 'style', 'title'];
Expand Down
16 changes: 0 additions & 16 deletions packages/svelte/src/internal/client/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,20 +308,4 @@ export function state_unsafe_mutation() {
// TODO print a link to the documentation
throw new Error("state_unsafe_mutation");
}
}

/**
* The `this={...}` property of a `<svelte:component>` must be a Svelte component, if defined
* @returns {never}
*/
export function svelte_component_invalid_this_value() {
if (DEV) {
const error = new Error(`svelte_component_invalid_this_value\nThe \`this={...}\` property of a \`<svelte:component>\` must be a Svelte component, if defined`);

error.name = 'Svelte error';
throw error;
} else {
// TODO print a link to the documentation
throw new Error("svelte_component_invalid_this_value");
}
}
8 changes: 2 additions & 6 deletions packages/svelte/src/internal/client/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { FILENAME, ORIGINAL } from '../../constants.js';
export { FILENAME, ORIGINAL, IS_COMPONENT } from '../../constants.js';
export { add_locations } from './dev/elements.js';
export { hmr } from './dev/hmr.js';
export {
Expand Down Expand Up @@ -145,11 +145,7 @@ export {
setContext,
hasContext
} from './runtime.js';
export {
validate_dynamic_component,
validate_each_keys,
validate_prop_bindings
} from './validate.js';
export { validate_each_keys, validate_prop_bindings } from './validate.js';
export { raf } from './timing.js';
export { proxy, is } from './proxy.js';
export { create_custom_element } from './dom/elements/custom-element.js';
Expand Down
6 changes: 3 additions & 3 deletions packages/svelte/src/internal/client/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ 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 { validate_component_is_not_a_snippet } from '../shared/validate.js';
import { assign_nodes } from './dom/template.js';

/**
Expand Down Expand Up @@ -80,7 +80,7 @@ export function set_text(text, value) {
*/
export function mount(component, options) {
if (DEV) {
validate_component(component);
validate_component_is_not_a_snippet(component);
}

const anchor = options.anchor ?? options.target.appendChild(empty());
Expand Down Expand Up @@ -113,7 +113,7 @@ export function mount(component, options) {
*/
export function hydrate(component, options) {
if (DEV) {
validate_component(component);
validate_component_is_not_a_snippet(component);
}

options.intro = options.intro ?? false;
Expand Down
25 changes: 0 additions & 25 deletions packages/svelte/src/internal/client/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,6 @@ function is_void(tag) {
return void_element_names.test(tag) || tag.toLowerCase() === '!doctype';
}

/**
* @template Component
* @param {() => Component} component_fn
* @returns {Component}
*/
export function validate_dynamic_component(component_fn) {
try {
const instance = component_fn();

if (instance !== undefined && typeof instance !== 'object') {
e.svelte_component_invalid_this_value();
}

return instance;
} catch (err) {
const { message } = /** @type {Error} */ (err);

if (typeof message === 'string' && message.indexOf('is not a function') !== -1) {
e.svelte_component_invalid_this_value();
}

throw err;
}
}

/**
* @param {() => any} collection
* @param {(item: any, index: number) => string} key_fn
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/internal/server/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/** @import { Component, Payload, RenderOutput } from '#server' */
/** @import { Store } from '#shared' */
export { FILENAME, ORIGINAL } from '../../constants.js';
export { FILENAME, ORIGINAL, IS_COMPONENT } from '../../constants.js';
import { is_promise, noop } from '../shared/utils.js';
import { subscribe_to_store } from '../../store/utils.js';
import {
Expand Down
32 changes: 32 additions & 0 deletions packages/svelte/src/internal/shared/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@ export function lifecycle_outside_component(name) {
}
}

/**
* You are trying to render something that is not a Svelte component
* @returns {never}
*/
export function not_a_svelte_component() {
if (DEV) {
const error = new Error(`not_a_svelte_component\nYou are trying to render something that is not a Svelte component`);

error.name = 'Svelte error';
throw error;
} else {
// TODO print a link to the documentation
throw new Error("not_a_svelte_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 ...}`
* @returns {never}
Expand Down Expand Up @@ -68,6 +84,22 @@ export function store_invalid_shape(name) {
}
}

/**
* The `this={...}` property of a `<svelte:component>` must be a Svelte component, if defined
* @returns {never}
*/
export function svelte_component_invalid_this_value() {
if (DEV) {
const error = new Error(`svelte_component_invalid_this_value\nThe \`this={...}\` property of a \`<svelte:component>\` must be a Svelte component, if defined`);

error.name = 'Svelte error';
throw error;
} else {
// TODO print a link to the documentation
throw new Error("svelte_component_invalid_this_value");
}
}

/**
* The `this` prop on `<svelte:element>` must be a string, if defined
* @returns {never}
Expand Down
32 changes: 30 additions & 2 deletions packages/svelte/src/internal/shared/validate.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/** @import { TemplateNode } from '#client' */
/** @import { Getters } from '#shared' */
import { is_void } from '../../constants.js';
import { is_void, IS_COMPONENT } from '../../constants.js';
import * as w from './warnings.js';
import * as e from './errors.js';

Expand Down Expand Up @@ -35,10 +35,38 @@ export function validate_snippet(snippet_fn, $$props) {
* Validate that the function behind `<Component />` isn't a snippet.
* @param {any} component_fn
*/
export function validate_component(component_fn) {
export function validate_component_is_not_a_snippet(component_fn) {
if (component_fn?.[snippet_symbol] === true) {
e.snippet_used_as_component();
}
}

/**
* Validate that the function behind `<Component />` is a valid svelte component.
*
* If `dynamic` is true checks that `component_fn` isn't a snippet and that is actually a svelte component
* If `dynamic` is false checks that `component_fn` isn't a snippet, is not undefined and it's actually a svelte component
* @param {any} component_fn
* @param {boolean} dynamic
*/
export function validate_component(component_fn, dynamic) {
validate_component_is_not_a_snippet(component_fn);

// this is the case for both `<svelte:component>` with a wrong this and `<Something >`
// where Something is not a svelte component. In this case `component_fn` needs to be
// defined for the error to throw because you can do `<svelte:component this={undefined} />`
if (component_fn && !(IS_COMPONENT in component_fn)) {
if (dynamic) {
e.svelte_component_invalid_this_value();
}
e.not_a_svelte_component();
}

// if dynamic is false and component_fn is undefined we still throw to cover
// the case of `<Something />` where Something = undefined
if (!dynamic && !component_fn) {
e.not_a_svelte_component();
}

return component_fn;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
let { prop } = $props();
</script>

{prop}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { test } from '../../test';

export default test({
expect_unhandled_rejections: true,
error: 'is not a function',
compileOptions: {
dev: true
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<script>
import A from "./A.svelte";
</script>

{#snippet snip(comp, func)}
<svelte:component this={comp} prop={func(10)}></svelte:component>
{/snippet}

{@render snip(A)}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { test } from '../../test';

export default test({
compileOptions: {
dev: true
},
error: 'You are trying to render something that is not a Svelte component'
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
let Foo = undefined;
</script>

<Foo />
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { test } from '../../test';

export default test({
compileOptions: {
dev: true
},
error: 'You are trying to render something that is not a Svelte component'
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
let Foo = {};
</script>

<Foo />
Loading
Loading