msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

breaking: disallow binding to component exports in runes mode (#11238)

Browse Source 
* breaking: disallow binding to component exports in runes mode

Svelte 4 allowed you to have `export const foo = ..` in component A and then do `<A bind:foo />`. This is confusing because it's not clear whether the binding is for a property or an export, and we have to sanitize rest props from the export bindings.
This PR therefore introduces a breaking change in runes mode: You cannot bind to these exports anymore. Instead use `<A bind:this={a} />` and then do `a.foo` - makes things easier to reason about.

* Update sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md

Co-authored-by: Rich Harris <rich.harris@vercel.com>

* tweak messages

* fix tests

* use component.name

* oops

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
pull/11296/head
Simon H 1 year ago committed by GitHub
parent e3c8589737
commit 2d378bb762
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 59 additions and 60 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/beige-seas-share.md
Unescape View File

@ -0,0 +1,5 @@
---
"svelte": patch
---
breaking: disallow binding to component exports in runes mode

14
packages/svelte/src/compiler/phases/3-transform/client/transform-client.js
Unescape View File

@ -255,14 +255,24 @@ export function client_component(source, analysis, options) {
);
if (analysis.runes && options.dev) {
const bindable = analysis.exports.map(({ name, alias }) => b.literal(alias ?? name));
const exports = analysis.exports.map(({ name, alias }) => b.literal(alias ?? name));
/** @type {import('estree').Literal[]} */
const bindable = [];
for (const [name, binding] of properties) {
if (binding.kind === 'bindable_prop') {
bindable.push(b.literal(binding.prop_alias ?? name));
}
}
instance.body.unshift(
b.stmt(b.call('$.validate_prop_bindings', b.id('$$props'), b.array(bindable)))
b.stmt(
b.call(
'$.validate_prop_bindings',
b.id('$$props'),
b.array(bindable),
b.array(exports),
b.id(`${analysis.name}`)
)
)
);
}

3
packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js
Unescape View File

@ -222,7 +222,8 @@ export const javascript_visitors_runes = {
if (rune === '$props') {
assert.equal(declarator.id.type, 'ObjectPattern');
const seen = state.analysis.exports.map(({ name, alias }) => alias ?? name);
/** @type {string[]} */
const seen = [];
for (const property of declarator.id.properties) {
if (property.type === 'Property') {

22
packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
Unescape View File

@ -692,8 +692,7 @@ const javascript_visitors_runes = {
}
if (rune === '$props') {
// remove $bindable() from props declaration and handle rest props
let uses_rest_props = false;
// remove $bindable() from props declaration
const id = walk(declarator.id, null, {
AssignmentPattern(node) {
if (
@ -705,26 +704,9 @@ const javascript_visitors_runes = {
: b.id('undefined');
return b.assignment_pattern(node.left, right);
}
},
RestElement(node, { path }) {
if (path.at(-1) === declarator.id) {
uses_rest_props = true;
}
}
});
const exports = /** @type {import('../../types').ComponentAnalysis} */ (
state.analysis
).exports.map(({ name, alias }) => b.literal(alias ?? name));
declarations.push(
b.declarator(
id,
uses_rest_props && exports.length > 0
? b.call('$.rest_props', b.id('$$props'), b.array(exports))
: b.id('$$props')
)
);
declarations.push(b.declarator(id, b.id('$$props')));
continue;
}

18
packages/svelte/src/internal/client/validate.js
Unescape View File

@ -85,16 +85,26 @@ export function loop_guard(timeout) {
/**
* @param {Record<string, any>} $$props
* @param {string[]} bindable
* @param {string[]} exports
* @param {Function & { filename: string }} component
*/
export function validate_prop_bindings($$props, bindable) {
export function validate_prop_bindings($$props, bindable, exports, component) {
for (const key in $$props) {
if (!bindable.includes(key)) {
var setter = get_descriptor($$props, key)?.set;
var name = component.name;
if (setter) {
if (exports.includes(key)) {
throw new Error(
`Component ${component.filename} has an export named ${key} that a consumer component is trying to access using bind:${key}, which is disallowed. ` +
`Instead, use bind:this (e.g. <${name} bind:this={component} />) ` +
`and then access the property on the bound component instance (e.g. component.${key}).`
);
}
if (!bindable.includes(key)) {
throw new Error(
`Cannot use bind:${key} on this component because the property was not declared as bindable. ` +
`To mark a property as bindable, use the $bindable() rune like this: \`let { ${key} = $bindable() } = $props()\``
`A component is binding to property ${key} of ${name}.svelte (i.e. <${name} bind:${key} />). This is disallowed because the property was not declared as bindable inside ${component.filename}. ` +
`To mark a property as bindable, use the $bindable() rune in ${name}.svelte like this: \`let { ${key} = $bindable() } = $props()\``
);
}
}

12
packages/svelte/tests/runtime-runes/samples/export-binding/Counter.svelte
Unescape View File

@ -1,12 +0,0 @@
<script>
let { ...rest } = $props();
let count = $state(0);
export function increment() {
count++;
}
</script>
<!-- test that the binding isn't inside rest -->
{Object.keys(rest).length}
{count}

15
packages/svelte/tests/runtime-runes/samples/export-binding/_config.js
Unescape View File

@ -2,16 +2,9 @@ import { test } from '../../test';
export default test({
compileOptions: {
dev: true // to ensure we don't throw a false-positive "cannot bind to this" error
dev: true // to ensure we we catch the error
},
html: `0 0 <button>increment</button>`,
async test({ assert, target }) {
const btn = target.querySelector('button');
btn?.click();
await Promise.resolve();
assert.htmlEqual(target.innerHTML, `0 1 <button>increment</button>`);
}
error:
'Component .../export-binding/counter/index.svelte has an export named increment that a consumer component is trying to access using bind:increment, which is disallowed. ' +
'Instead, use bind:this (e.g. <Counter bind:this={component} />) and then access the property on the bound component instance (e.g. component.increment).'
});

8
packages/svelte/tests/runtime-runes/samples/export-binding/counter/index.svelte
Unescape View File

@ -0,0 +1,8 @@
<script>
let count = $state(0);
export function increment() {
count++;
}
</script>
{count}

2
packages/svelte/tests/runtime-runes/samples/export-binding/main.svelte
Unescape View File

@ -1,5 +1,5 @@
<script>
import Counter from './Counter.svelte';
import Counter from './counter/index.svelte';
let increment;
</script>

5
packages/svelte/tests/runtime-runes/samples/props-not-bindable-spread/_config.js
Unescape View File

@ -5,7 +5,8 @@ export default test({
dev: true
},
error:
'Cannot use bind:count on this component because the property was not declared as bindable. ' +
'To mark a property as bindable, use the $bindable() rune like this: `let { count = $bindable() } = $props()`',
'A component is binding to property count of Counter.svelte (i.e. <Counter bind:count />). This is disallowed because the property was ' +
'not declared as bindable inside .../samples/props-not-bindable-spread/Counter.svelte. To mark a property as bindable, use the $bindable() rune ' +
'in Counter.svelte like this: `let { count = $bindable() } = $props()`',
html: `0`
});

5
packages/svelte/tests/runtime-runes/samples/props-not-bindable/_config.js
Unescape View File

@ -5,7 +5,8 @@ export default test({
dev: true
},
error:
'Cannot use bind:count on this component because the property was not declared as bindable. ' +
'To mark a property as bindable, use the $bindable() rune like this: `let { count = $bindable() } = $props()`',
'A component is binding to property count of Counter.svelte (i.e. <Counter bind:count />). This is disallowed because the property was ' +
'not declared as bindable inside .../samples/props-not-bindable/Counter.svelte. To mark a property as bindable, use the $bindable() rune ' +
'in Counter.svelte like this: `let { count = $bindable() } = $props()`',
html: `0`
});

4
sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md
Unescape View File

@ -121,9 +121,9 @@ Content inside component tags becomes a [snippet prop](/docs/snippets) called `c
Some breaking changes only apply once your component is in runes mode.
### Bindings to component exports don't show up in rest props
### Bindings to component exports are not allowed
In runes mode, bindings to component exports don't show up in rest props. For example, `rest` in `let { foo, bar, ...rest } = $props();` would not contain `baz` if `baz` was defined as `export const baz = ...;` inside the component. In Svelte 4 syntax, the equivalent to `rest` would be `$$restProps`, which contains these component exports.
Exports from runes mode components cannot be bound to directly. For example, having `export const foo = ...` in component `A` and then doing `<A bind:foo />` causes an error. Use `bind:this` instead — `<A bind:this={a} />` — and access the export as `a.foo`. This change makes things easier to reason about, as it enforces a clear separation between props and exports.
### Bindings need to be explicitly defined using `$bindable()`

Write Preview
Loading…
Cancel
Save