Skip to content

Commit b5e23a6

Browse files
Rich-HarrisRich-Harris
authored
chore: tweak memoizer logic (#17042)
1 parent fc181cf commit b5e23a6

File tree

7 files changed

+55
-38
lines changed

7 files changed

+55
-38
lines changed

‎.changeset/slimy-shirts-lose.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
chore: tweak memoizer logic

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js‎

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
import { is_ignored } from '../../../../state.js';
1212
import { is_event_attribute, is_text_attribute } from '../../../../utils/ast.js';
1313
import * as b from '#compiler/builders';
14-
import { create_attribute, is_custom_element_node } from '../../../nodes.js';
14+
import { create_attribute, ExpressionMetadata, is_custom_element_node } from '../../../nodes.js';
1515
import { clean_nodes, determine_namespace_for_children } from '../../utils.js';
1616
import { build_getter } from '../utils.js';
1717
import {
@@ -267,10 +267,7 @@ export function RegularElement(node, context) {
267267
const { value, has_state } = build_attribute_value(
268268
attribute.value,
269269
context,
270-
(value, metadata) =>
271-
metadata.has_call || metadata.has_await
272-
? context.state.memoizer.add(value, metadata.has_await)
273-
: value
270+
(value, metadata) => context.state.memoizer.add(value, metadata)
274271
);
275272

276273
const update = build_element_attribute_update(node, node_id, name, value, attributes);
@@ -487,38 +484,51 @@ function setup_select_synchronization(value_binding, context) {
487484
);
488485
}
489486

487+
/**
488+
* @param {ExpressionMetadata} target
489+
* @param {ExpressionMetadata} source
490+
*/
491+
function merge_metadata(target, source) {
492+
target.has_assignment ||= source.has_assignment;
493+
target.has_await ||= source.has_await;
494+
target.has_call ||= source.has_call;
495+
target.has_member_expression ||= source.has_member_expression;
496+
target.has_state ||= source.has_state;
497+
498+
for (const r of source.references) target.references.add(r);
499+
for (const b of source.dependencies) target.dependencies.add(b);
500+
}
501+
490502
/**
491503
* @param {AST.ClassDirective[]} class_directives
492504
* @param {ComponentContext} context
493505
* @param {Memoizer} memoizer
494-
* @return {ObjectExpression | Identifier}
495506
*/
496507
export function build_class_directives_object(
497508
class_directives,
498509
context,
499510
memoizer = context.state.memoizer
500511
) {
501512
let properties = [];
502-
let has_call_or_state = false;
503-
let has_await = false;
513+
514+
const metadata = new ExpressionMetadata();
504515

505516
for (const d of class_directives) {
517+
merge_metadata(metadata, d.metadata.expression);
518+
506519
const expression = /** @type Expression */ (context.visit(d.expression));
507520
properties.push(b.init(d.name, expression));
508-
has_call_or_state ||= d.metadata.expression.has_call || d.metadata.expression.has_state;
509-
has_await ||= d.metadata.expression.has_await;
510521
}
511522

512523
const directives = b.object(properties);
513524

514-
return has_call_or_state || has_await ? memoizer.add(directives, has_await) : directives;
525+
return memoizer.add(directives, metadata);
515526
}
516527

517528
/**
518529
* @param {AST.StyleDirective[]} style_directives
519530
* @param {ComponentContext} context
520531
* @param {Memoizer} memoizer
521-
* @return {ObjectExpression | ArrayExpression | Identifier}}
522532
*/
523533
export function build_style_directives_object(
524534
style_directives,
@@ -528,25 +538,23 @@ export function build_style_directives_object(
528538
const normal = b.object([]);
529539
const important = b.object([]);
530540

531-
let has_call_or_state = false;
532-
let has_await = false;
541+
const metadata = new ExpressionMetadata();
533542

534543
for (const d of style_directives) {
544+
merge_metadata(metadata, d.metadata.expression);
545+
535546
const expression =
536547
d.value === true
537548
? build_getter(b.id(d.name), context.state)
538549
: build_attribute_value(d.value, context).value;
539550

540551
const object = d.modifiers.includes('important') ? important : normal;
541552
object.properties.push(b.init(d.name, expression));
542-
543-
has_call_or_state ||= d.metadata.expression.has_call || d.metadata.expression.has_state;
544-
has_await ||= d.metadata.expression.has_await;
545553
}
546554

547555
const directives = important.properties.length ? b.array([normal, important]) : normal;
548556

549-
return has_call_or_state || has_await ? memoizer.add(directives, has_await) : directives;
557+
return memoizer.add(directives, metadata);
550558
}
551559

552560
/**
@@ -675,7 +683,7 @@ function build_element_special_value_attribute(
675683
element === 'select' && attribute.value !== true && !is_text_attribute(attribute);
676684

677685
const { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) =>
678-
metadata.has_call || metadata.has_await ? state.memoizer.add(value, metadata.has_await) : value
686+
state.memoizer.add(value, metadata)
679687
);
680688

681689
const evaluated = context.state.scope.evaluate(value);

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/RenderTag.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export function RenderTag(node, context) {
2626
let expression = build_expression(context, arg, metadata);
2727

2828
if (metadata.has_await || metadata.has_call) {
29-
expression = b.call('$.get', memoizer.add(expression, metadata.has_await));
29+
expression = b.call('$.get', memoizer.add(expression, metadata));
3030
}
3131

3232
args.push(b.thunk(expression));

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export function SlotElement(node, context) {
3535
context,
3636
(value, metadata) =>
3737
metadata.has_call || metadata.has_await
38-
? b.call('$.get', memoizer.add(value, metadata.has_await))
38+
? b.call('$.get', memoizer.add(value, metadata))
3939
: value
4040
);
4141

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ export function build_component(node, component_name, context) {
134134
props_and_spreads.push(
135135
b.thunk(
136136
attribute.metadata.expression.has_await || attribute.metadata.expression.has_call
137-
? b.call('$.get', memoizer.add(expression, attribute.metadata.expression.has_await))
137+
? b.call('$.get', memoizer.add(expression, attribute.metadata.expression))
138138
: expression
139139
)
140140
);
@@ -149,7 +149,7 @@ export function build_component(node, component_name, context) {
149149
build_attribute_value(attribute.value, context, (value, metadata) => {
150150
// TODO put the derived in the local block
151151
return metadata.has_call || metadata.has_await
152-
? b.call('$.get', memoizer.add(value, metadata.has_await))
152+
? b.call('$.get', memoizer.add(value, metadata))
153153
: value;
154154
}).value
155155
)
@@ -185,7 +185,7 @@ export function build_component(node, component_name, context) {
185185
});
186186

187187
return should_wrap_in_derived
188-
? b.call('$.get', memoizer.add(value, metadata.has_await))
188+
? b.call('$.get', memoizer.add(value, metadata, true))
189189
: value;
190190
}
191191
);

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export function build_attribute_effect(
3636
for (const attribute of attributes) {
3737
if (attribute.type === 'Attribute') {
3838
const { value } = build_attribute_value(attribute.value, context, (value, metadata) =>
39-
metadata.has_call || metadata.has_await ? memoizer.add(value, metadata.has_await) : value
39+
memoizer.add(value, metadata)
4040
);
4141

4242
if (
@@ -53,9 +53,7 @@ export function build_attribute_effect(
5353
} else {
5454
let value = /** @type {Expression} */ (context.visit(attribute));
5555

56-
if (attribute.metadata.expression.has_call || attribute.metadata.expression.has_await) {
57-
value = memoizer.add(value, attribute.metadata.expression.has_await);
58-
}
56+
value = memoizer.add(value, attribute.metadata.expression);
5957

6058
values.push(b.spread(value));
6159
}
@@ -156,9 +154,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c
156154
value = b.call('$.clsx', value);
157155
}
158156

159-
return metadata.has_call || metadata.has_await
160-
? context.state.memoizer.add(value, metadata.has_await)
161-
: value;
157+
return context.state.memoizer.add(value, metadata);
162158
});
163159

164160
/** @type {Identifier | undefined} */
@@ -167,7 +163,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c
167163
/** @type {ObjectExpression | Identifier | undefined} */
168164
let prev;
169165

170-
/** @type {ObjectExpression | Identifier | undefined} */
166+
/** @type {Expression | undefined} */
171167
let next;
172168

173169
if (class_directives.length) {
@@ -228,7 +224,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c
228224
*/
229225
export function build_set_style(node_id, attribute, style_directives, context) {
230226
let { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) =>
231-
metadata.has_call ? context.state.memoizer.add(value, metadata.has_await) : value
227+
context.state.memoizer.add(value, metadata)
232228
);
233229

234230
/** @type {Identifier | undefined} */

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,21 @@ export class Memoizer {
2424

2525
/**
2626
* @param {Expression} expression
27-
* @param {boolean} has_await
27+
* @param {ExpressionMetadata} metadata
28+
* @param {boolean} memoize_if_state
2829
*/
29-
add(expression, has_await) {
30+
add(expression, metadata, memoize_if_state = false) {
31+
const should_memoize =
32+
metadata.has_call || metadata.has_await || (memoize_if_state && metadata.has_state);
33+
34+
if (!should_memoize) {
35+
// no memoization required
36+
return expression;
37+
}
38+
3039
const id = b.id('#'); // filled in later
3140

32-
(has_await ? this.#async : this.#sync).push({ id, expression });
41+
(metadata.has_await ? this.#async : this.#sync).push({ id, expression });
3342

3443
return id;
3544
}
@@ -73,8 +82,7 @@ export function build_template_chunk(
7382
values,
7483
context,
7584
state = context.state,
76-
memoize = (value, metadata) =>
77-
metadata.has_call || metadata.has_await ? state.memoizer.add(value, metadata.has_await) : value
85+
memoize = (value, metadata) => state.memoizer.add(value, metadata)
7886
) {
7987
/** @type {Expression[]} */
8088
const expressions = [];

0 commit comments

Comments
 (0)