msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

chore: remove `binding.expression` (#12530)

Browse Source 
* add state.getters as alternative to binding.expression

* on second thoughts

* fix

* first of many

* couple more

* regenerate types

* more

* another

* more

* another

* another

* another

* remove binding.expression from client-side code

* tweak

* last one

* comment

* regenerate types

* add a changeset

* small tidy up

* simplify

* simplify

* simplify and fix

* simplify
pull/12578/head
Rich Harris 4 months ago committed by GitHub
parent 37782be9b1
commit cf8df0bacc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 154 additions and 121 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/four-peas-tickle.md
Unescape View File

@ -0,0 +1,5 @@
---
'svelte': patch
---
chore: remove internal `binding.expression` mechanism

2
packages/svelte/src/compiler/phases/3-transform/client/transform-client.js
Unescape View File

@ -65,6 +65,7 @@ export function client_component(source, analysis, options) {
preserve_whitespace: options.preserveWhitespace,
public_state: new Map(),
private_state: new Map(),
getters: {},
in_constructor: false,
// these are set inside the `Fragment` visitor, and cannot be used until then
@ -583,6 +584,7 @@ export function client_module(analysis, options) {
legacy_reactive_statements: new Map(),
public_state: new Map(),
private_state: new Map(),
getters: {},
in_constructor: false
};

24
packages/svelte/src/compiler/phases/3-transform/client/utils.js
Unescape View File

@ -84,8 +84,9 @@ export function serialize_get_binding(node, state) {
return b.call(node);
}
if (binding.expression) {
return typeof binding.expression === 'function' ? binding.expression(node) : binding.expression;
if (Object.hasOwn(state.getters, node.name)) {
const getter = state.getters[node.name];
return typeof getter === 'function' ? getter(node) : getter;
}
if (binding.kind === 'prop' || binding.kind === 'bindable_prop') {
@ -527,17 +528,20 @@ function get_hoistable_params(node, context) {
);
}
const expression = context.state.getters[reference];
if (
// If it's a destructured derived binding, then we can extract the derived signal reference and use that.
binding.expression !== null &&
typeof binding.expression !== 'function' &&
binding.expression.type === 'MemberExpression' &&
binding.expression.object.type === 'CallExpression' &&
binding.expression.object.callee.type === 'Identifier' &&
binding.expression.object.callee.name === '$.get' &&
binding.expression.object.arguments[0].type === 'Identifier'
// TODO this code is bad, we need to kill it
expression != null &&
typeof expression !== 'function' &&
expression.type === 'MemberExpression' &&
expression.object.type === 'CallExpression' &&
expression.object.callee.type === 'Identifier' &&
expression.object.callee.name === '$.get' &&
expression.object.arguments[0].type === 'Identifier'
) {
push_unique(b.id(binding.expression.object.arguments[0].name));
push_unique(b.id(expression.object.arguments[0].name));
} else if (
// If we are referencing a simple $$props value, then we need to reference the object property instead
(binding.kind === 'prop' || binding.kind === 'bindable_prop') &&

177
packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
Unescape View File

@ -51,6 +51,7 @@ import { javascript_visitors_runes } from './javascript-runes.js';
import { sanitize_template_string } from '../../../../utils/sanitize_template_string.js';
import { walk } from 'zimmerframe';
import { locator } from '../../../../state.js';
import is_reference from 'is-reference';
/**
* @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} element
@ -939,11 +940,7 @@ function serialize_inline_component(node, component_name, context, anchor = cont
const prev = fn;
fn = (node_id) => {
return serialize_bind_this(
/** @type {Identifier | MemberExpression} */ (bind_this),
context,
prev(node_id)
);
return serialize_bind_this(bind_this, prev(node_id), context);
};
}
@ -996,71 +993,69 @@ function serialize_inline_component(node, component_name, context, anchor = cont
/**
* Serializes `bind:this` for components and elements.
* @param {Identifier | MemberExpression} bind_this
* @param {Identifier | MemberExpression} expression
* @param {Expression} value
* @param {import('zimmerframe').Context<import('#compiler').SvelteNode, import('../types.js').ComponentClientTransformState>} context
* @param {Expression} node
* @returns
*/
function serialize_bind_this(bind_this, context, node) {
let i = 0;
/** @type {Map<import('#compiler').Binding, [arg_idx: number, transformed: Expression, expression: import('#compiler').Binding['expression']]>} */
const each_ids = new Map();
// Transform each reference to an each block context variable into a $$value_<i> variable
// by temporarily changing the `expression` of the corresponding binding.
// These $$value_<i> variables will be filled in by the bind_this runtime function through its last argument.
function serialize_bind_this(expression, value, { state, visit }) {
/** @type {Identifier[]} */
const ids = [];
/** @type {Expression[]} */
const values = [];
/** @type {typeof state.getters} */
const getters = {};
// Pass in each context variables to the get/set functions, so that we can null out old values on teardown.
// Note that we only do this for each context variables, the consequence is that the value might be stale in
// some scenarios where the value is a member expression with changing computed parts or using a combination of multiple
// variables, but that was the same case in Svelte 4, too. Once legacy mode is gone completely, we can revisit this.
walk(
bind_this,
{},
{
Identifier(node) {
const binding = context.state.scope.get(node.name);
if (!binding || each_ids.has(binding)) return;
const associated_node = Array.from(context.state.scopes.entries()).find(
([_, scope]) => scope === binding?.scope
)?.[0];
if (associated_node?.type === 'EachBlock') {
each_ids.set(binding, [
i,
/** @type {Expression} */ (context.visit(node)),
binding.expression
]);
binding.expression = b.id('$$value_' + i);
i++;
walk(expression, null, {
Identifier(node, { path }) {
if (Object.hasOwn(getters, node.name)) return;
const parent = /** @type {Expression} */ (path.at(-1));
if (!is_reference(node, parent)) return;
const binding = state.scope.get(node.name);
if (!binding) return;
for (const [owner, scope] of state.scopes) {
if (owner.type === 'EachBlock' && scope === binding.scope) {
ids.push(node);
values.push(/** @type {Expression} */ (visit(node)));
getters[node.name] = node;
break;
}
}
}
);
});
const bind_this_id = /** @type {Expression} */ (context.visit(bind_this));
const ids = Array.from(each_ids.values()).map((id) => b.id('$$value_' + id[0]));
const assignment = b.assignment('=', bind_this, b.id('$$value'));
const update = serialize_set_binding(assignment, context, () => context.visit(assignment));
const child_state = { ...state, getters: { ...state.getters, ...getters } };
for (const [binding, [, , expression]] of each_ids) {
// reset expressions to what they were before
binding.expression = expression;
}
const get = /** @type {Expression} */ (visit(expression, child_state));
const set = /** @type {Expression} */ (
visit(b.assignment('=', expression, b.id('$$value')), child_state)
);
/** @type {Expression[]} */
const args = [node, b.arrow([b.id('$$value'), ...ids], update), b.arrow([...ids], bind_this_id)];
// If we're mutating a property, then it might already be non-existent.
// If we make all the object nodes optional, then it avoids any runtime exceptions.
/** @type {Expression | Super} */
let bind_node = bind_this_id;
let node = get;
while (bind_node?.type === 'MemberExpression') {
bind_node.optional = true;
bind_node = bind_node.object;
}
if (each_ids.size) {
args.push(b.thunk(b.array(Array.from(each_ids.values()).map((id) => id[1]))));
while (node.type === 'MemberExpression') {
node.optional = true;
node = node.object;
}
return b.call('$.bind_this', ...args);
return b.call(
'$.bind_this',
value,
b.arrow([b.id('$$value'), ...ids], set),
b.arrow([...ids], get),
values.length > 0 && b.thunk(b.array(values))
);
}
/**
@ -1394,7 +1389,7 @@ function process_children(nodes, expression, is_element, { visit, state }) {
const text_id = get_node_id(expression(true), state, 'text');
const update = b.stmt(
b.call('$.set_text', text_id, /** @type {Expression} */ (visit(node.expression)))
b.call('$.set_text', text_id, /** @type {Expression} */ (visit(node.expression, state)))
);
if (node.metadata.contains_call_expression && !within_bound_contenteditable) {
@ -1654,6 +1649,7 @@ export const template_visitors = {
after_update: [],
template: [],
locations: [],
getters: { ...context.state.getters },
metadata: {
context: {
template_needs_import_node: false,
@ -1816,6 +1812,8 @@ export const template_visitors = {
)
);
state.getters[declaration.id.name] = b.call('$.get', declaration.id);
// we need to eagerly evaluate the expression in order to hit any
// 'Cannot access x before initialization' errors
if (state.options.dev) {
@ -1825,21 +1823,24 @@ export const template_visitors = {
const identifiers = extract_identifiers(declaration.id);
const tmp = b.id(state.scope.generate('computed_const'));
const getters = { ...state.getters };
// Make all identifiers that are declared within the following computed regular
// variables, as they are not signals in that context yet
for (const node of identifiers) {
const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(node.name));
binding.expression = node;
getters[node.name] = node;
}
const child_state = { ...state, getters };
// TODO optimise the simple `{ x } = y` case — we can just return `y`
// instead of destructuring it only to return a new object
const fn = b.arrow(
[],
b.block([
b.const(
/** @type {Pattern} */ (visit(declaration.id)),
/** @type {Expression} */ (visit(declaration.init))
/** @type {Pattern} */ (visit(declaration.id, child_state)),
/** @type {Expression} */ (visit(declaration.init, child_state))
),
b.return(b.object(identifiers.map((node) => b.prop('init', node, node))))
])
@ -1854,8 +1855,7 @@ export const template_visitors = {
}
for (const node of identifiers) {
const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(node.name));
binding.expression = b.member(b.call('$.get', tmp), node);
state.getters[node.name] = b.member(b.call('$.get', tmp), node);
}
}
},
@ -2484,6 +2484,11 @@ export const template_visitors = {
indirect_dependencies.push(...transitive_dependencies);
}
const child_state = {
...context.state,
getters: { ...context.state.getters }
};
/**
* @param {Pattern} expression_for_id
* @returns {import('#compiler').Binding['mutation']}
@ -2532,17 +2537,14 @@ export const template_visitors = {
: b.id(node.index);
const item = each_node_meta.item;
const binding = /** @type {import('#compiler').Binding} */ (context.state.scope.get(item.name));
binding.expression = (/** @type {import("estree").Identifier} */ id) => {
const getter = (/** @type {import("estree").Identifier} */ id) => {
const item_with_loc = with_loc(item, id);
return b.call('$.unwrap', item_with_loc);
};
child_state.getters[item.name] = getter;
if (node.index) {
const index_binding = /** @type {import('#compiler').Binding} */ (
context.state.scope.get(node.index)
);
index_binding.expression = (id) => {
child_state.getters[node.index] = (id) => {
const index_with_loc = with_loc(index, id);
return b.call('$.unwrap', index_with_loc);
};
@ -2560,19 +2562,23 @@ export const template_visitors = {
)
);
} else {
const unwrapped = binding.expression(binding.node);
const unwrapped = getter(binding.node);
const paths = extract_paths(node.context);
for (const path of paths) {
const name = /** @type {Identifier} */ (path.node).name;
const binding = /** @type {import('#compiler').Binding} */ (context.state.scope.get(name));
const needs_derived = path.has_default_value; // to ensure that default value is only called once
const fn = b.thunk(/** @type {Expression} */ (context.visit(path.expression?.(unwrapped))));
const fn = b.thunk(
/** @type {Expression} */ (context.visit(path.expression?.(unwrapped), child_state))
);
declarations.push(
b.let(path.node, needs_derived ? b.call('$.derived_safe_equal', fn) : fn)
);
binding.expression = needs_derived ? b.call('$.get', b.id(name)) : b.call(name);
const getter = needs_derived ? b.call('$.get', b.id(name)) : b.call(name);
child_state.getters[name] = getter;
binding.mutation = create_mutation(
/** @type {Pattern} */ (path.update_expression(unwrapped))
);
@ -2580,21 +2586,23 @@ export const template_visitors = {
// we need to eagerly evaluate the expression in order to hit any
// 'Cannot access x before initialization' errors
if (context.state.options.dev) {
declarations.push(b.stmt(binding.expression));
declarations.push(b.stmt(getter));
}
}
}
const block = /** @type {BlockStatement} */ (context.visit(node.body));
const block = /** @type {BlockStatement} */ (context.visit(node.body, child_state));
const key_function = node.key
? b.arrow(
[node.context.type === 'Identifier' ? node.context : b.id('$$item'), index],
declarations.length > 0
? b.block(
declarations.concat(b.return(/** @type {Expression} */ (context.visit(node.key))))
declarations.concat(
b.return(/** @type {Expression} */ (context.visit(node.key, child_state)))
)
)
: /** @type {Expression} */ (context.visit(node.key))
: /** @type {Expression} */ (context.visit(node.key, child_state))
)
: b.id('$.index');
@ -2755,6 +2763,9 @@ export const template_visitors = {
/** @type {Statement[]} */
const declarations = [];
const getters = { ...context.state.getters };
const child_state = { ...context.state, getters };
for (let i = 0; i < node.parameters.length; i++) {
const argument = node.parameters[i];
@ -2766,10 +2777,8 @@ export const template_visitors = {
left: argument,
right: b.id('$.noop')
});
const binding = /** @type {import('#compiler').Binding} */ (
context.state.scope.get(argument.name)
);
binding.expression = b.call(argument);
getters[argument.name] = b.call(argument);
continue;
}
@ -2791,19 +2800,20 @@ export const template_visitors = {
declarations.push(
b.let(path.node, needs_derived ? b.call('$.derived_safe_equal', fn) : fn)
);
binding.expression = needs_derived ? b.call('$.get', b.id(name)) : b.call(name);
getters[name] = needs_derived ? b.call('$.get', b.id(name)) : b.call(name);
// we need to eagerly evaluate the expression in order to hit any
// 'Cannot access x before initialization' errors
if (context.state.options.dev) {
declarations.push(b.stmt(binding.expression));
declarations.push(b.stmt(getters[name]));
}
}
}
body = b.block([
...declarations,
.../** @type {BlockStatement} */ (context.visit(node.body)).body
.../** @type {BlockStatement} */ (context.visit(node.body, child_state)).body
]);
/** @type {Expression} */
@ -2996,7 +3006,7 @@ export const template_visitors = {
break;
case 'this':
call_expr = serialize_bind_this(node.expression, context, state.node);
call_expr = serialize_bind_this(node.expression, state.node, context);
break;
case 'textContent':
case 'innerHTML':
@ -3118,7 +3128,10 @@ export const template_visitors = {
const bindings = state.scope.get_bindings(node);
for (const binding of bindings) {
binding.expression = b.member(b.call('$.get', b.id(name)), b.id(binding.node.name));
state.getters[binding.node.name] = b.member(
b.call('$.get', b.id(name)),
b.id(binding.node.name)
);
}
return b.const(

50
packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
Unescape View File

@ -221,8 +221,9 @@ function serialize_get_binding(node, state) {
);
}
if (binding.expression) {
return typeof binding.expression === 'function' ? binding.expression(node) : binding.expression;
if (Object.hasOwn(state.getters, node.name)) {
const getter = state.getters[node.name];
return typeof getter === 'function' ? getter(node) : getter;
}
return node;
@ -1233,8 +1234,20 @@ const template_visitors = {
throw new Error('Node should have been handled elsewhere');
},
RegularElement(node, context) {
const namespace = determine_namespace_for_children(node, context.state.namespace);
/** @type {import('./types').ComponentServerTransformState} */
const state = {
...context.state,
getters: { ...context.state.getters },
namespace,
preserve_whitespace:
context.state.preserve_whitespace ||
((node.name === 'pre' || node.name === 'textarea') && namespace !== 'foreign')
};
context.state.template.push(b.literal(`<${node.name}`));
const body = serialize_element_attributes(node, context);
const body = serialize_element_attributes(node, { ...context, state });
context.state.template.push(b.literal('>'));
if ((node.name === 'script' || node.name === 'style') && node.fragment.nodes.length === 1) {
@ -1246,17 +1259,6 @@ const template_visitors = {
return;
}
const namespace = determine_namespace_for_children(node, context.state.namespace);
/** @type {import('./types').ComponentServerTransformState} */
const state = {
...context.state,
namespace,
preserve_whitespace:
context.state.preserve_whitespace ||
((node.name === 'pre' || node.name === 'textarea') && namespace !== 'foreign')
};
const { hoisted, trimmed } = clean_nodes(
node,
node.fragment.nodes,
@ -1339,6 +1341,7 @@ const template_visitors = {
const state = {
...context.state,
getteres: { ...context.state.getters },
namespace: determine_namespace_for_children(node, context.state.namespace),
template: [],
init: []
@ -1513,7 +1516,7 @@ const template_visitors = {
const bindings = state.scope.get_bindings(node);
for (const binding of bindings) {
binding.expression = b.member(b.id(name), b.id(binding.node.name));
state.getters[binding.node.name] = b.member(b.id(name), b.id(binding.node.name));
}
return b.const(
@ -1539,15 +1542,24 @@ const template_visitors = {
return visit(node.expression);
},
SvelteFragment(node, context) {
const child_state = {
...context.state,
getters: { ...context.state.getters }
};
for (const attribute of node.attributes) {
if (attribute.type === 'LetDirective') {
context.state.template.push(
/** @type {import('estree').ExpressionStatement} */ (context.visit(attribute))
/** @type {import('estree').ExpressionStatement} */ (
context.visit(attribute, child_state)
)
);
}
}
const block = /** @type {import('estree').BlockStatement} */ (context.visit(node.fragment));
const block = /** @type {import('estree').BlockStatement} */ (
context.visit(node.fragment, child_state)
);
context.state.template.push(block);
},
@ -1967,6 +1979,7 @@ export function server_component(analysis, options) {
namespace: options.namespace,
preserve_whitespace: options.preserveWhitespace,
private_derived: new Map(),
getters: {},
skip_hydration_boundaries: false
};
@ -2277,7 +2290,8 @@ export function server_module(analysis, options) {
// to be present for `javascript_visitors_legacy` and so is included in module
// transform state as well as component transform state
legacy_reactive_statements: new Map(),
private_derived: new Map()
private_derived: new Map(),
getters: {}
};
const module = /** @type {import('estree').Program} */ (

6
packages/svelte/src/compiler/phases/3-transform/types.d.ts vendored
Unescape View File

@ -1,10 +1,16 @@
import type { Scope } from '../scope.js';
import type { SvelteNode, ValidatedModuleCompileOptions } from '#compiler';
import type { Analysis } from '../types.js';
import type { Expression, Identifier } from 'estree';
export interface TransformState {
readonly analysis: Analysis;
readonly options: ValidatedModuleCompileOptions;
readonly scope: Scope;
readonly scopes: Map<SvelteNode, Scope>;
/**
* A map of `[name, node]` pairs, where `Identifier` nodes matching `name`
* will be replaced with `node` (e.g. `x` -> `$.get(x)`)
*/
readonly getters: Record<string, Expression | ((id: Identifier) => Expression)>;
}

1
packages/svelte/src/compiler/phases/scope.js
Unescape View File

@ -115,7 +115,6 @@ export class Scope {
declaration_kind,
is_called: false,
prop_alias: null,
expression: null,
mutation: null,
reassigned: false,
metadata: null

5
packages/svelte/src/compiler/types/index.d.ts vendored
Unescape View File

@ -305,11 +305,6 @@ export interface Binding {
legacy_dependencies: Binding[];
/** Legacy props: the `class` in `{ export klass as class}`. $props(): The `class` in { class: klass } = $props() */
prop_alias: string | null;
/**
* If this is set, all references should use this expression instead of the identifier name.
* If a function is given, it will be called with the identifier at that location and should return the new expression.
*/
expression: Expression | ((id: Identifier) => Expression) | null;
/** If this is set, all mutations should use this expression */
mutation: ((assignment: AssignmentExpression, context: Context<any, any>) => Expression) | null;
/** Additional metadata, varies per binding type */

5
packages/svelte/types/index.d.ts vendored
Unescape View File

@ -955,11 +955,6 @@ declare module 'svelte/compiler' {
legacy_dependencies: Binding[];
/** Legacy props: the `class` in `{ export klass as class}`. $props(): The `class` in { class: klass } = $props() */
prop_alias: string | null;
/**
* If this is set, all references should use this expression instead of the identifier name.
* If a function is given, it will be called with the identifier at that location and should return the new expression.
*/
expression: Expression | ((id: Identifier) => Expression) | null;
/** If this is set, all mutations should use this expression */
mutation: ((assignment: AssignmentExpression, context: Context<any, any>) => Expression) | null;
/** Additional metadata, varies per binding type */

Write Preview
Loading…
Cancel
Save