Skip to content

Commit

Permalink
fix: handle reassignment of $$props and $$restProps
Browse files Browse the repository at this point in the history
Some libraries assign to properties of `$$props` and `$$restProps`. These were previously resulting in an error but are now handled properly

#10359 (comment)
  • Loading branch information
dummdidumm committed Apr 27, 2024
1 parent 7d19e5b commit c6e7284
Show file tree
Hide file tree
Showing 12 changed files with 207 additions and 9 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`
3 changes: 0 additions & 3 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Expand Up @@ -447,9 +447,6 @@ export function analyze_component(root, source, options) {
);
}
} else {
instance.scope.declare(b.id('$$props'), 'bindable_prop', 'synthetic');
instance.scope.declare(b.id('$$restProps'), 'rest_prop', 'synthetic');

for (const { ast, scope, scopes } of [module, instance, template]) {
/** @type {import('./types').LegacyAnalysisState} */
const state = {
Expand Down
Expand Up @@ -418,9 +418,10 @@ 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)))
b.array(named_props.map((name) => b.literal(name))),
b.array([])
)
)
);
Expand All @@ -431,8 +432,34 @@ export function client_component(source, analysis, options) {
if (analysis.custom_element) {
to_remove.push(b.literal('$$host'));
}

// Find keys that are reassigned in the component because those can get out of sync
// with the parent and therefore need special handling. This currently doesn't handle
// dynamic keys, let's hope noone does that.
const reassigned = [];
const props = [
analysis.instance.scope.get('$$restProps'),
analysis.instance.scope.get('$$props')
];
for (const prop of props) {
for (const { path } of prop?.references ?? []) {
const reference = path.at(-1);
const parent = path.at(-2);
if (
reference?.type === 'MemberExpression' &&
reference.property.type === 'Identifier' &&
parent?.type === 'AssignmentExpression'
) {
reassigned.push(b.literal(reference.property.name));
}
}
}

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), b.array(reassigned))
)
);
}

Expand Down
12 changes: 10 additions & 2 deletions packages/svelte/src/compiler/phases/scope.js
Expand Up @@ -688,8 +688,16 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {

// we do this after the fact, so that we don't need to worry
// about encountering references before their declarations
for (const [scope, { node, path }] of references) {
scope.reference(node, path);
for (const [ref_scope, { node, path }] of references) {
// We need to declare the synthetic $$props and $$restProps bindings if they are used
// right here in order to get their references, which are important later on
if (node.name === '$$props' && !scope.get('$$props')) {
scope.declare(b.id('$$props'), 'rest_prop', 'synthetic');
} else if (node.name === '$$restProps' && !scope.get('$$restProps')) {
scope.declare(b.id('$$restProps'), 'rest_prop', 'synthetic');
}

ref_scope.reference(node, path);
}

for (const [scope, node] of updates) {
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: 65 additions & 1 deletion packages/svelte/src/internal/client/reactivity/props.js
Expand Up @@ -72,14 +72,78 @@ const 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> }>}}
*/
const legacy_rest_props_handler = {
get(target, key) {
if (target.exclude.includes(key)) return;
return key in target.special ? target.special[key]() : target.props[key];
},
set(target, key, value) {
if (key in target.special) {
target.special[key](value);
} else {
target.props[key] = value;
}

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
* @param {string} [name]
* @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
);
}

/**
* @param {Record<string, unknown>} props
* @param {string[]} exclude
* @param {string[]} reassigned
* @returns {Record<string, unknown>}
*/
export function legacy_rest_props(props, exclude, reassigned) {
// Handle props that can temporarily get out of sync with the parent
/** @type {Record<string, (v?: unknown) => unknown>} */
const special = {};
for (const key of reassigned) {
special[key] = prop(
{
get [key]() {
return props[key];
}
},
key,
PROPS_IS_UPDATED
);
}
return new Proxy({ props, exclude, special }, 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>2 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.a = $$restProps.a * 2;
</script>

<p>{$$restProps.a} {$$restProps.b}</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 </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>2 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 comments on commit c6e7284

Please sign in to comment.