From a9e15bdf2d2aed6176d2e46f48c7744a2b8bc715 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Sat, 6 Apr 2024 14:36:51 +0100 Subject: [PATCH] breaking: robustify interop of exports and props (#11064) - don't throw a dev time error when binding to an export (fixes #11008) - remove bindings that are for component exports - throw an error when using a component export with the same name as a property --- .changeset/heavy-ducks-leave.md | 5 +++++ packages/svelte/src/compiler/errors.js | 4 +++- .../src/compiler/phases/2-analyze/index.js | 14 ++++++++++++ .../3-transform/client/transform-client.js | 4 +--- .../client/visitors/javascript-runes.js | 3 +-- .../3-transform/server/transform-server.js | 22 +++++++++++++++++-- .../runes-prop-export-conflict/_config.js | 8 +++++++ .../runes-prop-export-conflict/main.svelte | 4 ++++ .../samples/export-binding/Counter.svelte | 12 ++++++++++ .../samples/export-binding/_config.js | 17 ++++++++++++++ .../samples/export-binding/main.svelte | 7 ++++++ .../03-appendix/02-breaking-changes.md | 12 ++++++++++ 12 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 .changeset/heavy-ducks-leave.md create mode 100644 packages/svelte/tests/compiler-errors/samples/runes-prop-export-conflict/_config.js create mode 100644 packages/svelte/tests/compiler-errors/samples/runes-prop-export-conflict/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/export-binding/Counter.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/export-binding/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/export-binding/main.svelte diff --git a/.changeset/heavy-ducks-leave.md b/.changeset/heavy-ducks-leave.md new file mode 100644 index 0000000000..6adf79c667 --- /dev/null +++ b/.changeset/heavy-ducks-leave.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +breaking: robustify interop of exports and props in runes mode diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index 318225be95..4baa1aeaab 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -212,7 +212,9 @@ const runes = { 'duplicate-props-rune': () => `Cannot use $props() more than once`, 'invalid-each-assignment': () => `Cannot reassign or bind to each block argument in runes mode. Use the array and index variables instead (e.g. 'array[i] = value' instead of 'entry = value')`, - 'invalid-derived-call': () => `$derived.call(...) has been replaced with $derived.by(...)` + 'invalid-derived-call': () => `$derived.call(...) has been replaced with $derived.by(...)`, + 'conflicting-property-name': () => + `Cannot have a property and a component export with the same name` }; /** @satisfies {Errors} */ diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 5773a562ee..887c961734 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -437,6 +437,20 @@ export function analyze_component(root, source, options) { merge(set_scope(scopes), validation_runes, runes_scope_tweaker, common_visitors) ); } + + if (analysis.exports.length > 0) { + for (const [_, binding] of instance.scope.declarations) { + if (binding.kind === 'prop' || binding.kind === 'bindable_prop') { + if ( + analysis.exports.some( + ({ alias, name }) => (binding.prop_alias ?? binding.node.name) === (alias ?? name) + ) + ) { + error(binding.node, 'conflicting-property-name'); + } + } + } + } } else { instance.scope.declare(b.id('$$props'), 'bindable_prop', 'synthetic'); instance.scope.declare(b.id('$$restProps'), 'rest_prop', 'synthetic'); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 8c88a405f1..b4a66ea003 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -255,8 +255,7 @@ export function client_component(source, analysis, options) { ); if (analysis.runes && options.dev) { - /** @type {import('estree').Literal[]} */ - const bindable = []; + const bindable = analysis.exports.map(({ name, alias }) => b.literal(alias ?? name)); for (const [name, binding] of properties) { if (binding.kind === 'bindable_prop') { bindable.push(b.literal(binding.prop_alias ?? name)); @@ -382,7 +381,6 @@ export function client_component(source, analysis, options) { ); if (analysis.uses_rest_props) { - /** @type {string[]} */ const named_props = analysis.exports.map(({ name, alias }) => alias ?? name); for (const [name, binding] of analysis.instance.scope.declarations) { if (binding.kind === 'bindable_prop') named_props.push(binding.prop_alias ?? name); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index 25a2564453..f464d71ffb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -195,8 +195,7 @@ export const javascript_visitors_runes = { if (rune === '$props') { assert.equal(declarator.id.type, 'ObjectPattern'); - /** @type {string[]} */ - const seen = []; + const seen = state.analysis.exports.map(({ name, alias }) => alias ?? name); for (const property of declarator.id.properties) { if (property.type === 'Property') { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 19c626ae58..bc90ce95db 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -691,7 +691,8 @@ const javascript_visitors_runes = { } if (rune === '$props') { - // remove $bindable() from props declaration + // remove $bindable() from props declaration and handle rest props + let uses_rest_props = false; const id = walk(declarator.id, null, { AssignmentPattern(node) { if ( @@ -703,9 +704,26 @@ 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; + } } }); - declarations.push(b.declarator(id, b.id('$$props'))); + + 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') + ) + ); continue; } diff --git a/packages/svelte/tests/compiler-errors/samples/runes-prop-export-conflict/_config.js b/packages/svelte/tests/compiler-errors/samples/runes-prop-export-conflict/_config.js new file mode 100644 index 0000000000..a6e4a4c018 --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/runes-prop-export-conflict/_config.js @@ -0,0 +1,8 @@ +import { test } from '../../test'; + +export default test({ + error: { + code: 'conflicting-property-name', + message: 'Cannot have a property and a component export with the same name' + } +}); diff --git a/packages/svelte/tests/compiler-errors/samples/runes-prop-export-conflict/main.svelte b/packages/svelte/tests/compiler-errors/samples/runes-prop-export-conflict/main.svelte new file mode 100644 index 0000000000..0ea9ef8fbb --- /dev/null +++ b/packages/svelte/tests/compiler-errors/samples/runes-prop-export-conflict/main.svelte @@ -0,0 +1,4 @@ + diff --git a/packages/svelte/tests/runtime-runes/samples/export-binding/Counter.svelte b/packages/svelte/tests/runtime-runes/samples/export-binding/Counter.svelte new file mode 100644 index 0000000000..94c822f31f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/export-binding/Counter.svelte @@ -0,0 +1,12 @@ + + + +{Object.keys(rest).length} + +{count} diff --git a/packages/svelte/tests/runtime-runes/samples/export-binding/_config.js b/packages/svelte/tests/runtime-runes/samples/export-binding/_config.js new file mode 100644 index 0000000000..aa41757bd1 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/export-binding/_config.js @@ -0,0 +1,17 @@ +import { test } from '../../test'; + +export default test({ + compileOptions: { + dev: true // to ensure we don't throw a false-positive "cannot bind to this" error + }, + html: `0 0 `, + + async test({ assert, target }) { + const btn = target.querySelector('button'); + + btn?.click(); + await Promise.resolve(); + + assert.htmlEqual(target.innerHTML, `0 1 `); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/export-binding/main.svelte b/packages/svelte/tests/runtime-runes/samples/export-binding/main.svelte new file mode 100644 index 0000000000..89f0aff9a0 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/export-binding/main.svelte @@ -0,0 +1,7 @@ + + + + \ No newline at end of file diff --git a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md index d88a749bc8..3edaac83d6 100644 --- a/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md +++ b/sites/svelte-5-preview/src/routes/docs/content/03-appendix/02-breaking-changes.md @@ -117,6 +117,18 @@ Svelte now use Mutation Observers instead of IFrames to measure dimensions for ` Content inside component tags becomes a [snippet prop](/docs/snippets) called `children`. You cannot have a separate prop by that name. +## Breaking changes in runes mode + +Some breaking changes only apply once your component is in runes mode. + +### Bindings to component exports don't show up in rest props + +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. + +### Bindings need to be explicitly defined using `$bindable()` + +In Svelte 4 syntax, every property (declared via `export let`) is bindable, meaning you can `bind:` to it. In runes mode, properties are not bindable by default: you need to denote bindable props with the [`$bindable`](/docs/runes#$bindable) rune. + ## Other breaking changes ### Stricter `@const` assignment validation