Skip to content

chore: use proxies instead of signals for spread/rest props #9801

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

Merged
merged 15 commits into from
Dec 6, 2023
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ function read_attribute(parser) {
expression,
parent: null,
metadata: {
contains_call_expression: false,
dynamic: false
}
};
Expand Down
5 changes: 4 additions & 1 deletion packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,10 @@ const common_visitors = {
}
},
CallExpression(node, context) {
if (context.state.expression?.type === 'ExpressionTag' && !is_known_safe_call(node, context)) {
if (
context.state.expression?.type === 'ExpressionTag' ||
(context.state.expression?.type === 'SpreadAttribute' && !is_known_safe_call(node, context))
) {
context.state.expression.metadata.contains_call_expression = true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export function serialize_get_binding(node, state) {

if (binding.kind === 'prop' && binding.node.name === '$$props') {
// Special case for $$props which only exists in the old world
return b.call('$.unwrap', node);
return node;
}

if (
Expand All @@ -88,7 +88,6 @@ export function serialize_get_binding(node, state) {
(!state.analysis.immutable || state.analysis.accessors || binding.reassigned)) ||
binding.kind === 'derived' ||
binding.kind === 'prop' ||
binding.kind === 'rest_prop' ||
binding.kind === 'legacy_reactive'
) {
return b.call('$.get', node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export const global_visitors = {
Identifier(node, { path, state }) {
if (is_reference(node, /** @type {import('estree').Node} */ (path.at(-1)))) {
if (node.name === '$$props') {
return b.call('$.get', b.id('$$sanitized_props'));
return b.id('$$sanitized_props');
}
return serialize_get_binding(node, state);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,20 @@ function serialize_inline_component(node, component_name, context) {
}
events[attribute.name].push(handler);
} else if (attribute.type === 'SpreadAttribute') {
props_and_spreads.push(/** @type {import('estree').Expression} */ (context.visit(attribute)));
const expression = /** @type {import('estree').Expression} */ (context.visit(attribute));
if (attribute.metadata.dynamic) {
let value = expression;

if (attribute.metadata.contains_call_expression) {
const id = b.id(context.state.scope.generate('spread_element'));
context.state.init.push(b.var(id, b.call('$.derived', b.thunk(value))));
value = b.call('$.get', id);
}

props_and_spreads.push(b.thunk(value));
} else {
props_and_spreads.push(expression);
}
} else if (attribute.type === 'Attribute') {
if (attribute.name.startsWith('--')) {
custom_css_props.push(
Expand Down Expand Up @@ -895,7 +908,7 @@ function serialize_inline_component(node, component_name, context) {
? b.object(/** @type {import('estree').Property[]} */ (props_and_spreads[0]) || [])
: b.call(
'$.spread_props',
b.thunk(b.array(props_and_spreads.map((p) => (Array.isArray(p) ? b.object(p) : p))))
...props_and_spreads.map((p) => (Array.isArray(p) ? b.object(p) : p))
);
/** @param {import('estree').Identifier} node_id */
let fn = (node_id) =>
Expand Down Expand Up @@ -2764,8 +2777,8 @@ export const template_visitors = {
}
},
LetDirective(node, { state }) {
// let:x --> const x = $.derived(() => $.unwrap($$slotProps).x);
// let:x={{y, z}} --> const derived_x = $.derived(() => { const { y, z } = $.unwrap($$slotProps).x; return { y, z }));
// let:x --> const x = $.derived(() => $$slotProps.x);
// let:x={{y, z}} --> const derived_x = $.derived(() => { const { y, z } = $$slotProps.x; return { y, z }));
if (node.expression && node.expression.type !== 'Identifier') {
const name = state.scope.generate(node.name);
const bindings = state.scope.get_bindings(node);
Expand All @@ -2787,7 +2800,7 @@ export const template_visitors = {
b.object_pattern(node.expression.properties)
: // @ts-expect-error types don't match, but it can't contain spread elements and the structure is otherwise fine
b.array_pattern(node.expression.elements),
b.member(b.call('$.unwrap', b.id('$$slotProps')), b.id(node.name))
b.member(b.id('$$slotProps'), b.id(node.name))
),
b.return(b.object(bindings.map((binding) => b.init(binding.node.name, binding.node))))
])
Expand All @@ -2798,10 +2811,7 @@ export const template_visitors = {
const name = node.expression === null ? node.name : node.expression.name;
return b.const(
name,
b.call(
'$.derived',
b.thunk(b.member(b.call('$.unwrap', b.id('$$slotProps')), b.id(node.name)))
)
b.call('$.derived', b.thunk(b.member(b.id('$$slotProps'), b.id(node.name))))
);
}
},
Expand Down Expand Up @@ -2854,7 +2864,9 @@ export const template_visitors = {

for (const attribute of node.attributes) {
if (attribute.type === 'SpreadAttribute') {
spreads.push(/** @type {import('estree').Expression} */ (context.visit(attribute)));
spreads.push(
b.thunk(/** @type {import('estree').Expression} */ (context.visit(attribute)))
);
} else if (attribute.type === 'Attribute') {
const [, value] = serialize_attribute_value(attribute.value, context);
if (attribute.name === 'name') {
Expand All @@ -2873,7 +2885,7 @@ export const template_visitors = {
const props_expression =
spreads.length === 0
? b.object(props)
: b.call('$.spread_props', b.thunk(b.array([b.object(props), ...spreads])));
: b.call('$.spread_props', b.object(props), ...spreads);
const fallback =
node.fragment.nodes.length === 0
? b.literal(null)
Expand All @@ -2883,8 +2895,8 @@ export const template_visitors = {
);

const expression = is_default
? b.member(b.call('$.unwrap', b.id('$$props')), b.id('children'))
: b.member(b.member(b.call('$.unwrap', b.id('$$props')), b.id('$$slots')), name, true, true);
? b.member(b.id('$$props'), b.id('children'))
: b.member(b.member(b.id('$$props'), b.id('$$slots')), name, true, true);

const slot = b.call('$.slot', context.state.node, expression, props_expression, fallback);
context.state.init.push(b.stmt(slot));
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/compiler/types/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ export interface SpreadAttribute extends BaseNode {
type: 'SpreadAttribute';
expression: Expression;
metadata: {
contains_call_expression: boolean;
dynamic: boolean;
};
}
Expand Down
Loading