Skip to content

fix: more accurate default value handling #11183

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.

Sign up for GitHub

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 1 commit into from
Apr 16, 2024
Merged

fix: more accurate default value handling #11183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/selfish-socks-smile.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: more accurate default value handling
104 changes: 6 additions & 98 deletions packages/svelte/src/compiler/phases/3-transform/client/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import * as b from '../../../utils/builders.js';
import { extract_paths, is_simple_expression, object } from '../../../utils/ast.js';
import {
extract_paths,
is_expression_async,
is_simple_expression,
object
} from '../../../utils/ast.js';
import { error } from '../../../errors.js';
import {
PROPS_IS_LAZY_INITIAL,
Expand Down Expand Up @@ -115,103 +120,6 @@ export function serialize_get_binding(node, state) {
return node;
}

/**
* @param {import('estree').Expression | import('estree').Pattern} expression
* @returns {boolean}
*/
function is_expression_async(expression) {
switch (expression.type) {
case 'AwaitExpression': {
return true;
}
case 'ArrayPattern': {
return expression.elements.some((element) => element && is_expression_async(element));
}
case 'ArrayExpression': {
return expression.elements.some((element) => {
if (!element) {
return false;
} else if (element.type === 'SpreadElement') {
return is_expression_async(element.argument);
} else {
return is_expression_async(element);
}
});
}
case 'AssignmentPattern':
case 'AssignmentExpression':
case 'BinaryExpression':
case 'LogicalExpression': {
return is_expression_async(expression.left) || is_expression_async(expression.right);
}
case 'CallExpression':
case 'NewExpression': {
return (
(expression.callee.type !== 'Super' && is_expression_async(expression.callee)) ||
expression.arguments.some((element) => {
if (element.type === 'SpreadElement') {
return is_expression_async(element.argument);
} else {
return is_expression_async(element);
}
})
);
}
case 'ChainExpression': {
return is_expression_async(expression.expression);
}
case 'ConditionalExpression': {
return (
is_expression_async(expression.test) ||
is_expression_async(expression.alternate) ||
is_expression_async(expression.consequent)
);
}
case 'ImportExpression': {
return is_expression_async(expression.source);
}
case 'MemberExpression': {
return (
(expression.object.type !== 'Super' && is_expression_async(expression.object)) ||
(expression.property.type !== 'PrivateIdentifier' &&
is_expression_async(expression.property))
);
}
case 'ObjectPattern':
case 'ObjectExpression': {
return expression.properties.some((property) => {
if (property.type === 'SpreadElement') {
return is_expression_async(property.argument);
} else if (property.type === 'Property') {
return (
(property.key.type !== 'PrivateIdentifier' && is_expression_async(property.key)) ||
is_expression_async(property.value)
);
}
});
}
case 'RestElement': {
return is_expression_async(expression.argument);
}
case 'SequenceExpression':
case 'TemplateLiteral': {
return expression.expressions.some((subexpression) => is_expression_async(subexpression));
}
case 'TaggedTemplateExpression': {
return is_expression_async(expression.tag) || is_expression_async(expression.quasi);
}
case 'UnaryExpression':
case 'UpdateExpression': {
return is_expression_async(expression.argument);
}
case 'YieldExpression': {
return expression.argument ? is_expression_async(expression.argument) : false;
}
default:
return false;
}
}

/**
* @template {import('./types').ClientTransformState} State
* @param {import('estree').AssignmentExpression} node
Expand Down
46 changes: 21 additions & 25 deletions packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2277,27 +2277,24 @@ export const template_visitors = {
for (const path of paths) {
const name = /** @type {import('estree').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 {import('estree').Expression} */ (context.visit(path.expression?.(unwrapped)))
);

declarations.push(
b.let(
path.node,
b.thunk(
/** @type {import('estree').Expression} */ (
context.visit(path.expression?.(unwrapped))
)
)
)
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);
binding.mutation = create_mutation(
/** @type {import('estree').Pattern} */ (path.update_expression(unwrapped))
);

// 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(b.call(name)));
declarations.push(b.stmt(binding.expression));
}

binding.expression = b.call(name);
binding.mutation = create_mutation(
/** @type {import('estree').Pattern} */ (path.update_expression(unwrapped))
);
}
}

Expand Down Expand Up @@ -2486,24 +2483,23 @@ export const template_visitors = {
for (const path of paths) {
const name = /** @type {import('estree').Identifier} */ (path.node).name;
const binding = /** @type {import('#compiler').Binding} */ (context.state.scope.get(name));
declarations.push(
b.let(
path.node,
b.thunk(
/** @type {import('estree').Expression} */ (
context.visit(path.expression?.(b.maybe_call(b.id(arg_alias))))
)
)
const needs_derived = path.has_default_value; // to ensure that default value is only called once
const fn = b.thunk(
/** @type {import('estree').Expression} */ (
context.visit(path.expression?.(b.maybe_call(b.id(arg_alias))))
)
);

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);

// 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(b.call(name)));
declarations.push(b.stmt(binding.expression));
}

binding.expression = b.call(name);
}
}

Expand Down
21 changes: 13 additions & 8 deletions packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
extract_identifiers,
extract_paths,
is_event_attribute,
is_expression_async,
unwrap_optional
} from '../../../utils/ast.js';
import * as b from '../../../utils/builders.js';
Expand Down Expand Up @@ -1191,7 +1192,9 @@ const javascript_visitors_legacy = {
const name = /** @type {import('estree').Identifier} */ (path.node).name;
const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(name));
const prop = b.member(b.id('$$props'), b.literal(binding.prop_alias ?? name), true);
declarations.push(b.declarator(path.node, b.call('$.value_or_fallback', prop, value)));
declarations.push(
b.declarator(path.node, b.call('$.value_or_fallback', prop, b.thunk(value)))
);
}
continue;
}
Expand All @@ -1204,13 +1207,15 @@ const javascript_visitors_legacy = {
b.literal(binding.prop_alias ?? declarator.id.name),
true
);
const init = declarator.init
? b.call(
'$.value_or_fallback',
prop,
/** @type {import('estree').Expression} */ (visit(declarator.init))
)
: prop;

/** @type {import('estree').Expression} */
let init = prop;
if (declarator.init) {
const default_value = /** @type {import('estree').Expression} */ (visit(declarator.init));
init = is_expression_async(default_value)
? b.await(b.call('$.value_or_fallback_async', prop, b.thunk(default_value, true)))
: b.call('$.value_or_fallback', prop, b.thunk(default_value));
}

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

Expand Down
Loading