From 2754e4eb398b4bcc132bf55587d6fc892a0bec28 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Mon, 29 Apr 2024 22:50:30 +0200 Subject: [PATCH] fix: handle reassignment of `$$props` and `$$restProps` (#11348) * fix: handle reassignment of `$$props` and `$$restProps` Some libraries assign to properties of `$$props` and `$$restProps`. These were previously resulting in an error but are now handled properly https://github.com/sveltejs/svelte/issues/10359#issuecomment-2080067464 * $$props is coarse grained on updates, so we can simplify this * fix * fix comment --- .changeset/lucky-teachers-exist.md | 5 ++ .../src/compiler/phases/2-analyze/index.js | 2 +- .../3-transform/client/transform-client.js | 8 ++- .../phases/3-transform/client/utils.js | 11 ++-- packages/svelte/src/internal/client/index.js | 1 + .../src/internal/client/reactivity/props.js | 66 ++++++++++++++++++- .../samples/props-reassign/App.svelte | 6 ++ .../samples/props-reassign/_config.js | 35 ++++++++++ .../samples/props-reassign/main.svelte | 7 ++ .../samples/rest-props-reassign/App.svelte | 6 ++ .../samples/rest-props-reassign/_config.js | 35 ++++++++++ .../samples/rest-props-reassign/main.svelte | 7 ++ 12 files changed, 177 insertions(+), 12 deletions(-) create mode 100644 .changeset/lucky-teachers-exist.md create mode 100644 packages/svelte/tests/runtime-legacy/samples/props-reassign/App.svelte create mode 100644 packages/svelte/tests/runtime-legacy/samples/props-reassign/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/props-reassign/main.svelte create mode 100644 packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/App.svelte create mode 100644 packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/main.svelte diff --git a/.changeset/lucky-teachers-exist.md b/.changeset/lucky-teachers-exist.md new file mode 100644 index 0000000000..26d5af6655 --- /dev/null +++ b/.changeset/lucky-teachers-exist.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: handle reassignment of `$$props` and `$$restProps` diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 8d3088806d..562065ec61 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -449,7 +449,7 @@ export function analyze_component(root, source, options) { ); } } else { - instance.scope.declare(b.id('$$props'), 'bindable_prop', 'synthetic'); + instance.scope.declare(b.id('$$props'), 'rest_prop', 'synthetic'); instance.scope.declare(b.id('$$restProps'), 'rest_prop', 'synthetic'); for (const { ast, scope, scopes } of [module, instance, template]) { 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 aabf99d821..6e9c17faab 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 @@ -418,7 +418,7 @@ export function client_component(source, analysis, options) { b.const( '$$restProps', b.call( - '$.rest_props', + '$.legacy_rest_props', b.id('$$sanitized_props'), b.array(named_props.map((name) => b.literal(name))) ) @@ -431,8 +431,12 @@ export function client_component(source, analysis, options) { if (analysis.custom_element) { to_remove.push(b.literal('$$host')); } + component_block.body.unshift( - b.const('$$sanitized_props', b.call('$.rest_props', b.id('$$props'), b.array(to_remove))) + b.const( + '$$sanitized_props', + b.call('$.legacy_rest_props', b.id('$$props'), b.array(to_remove)) + ) ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index e6fdb902f0..cc15c0c2f9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -74,6 +74,11 @@ export function serialize_get_binding(node, state) { return node; } + if (binding.node.name === '$$props') { + // Special case for $$props which only exists in the old world + return b.id('$$sanitized_props'); + } + if (binding.kind === 'store_sub') { return b.call(node); } @@ -83,12 +88,6 @@ export function serialize_get_binding(node, state) { } if (binding.kind === 'prop' || binding.kind === 'bindable_prop') { - if (binding.node.name === '$$props') { - // Special case for $$props which only exists in the old world - // TODO this probably shouldn't have a 'prop' binding kind - return node; - } - if ( state.analysis.accessors || (state.analysis.immutable ? binding.reassigned : binding.mutated) || diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 778215dc41..5f58555929 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -88,6 +88,7 @@ export { mutable_source, mutate, source, set } from './reactivity/sources.js'; export { prop, rest_props, + legacy_rest_props, spread_props, update_pre_prop, update_prop diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index ae5ef9b29b..9934277be4 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -6,9 +6,9 @@ import { PROPS_IS_UPDATED } from '../../../constants.js'; import { get_descriptor, is_function } from '../utils.js'; -import { mutable_source, set } from './sources.js'; +import { mutable_source, set, source } from './sources.js'; import { derived } from './deriveds.js'; -import { get, is_signals_recorded, untrack } from '../runtime.js'; +import { get, is_signals_recorded, untrack, update } from '../runtime.js'; import { safe_equals } from './equality.js'; import { inspect_fn } from '../dev/inspect.js'; import * as e from '../errors.js'; @@ -79,7 +79,67 @@ const rest_props_handler = { * @returns {Record} */ export function rest_props(props, exclude, name) { - return new Proxy(DEV ? { props, exclude, name } : { props, exclude }, rest_props_handler); + return new Proxy( + DEV ? { props, exclude, name, other: {}, to_proxy: [] } : { props, exclude }, + rest_props_handler + ); +} + +/** + * The proxy handler for legacy $$restProps and $$props + * @type {ProxyHandler<{ props: Record, exclude: Array, special: Record unknown>, version: import('./types.js').Source }>}} + */ +const legacy_rest_props_handler = { + get(target, key) { + if (target.exclude.includes(key)) return; + get(target.version); + return key in target.special ? target.special[key]() : target.props[key]; + }, + set(target, key, value) { + if (!(key in target.special)) { + // Handle props that can temporarily get out of sync with the parent + /** @type {Record unknown>} */ + target.special[key] = prop( + { + get [key]() { + return target.props[key]; + } + }, + /** @type {string} */ (key), + PROPS_IS_UPDATED + ); + } + + target.special[key](value); + update(target.version); // $$props is coarse-grained: when $$props.x is updated, usages of $$props.y etc are also rerun + return true; + }, + getOwnPropertyDescriptor(target, key) { + if (target.exclude.includes(key)) return; + if (key in target.props) { + return { + enumerable: true, + configurable: true, + value: target.props[key] + }; + } + }, + has(target, key) { + if (target.exclude.includes(key)) return false; + return key in target.props; + }, + ownKeys(target) { + return Reflect.ownKeys(target.props).filter((key) => !target.exclude.includes(key)); + } +}; + +/** + * @param {Record} props + * @param {string[]} exclude + * @returns {Record} + */ +export function legacy_rest_props(props, exclude) { + return new Proxy({ props, exclude, special: {}, version: source(0) }, legacy_rest_props_handler); } /** diff --git a/packages/svelte/tests/runtime-legacy/samples/props-reassign/App.svelte b/packages/svelte/tests/runtime-legacy/samples/props-reassign/App.svelte new file mode 100644 index 0000000000..7434b53fb1 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/props-reassign/App.svelte @@ -0,0 +1,6 @@ + + +

{$$props.a} {$$props.b}

+ diff --git a/packages/svelte/tests/runtime-legacy/samples/props-reassign/_config.js b/packages/svelte/tests/runtime-legacy/samples/props-reassign/_config.js new file mode 100644 index 0000000000..d5760f7a77 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/props-reassign/_config.js @@ -0,0 +1,35 @@ +import { test } from '../../test'; + +export default test({ + html: ` + +

0

+ + `, + + async test({ assert, target }) { + const [btn1, btn2] = target.querySelectorAll('button'); + + await btn1.click(); + + assert.htmlEqual( + target.innerHTML, + ` + +

2

+ + ` + ); + + await btn2.click(); + + assert.htmlEqual( + target.innerHTML, + ` + +

4 b

+ + ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/props-reassign/main.svelte b/packages/svelte/tests/runtime-legacy/samples/props-reassign/main.svelte new file mode 100644 index 0000000000..3766503697 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/props-reassign/main.svelte @@ -0,0 +1,7 @@ + + + + diff --git a/packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/App.svelte b/packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/App.svelte new file mode 100644 index 0000000000..3121bb9da3 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/App.svelte @@ -0,0 +1,6 @@ + + +

{$$restProps.a} {$$restProps.b} {$$restProps.c}

+ diff --git a/packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/_config.js b/packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/_config.js new file mode 100644 index 0000000000..4e8638eab2 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/_config.js @@ -0,0 +1,35 @@ +import { test } from '../../test'; + +export default test({ + html: ` + +

0 c

+ + `, + + async test({ assert, target }) { + const [btn1, btn2] = target.querySelectorAll('button'); + + await btn1.click(); + + assert.htmlEqual( + target.innerHTML, + ` + +

1 c

+ + ` + ); + + await btn2.click(); + + assert.htmlEqual( + target.innerHTML, + ` + +

1 b c

+ + ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/main.svelte b/packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/main.svelte new file mode 100644 index 0000000000..3766503697 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/rest-props-reassign/main.svelte @@ -0,0 +1,7 @@ + + + +