Skip to content

Commit

Permalink
fix: handle reassignment of $$props and $$restProps (#11348)
Browse files Browse the repository at this point in the history
* 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

#10359 (comment)

* $$props is coarse grained on updates, so we can simplify this

* fix

* fix comment
  • Loading branch information
dummdidumm committed Apr 29, 2024
1 parent 5e0845f commit 2754e4e
Show file tree
Hide file tree
Showing 12 changed files with 177 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/lucky-teachers-exist.md
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: handle reassignment of `$$props` and `$$restProps`
2 changes: 1 addition & 1 deletion packages/svelte/src/compiler/phases/2-analyze/index.js
Expand Up @@ -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]) {
Expand Down
Expand Up @@ -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)))
)
Expand All @@ -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))
)
);
}

Expand Down
11 changes: 5 additions & 6 deletions packages/svelte/src/compiler/phases/3-transform/client/utils.js
Expand Up @@ -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);
}
Expand All @@ -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) ||
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/internal/client/index.js
Expand Up @@ -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
Expand Down
66 changes: 63 additions & 3 deletions packages/svelte/src/internal/client/reactivity/props.js
Expand Up @@ -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';
Expand Down Expand Up @@ -79,7 +79,67 @@ const rest_props_handler = {
* @returns {Record<string, unknown>}
*/
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<string | symbol, unknown>, exclude: Array<string | symbol>, special: Record<string | symbol, (v?: unknown) => unknown>, version: import('./types.js').Source<number> }>}}
*/
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<string, (v?: unknown) => 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<string, unknown>} props
* @param {string[]} exclude
* @returns {Record<string, unknown>}
*/
export function legacy_rest_props(props, exclude) {
return new Proxy({ props, exclude, special: {}, version: source(0) }, legacy_rest_props_handler);
}

/**
Expand Down
@@ -0,0 +1,6 @@
<script>
$: $$props.a = $$props.a * 2;
</script>

<p>{$$props.a} {$$props.b}</p>
<button on:click={() => $$props.b = 'b'}>update</button>
@@ -0,0 +1,35 @@
import { test } from '../../test';

export default test({
html: `
<button>increment</button>
<p>0 </p>
<button>update</button>
`,

async test({ assert, target }) {
const [btn1, btn2] = target.querySelectorAll('button');

await btn1.click();

assert.htmlEqual(
target.innerHTML,
`
<button>increment</button>
<p>2 </p>
<button>update</button>
`
);

await btn2.click();

assert.htmlEqual(
target.innerHTML,
`
<button>increment</button>
<p>4 b</p>
<button>update</button>
`
);
}
});
@@ -0,0 +1,7 @@
<script>
import App from './App.svelte';
let a = 0;
</script>

<button on:click={() => a++}>increment</button>
<App {a} />
@@ -0,0 +1,6 @@
<script>
$: $$restProps.c = $$restProps.c ?? 'c';
</script>
<p>{$$restProps.a} {$$restProps.b} {$$restProps.c}</p>
<button on:click={() => $$restProps.b = 'b'}>update</button>
@@ -0,0 +1,35 @@
import { test } from '../../test';

export default test({
html: `
<button>increment</button>
<p>0 c</p>
<button>update</button>
`,

async test({ assert, target }) {
const [btn1, btn2] = target.querySelectorAll('button');

await btn1.click();

assert.htmlEqual(
target.innerHTML,
`
<button>increment</button>
<p>1 c</p>
<button>update</button>
`
);

await btn2.click();

assert.htmlEqual(
target.innerHTML,
`
<button>increment</button>
<p>1 b c</p>
<button>update</button>
`
);
}
});
@@ -0,0 +1,7 @@
<script>
import App from './App.svelte';
let a = 0;
</script>

<button on:click={() => a++}>increment</button>
<App {a} />

0 comments on commit 2754e4e

Please sign in to comment.