Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle reassignment of $$props and $$restProps #11348

Merged
merged 4 commits into from Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -447,7 +447,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} />