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

fix: avoid double deriveds in component props (#15089)

Browse Source 
* fix: prevent double deriveds in component props

* unused

* reuse mechanism

* move code to where it is used

* changeset
pull/15092/head
Rich Harris 8 months ago committed by GitHub
parent eee808fb79
commit 8bba70b8e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 93 additions and 87 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/tasty-pigs-greet.md
Unescape View File

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: avoid double deriveds in component props

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

@ -269,41 +269,6 @@ export function should_proxy(node, scope) {
return true;
}
/**
* @param {Pattern} node
* @param {import('zimmerframe').Context<AST.SvelteNode, ComponentClientTransformState>} context
* @returns {{ id: Pattern, declarations: null | Statement[] }}
*/
export function create_derived_block_argument(node, context) {
if (node.type === 'Identifier') {
context.state.transform[node.name] = { read: get_value };
return { id: node, declarations: null };
}
const pattern = /** @type {Pattern} */ (context.visit(node));
const identifiers = extract_identifiers(node);
const id = b.id('$$source');
const value = b.id('$$value');
const block = b.block([
b.var(pattern, b.call('$.get', id)),
b.return(b.object(identifiers.map((identifier) => b.prop('init', identifier, identifier))))
]);
const declarations = [b.var(value, create_derived(context.state, b.thunk(block)))];
for (const id of identifiers) {
context.state.transform[id.name] = { read: get_value };
declarations.push(
b.var(id, create_derived(context.state, b.thunk(b.member(b.call('$.get', value), id))))
);
}
return { id, declarations };
}
/**
* Svelte legacy mode should use safe equals in most places, runes mode shouldn't
* @param {ComponentClientTransformState} state

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

@ -1,8 +1,10 @@
/** @import { BlockStatement, Expression, Pattern, Statement } from 'estree' */
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types' */
/** @import { ComponentClientTransformState, ComponentContext } from '../types' */
import { extract_identifiers } from '../../../../utils/ast.js';
import * as b from '../../../../utils/builders.js';
import { create_derived_block_argument } from '../utils.js';
import { create_derived } from '../utils.js';
import { get_value } from './shared/declarations.js';
/**
* @param {AST.AwaitBlock} node
@ -65,3 +67,38 @@ export function AwaitBlock(node, context) {
)
);
}
/**
* @param {Pattern} node
* @param {import('zimmerframe').Context<AST.SvelteNode, ComponentClientTransformState>} context
* @returns {{ id: Pattern, declarations: null | Statement[] }}
*/
function create_derived_block_argument(node, context) {
if (node.type === 'Identifier') {
context.state.transform[node.name] = { read: get_value };
return { id: node, declarations: null };
}
const pattern = /** @type {Pattern} */ (context.visit(node));
const identifiers = extract_identifiers(node);
const id = b.id('$$source');
const value = b.id('$$value');
const block = b.block([
b.var(pattern, b.call('$.get', id)),
b.return(b.object(identifiers.map((identifier) => b.prop('init', identifier, identifier))))
]);
const declarations = [b.var(value, create_derived(context.state, b.thunk(block)))];
for (const id of identifiers) {
context.state.transform[id.name] = { read: get_value };
declarations.push(
b.var(id, create_derived(context.state, b.thunk(b.member(b.call('$.get', value), id))))
);
}
return { id, declarations };
}

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

@ -537,8 +537,8 @@ function build_element_attribute_update_assignment(
const is_svg = context.state.metadata.namespace === 'svg' || element.name === 'svg';
const is_mathml = context.state.metadata.namespace === 'mathml';
let { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
get_expression_id(state, value)
let { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) =>
metadata.has_call ? get_expression_id(state, value) : value
);
if (name === 'autofocus') {
@ -665,8 +665,8 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co
*/
function build_element_special_value_attribute(element, node_id, attribute, context) {
const state = context.state;
const { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
get_expression_id(state, value)
const { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) =>
metadata.has_call ? get_expression_id(state, value) : value
);
const inner_assignment = b.assignment(

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

@ -30,8 +30,10 @@ export function SlotElement(node, context) {
if (attribute.type === 'SpreadAttribute') {
spreads.push(b.thunk(/** @type {Expression} */ (context.visit(attribute))));
} else if (attribute.type === 'Attribute') {
const { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
memoize_expression(context.state, value)
const { value, has_state } = build_attribute_value(
attribute.value,
context,
(value, metadata) => (metadata.has_call ? memoize_expression(context.state, value) : value)
);
if (attribute.name === 'name') {

47
packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js
Unescape View File

@ -4,7 +4,6 @@
import { dev, is_ignored } from '../../../../../state.js';
import { get_attribute_chunks, object } from '../../../../../utils/ast.js';
import * as b from '../../../../../utils/builders.js';
import { create_derived } from '../../utils.js';
import { build_bind_this, memoize_expression, validate_binding } from '../shared/utils.js';
import { build_attribute_value } from '../shared/element.js';
import { build_event_handler } from './events.js';
@ -134,9 +133,9 @@ export function build_component(node, component_name, context, anchor = context.
custom_css_props.push(
b.init(
attribute.name,
build_attribute_value(attribute.value, context, (value) =>
build_attribute_value(attribute.value, context, (value, metadata) =>
// TODO put the derived in the local block
memoize_expression(context.state, value)
metadata.has_call ? memoize_expression(context.state, value) : value
).value
)
);
@ -151,31 +150,29 @@ export function build_component(node, component_name, context, anchor = context.
has_children_prop = true;
}
const { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
memoize_expression(context.state, value)
);
if (has_state) {
let arg = value;
// When we have a non-simple computation, anything other than an Identifier or Member expression,
// then there's a good chance it needs to be memoized to avoid over-firing when read within the
// child component.
const should_wrap_in_derived = get_attribute_chunks(attribute.value).some((n) => {
return (
n.type === 'ExpressionTag' &&
n.expression.type !== 'Identifier' &&
n.expression.type !== 'MemberExpression'
);
});
const { value, has_state } = build_attribute_value(
attribute.value,
context,
(value, metadata) => {
if (!metadata.has_state) return value;
// When we have a non-simple computation, anything other than an Identifier or Member expression,
// then there's a good chance it needs to be memoized to avoid over-firing when read within the
// child component (e.g. `active={i === index}`)
const should_wrap_in_derived = get_attribute_chunks(attribute.value).some((n) => {
return (
n.type === 'ExpressionTag' &&
n.expression.type !== 'Identifier' &&
n.expression.type !== 'MemberExpression'
);
});
if (should_wrap_in_derived) {
const id = b.id(context.state.scope.generate(attribute.name));
context.state.init.push(b.var(id, create_derived(context.state, b.thunk(value))));
arg = b.call('$.get', id);
return should_wrap_in_derived ? memoize_expression(context.state, value) : value;
}
);
push_prop(b.get(attribute.name, [b.return(arg)]));
if (has_state) {
push_prop(b.get(attribute.name, [b.return(value)]));
} else {
push_prop(b.init(attribute.name, value));
}

23
packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js
Unescape View File

@ -1,11 +1,11 @@
/** @import { Expression, Identifier, ObjectExpression } from 'estree' */
/** @import { AST } from '#compiler' */
/** @import { AST, ExpressionMetadata } from '#compiler' */
/** @import { ComponentClientTransformState, ComponentContext } from '../../types' */
import { normalize_attribute } from '../../../../../../utils.js';
import { is_ignored } from '../../../../../state.js';
import { is_event_attribute } from '../../../../../utils/ast.js';
import * as b from '../../../../../utils/builders.js';
import { build_getter, create_derived } from '../../utils.js';
import { build_getter } from '../../utils.js';
import { build_template_chunk, get_expression_id } from './utils.js';
/**
@ -35,8 +35,10 @@ export function build_set_attributes(
for (const attribute of attributes) {
if (attribute.type === 'Attribute') {
const { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
get_expression_id(context.state, value)
const { value, has_state } = build_attribute_value(
attribute.value,
context,
(value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value)
);
if (
@ -59,10 +61,9 @@ export function build_set_attributes(
let value = /** @type {Expression} */ (context.visit(attribute));
if (attribute.metadata.expression.has_call) {
const id = b.id(state.scope.generate('spread_with_call'));
state.init.push(b.const(id, create_derived(state, b.thunk(value))));
value = b.call('$.get', id);
value = get_expression_id(context.state, value);
}
values.push(b.spread(value));
}
}
@ -111,8 +112,8 @@ export function build_style_directives(
let value =
directive.value === true
? build_getter({ name: directive.name, type: 'Identifier' }, context.state)
: build_attribute_value(directive.value, context, (value) =>
get_expression_id(context.state, value)
: build_attribute_value(directive.value, context, (value, metadata) =>
metadata.has_call ? get_expression_id(context.state, value) : value
).value;
const update = b.stmt(
@ -169,7 +170,7 @@ export function build_class_directives(
/**
* @param {AST.Attribute['value']} value
* @param {ComponentContext} context
* @param {(value: Expression) => Expression} memoize
* @param {(value: Expression, metadata: ExpressionMetadata) => Expression} memoize
* @returns {{ value: Expression, has_state: boolean }}
*/
export function build_attribute_value(value, context, memoize = (value) => value) {
@ -187,7 +188,7 @@ export function build_attribute_value(value, context, memoize = (value) => value
let expression = /** @type {Expression} */ (context.visit(chunk.expression));
return {
value: chunk.metadata.expression.has_call ? memoize(expression) : expression,
value: memoize(expression, chunk.metadata.expression),
has_state: chunk.metadata.expression.has_state
};
}

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

@ -1,5 +1,5 @@
/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, SequenceExpression, Statement, Super } from 'estree' */
/** @import { AST } from '#compiler' */
/** @import { AST, ExpressionMetadata } from '#compiler' */
/** @import { ComponentClientTransformState } from '../../types' */
import { walk } from 'zimmerframe';
import { object } from '../../../../../utils/ast.js';
@ -79,14 +79,14 @@ function compare_expressions(a, b) {
* @param {Array<AST.Text | AST.ExpressionTag>} values
* @param {(node: AST.SvelteNode, state: any) => any} visit
* @param {ComponentClientTransformState} state
* @param {(value: Expression) => Expression} memoize
* @param {(value: Expression, metadata: ExpressionMetadata) => Expression} memoize
* @returns {{ value: Expression, has_state: boolean }}
*/
export function build_template_chunk(
values,
visit,
state,
memoize = (value) => get_expression_id(state, value)
memoize = (value, metadata) => (metadata.has_call ? get_expression_id(state, value) : value)
) {
/** @type {Expression[]} */
const expressions = [];
@ -106,14 +106,13 @@ export function build_template_chunk(
quasi.value.cooked += node.expression.value + '';
}
} else {
let value = /** @type {Expression} */ (visit(node.expression, state));
let value = memoize(
/** @type {Expression} */ (visit(node.expression, state)),
node.metadata.expression
);
has_state ||= node.metadata.expression.has_state;
if (node.metadata.expression.has_call) {
value = memoize(value);
}
if (values.length === 1) {
// If we have a single expression, then pass that in directly to possibly avoid doing
// extra work in the template_effect (instead we do the work in set_text).

Write Preview
Loading…
Cancel
Save