Skip to content

Commit cd7c3fe

Browse files
dummdidummdummdidumm
authored
fix: more accurate default value handling (#11183)
- don't call fallback values eagerly, only when it's actually needed. Avoids potential unwanted side effects - use derived_safe_equals to memoize results of destructured snippet/each context values with default values to ensure they're only recalculated when their dependencies change. Avoids unstable default values getting called multiple times yielding different results. Use derived_safe_equals to ensure new values are always set, even when mutated. fixes #11143
1 parent ae7d734 commit cd7c3fe

File tree

15 files changed

+296
-154
lines changed

15 files changed

+296
-154
lines changed

‎.changeset/selfish-socks-smile.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+
fix: more accurate default value handling

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

Lines changed: 6 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import * as b from '../../../utils/builders.js';
2-
import { extract_paths, is_simple_expression, object } from '../../../utils/ast.js';
2+
import {
3+
extract_paths,
4+
is_expression_async,
5+
is_simple_expression,
6+
object
7+
} from '../../../utils/ast.js';
38
import { error } from '../../../errors.js';
49
import {
510
PROPS_IS_LAZY_INITIAL,
@@ -115,103 +120,6 @@ export function serialize_get_binding(node, state) {
115120
return node;
116121
}
117122

118-
/**
119-
* @param {import('estree').Expression | import('estree').Pattern} expression
120-
* @returns {boolean}
121-
*/
122-
function is_expression_async(expression) {
123-
switch (expression.type) {
124-
case 'AwaitExpression': {
125-
return true;
126-
}
127-
case 'ArrayPattern': {
128-
return expression.elements.some((element) => element && is_expression_async(element));
129-
}
130-
case 'ArrayExpression': {
131-
return expression.elements.some((element) => {
132-
if (!element) {
133-
return false;
134-
} else if (element.type === 'SpreadElement') {
135-
return is_expression_async(element.argument);
136-
} else {
137-
return is_expression_async(element);
138-
}
139-
});
140-
}
141-
case 'AssignmentPattern':
142-
case 'AssignmentExpression':
143-
case 'BinaryExpression':
144-
case 'LogicalExpression': {
145-
return is_expression_async(expression.left) || is_expression_async(expression.right);
146-
}
147-
case 'CallExpression':
148-
case 'NewExpression': {
149-
return (
150-
(expression.callee.type !== 'Super' && is_expression_async(expression.callee)) ||
151-
expression.arguments.some((element) => {
152-
if (element.type === 'SpreadElement') {
153-
return is_expression_async(element.argument);
154-
} else {
155-
return is_expression_async(element);
156-
}
157-
})
158-
);
159-
}
160-
case 'ChainExpression': {
161-
return is_expression_async(expression.expression);
162-
}
163-
case 'ConditionalExpression': {
164-
return (
165-
is_expression_async(expression.test) ||
166-
is_expression_async(expression.alternate) ||
167-
is_expression_async(expression.consequent)
168-
);
169-
}
170-
case 'ImportExpression': {
171-
return is_expression_async(expression.source);
172-
}
173-
case 'MemberExpression': {
174-
return (
175-
(expression.object.type !== 'Super' && is_expression_async(expression.object)) ||
176-
(expression.property.type !== 'PrivateIdentifier' &&
177-
is_expression_async(expression.property))
178-
);
179-
}
180-
case 'ObjectPattern':
181-
case 'ObjectExpression': {
182-
return expression.properties.some((property) => {
183-
if (property.type === 'SpreadElement') {
184-
return is_expression_async(property.argument);
185-
} else if (property.type === 'Property') {
186-
return (
187-
(property.key.type !== 'PrivateIdentifier' && is_expression_async(property.key)) ||
188-
is_expression_async(property.value)
189-
);
190-
}
191-
});
192-
}
193-
case 'RestElement': {
194-
return is_expression_async(expression.argument);
195-
}
196-
case 'SequenceExpression':
197-
case 'TemplateLiteral': {
198-
return expression.expressions.some((subexpression) => is_expression_async(subexpression));
199-
}
200-
case 'TaggedTemplateExpression': {
201-
return is_expression_async(expression.tag) || is_expression_async(expression.quasi);
202-
}
203-
case 'UnaryExpression':
204-
case 'UpdateExpression': {
205-
return is_expression_async(expression.argument);
206-
}
207-
case 'YieldExpression': {
208-
return expression.argument ? is_expression_async(expression.argument) : false;
209-
}
210-
default:
211-
return false;
212-
}
213-
}
214-
215123
/**
216124
* @template {import('./types').ClientTransformState} State
217125
* @param {import('estree').AssignmentExpression} node

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

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2277,27 +2277,24 @@ export const template_visitors = {
22772277
for (const path of paths) {
22782278
const name = /** @type {import('estree').Identifier} */ (path.node).name;
22792279
const binding = /** @type {import('#compiler').Binding} */ (context.state.scope.get(name));
2280+
const needs_derived = path.has_default_value; // to ensure that default value is only called once
2281+
const fn = b.thunk(
2282+
/** @type {import('estree').Expression} */ (context.visit(path.expression?.(unwrapped)))
2283+
);
2284+
22802285
declarations.push(
2281-
b.let(
2282-
path.node,
2283-
b.thunk(
2284-
/** @type {import('estree').Expression} */ (
2285-
context.visit(path.expression?.(unwrapped))
2286-
)
2287-
)
2288-
)
2286+
b.let(path.node, needs_derived ? b.call('$.derived_safe_equal', fn) : fn)
2287+
);
2288+
binding.expression = needs_derived ? b.call('$.get', b.id(name)) : b.call(name);
2289+
binding.mutation = create_mutation(
2290+
/** @type {import('estree').Pattern} */ (path.update_expression(unwrapped))
22892291
);
22902292

22912293
// we need to eagerly evaluate the expression in order to hit any
22922294
// 'Cannot access x before initialization' errors
22932295
if (context.state.options.dev) {
2294-
declarations.push(b.stmt(b.call(name)));
2296+
declarations.push(b.stmt(binding.expression));
22952297
}
2296-
2297-
binding.expression = b.call(name);
2298-
binding.mutation = create_mutation(
2299-
/** @type {import('estree').Pattern} */ (path.update_expression(unwrapped))
2300-
);
23012298
}
23022299
}
23032300

@@ -2486,24 +2483,23 @@ export const template_visitors = {
24862483
for (const path of paths) {
24872484
const name = /** @type {import('estree').Identifier} */ (path.node).name;
24882485
const binding = /** @type {import('#compiler').Binding} */ (context.state.scope.get(name));
2489-
declarations.push(
2490-
b.let(
2491-
path.node,
2492-
b.thunk(
2493-
/** @type {import('estree').Expression} */ (
2494-
context.visit(path.expression?.(b.maybe_call(b.id(arg_alias))))
2495-
)
2496-
)
2486+
const needs_derived = path.has_default_value; // to ensure that default value is only called once
2487+
const fn = b.thunk(
2488+
/** @type {import('estree').Expression} */ (
2489+
context.visit(path.expression?.(b.maybe_call(b.id(arg_alias))))
24972490
)
24982491
);
24992492

2493+
declarations.push(
2494+
b.let(path.node, needs_derived ? b.call('$.derived_safe_equal', fn) : fn)
2495+
);
2496+
binding.expression = needs_derived ? b.call('$.get', b.id(name)) : b.call(name);
2497+
25002498
// we need to eagerly evaluate the expression in order to hit any
25012499
// 'Cannot access x before initialization' errors
25022500
if (context.state.options.dev) {
2503-
declarations.push(b.stmt(b.call(name)));
2501+
declarations.push(b.stmt(binding.expression));
25042502
}
2505-
2506-
binding.expression = b.call(name);
25072503
}
25082504
}
25092505

‎packages/svelte/src/compiler/phases/3-transform/server/transform-server.js

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
extract_identifiers,
55
extract_paths,
66
is_event_attribute,
7+
is_expression_async,
78
unwrap_optional
89
} from '../../../utils/ast.js';
910
import * as b from '../../../utils/builders.js';
@@ -1191,7 +1192,9 @@ const javascript_visitors_legacy = {
11911192
const name = /** @type {import('estree').Identifier} */ (path.node).name;
11921193
const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(name));
11931194
const prop = b.member(b.id('$$props'), b.literal(binding.prop_alias ?? name), true);
1194-
declarations.push(b.declarator(path.node, b.call('$.value_or_fallback', prop, value)));
1195+
declarations.push(
1196+
b.declarator(path.node, b.call('$.value_or_fallback', prop, b.thunk(value)))
1197+
);
11951198
}
11961199
continue;
11971200
}
@@ -1204,13 +1207,15 @@ const javascript_visitors_legacy = {
12041207
b.literal(binding.prop_alias ?? declarator.id.name),
12051208
true
12061209
);
1207-
const init = declarator.init
1208-
? b.call(
1209-
'$.value_or_fallback',
1210-
prop,
1211-
/** @type {import('estree').Expression} */ (visit(declarator.init))
1212-
)
1213-
: prop;
1210+
1211+
/** @type {import('estree').Expression} */
1212+
let init = prop;
1213+
if (declarator.init) {
1214+
const default_value = /** @type {import('estree').Expression} */ (visit(declarator.init));
1215+
init = is_expression_async(default_value)
1216+
? b.await(b.call('$.value_or_fallback_async', prop, b.thunk(default_value, true)))
1217+
: b.call('$.value_or_fallback', prop, b.thunk(default_value));
1218+
}
12141219

12151220
declarations.push(b.declarator(declarator.id, init));
12161221

0 commit comments

Comments
 (0)