Skip to content

Commit

Permalink
chore: remove unnecessary ?? '' on some expressions (#15287)
Browse files Browse the repository at this point in the history
* no `?? ''` on some expressions

* changeset

* delete operator returns boolean

* inverted conditions are a lil confusing

* no need for else after return

* simplify condition

* may as well add special handling for undefined while we're here

* use normal string literal when there are no values

* omit assignment when there is no text content

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
  • Loading branch information
adiguba and Rich-Harris authored Feb 21, 2025
1 parent 60b22c0 commit 1e1aea4
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 33 deletions.
5 changes: 5 additions & 0 deletions .changeset/serious-glasses-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

chore: remove unnecessary `?? ''` on some expressions
Original file line number Diff line number Diff line change
Expand Up @@ -364,15 +364,14 @@ export function RegularElement(node, context) {
trimmed.some((node) => node.type === 'ExpressionTag');

if (use_text_content) {
child_state.init.push(
b.stmt(
b.assignment(
'=',
b.member(context.state.node, 'textContent'),
build_template_chunk(trimmed, context.visit, child_state).value
)
)
);
const { value } = build_template_chunk(trimmed, context.visit, child_state);
const empty_string = value.type === 'Literal' && value.value === '';

if (!empty_string) {
child_state.init.push(
b.stmt(b.assignment('=', b.member(context.state.node, 'textContent'), value))
);
}
} else {
/** @type {Expression} */
let arg = context.state.node;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,15 @@ export function build_template_chunk(

if (node.type === 'Text') {
quasi.value.cooked += node.data;
} else if (node.type === 'ExpressionTag' && node.expression.type === 'Literal') {
} else if (node.expression.type === 'Literal') {
if (node.expression.value != null) {
quasi.value.cooked += node.expression.value + '';
}
} else {
} else if (
node.expression.type !== 'Identifier' ||
node.expression.name !== 'undefined' ||
state.scope.get('undefined')
) {
let value = memoize(
/** @type {Expression} */ (visit(node.expression, state)),
node.metadata.expression
Expand All @@ -117,31 +121,33 @@ export function build_template_chunk(
// 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).
return { value, has_state };
} else {
// add `?? ''` where necessary (TODO optimise more cases)
if (
value.type === 'LogicalExpression' &&
value.right.type === 'Literal' &&
(value.operator === '??' || value.operator === '||')
) {
// `foo ?? null` -=> `foo ?? ''`
// otherwise leave the expression untouched
if (value.right.value === null) {
value = { ...value, right: b.literal('') };
}
} else if (
state.analysis.props_id &&
value.type === 'Identifier' &&
value.name === state.analysis.props_id.name
) {
// do nothing ($props.id() is never null/undefined)
} else {
value = b.logical('??', value, b.literal(''));
}

if (
value.type === 'LogicalExpression' &&
value.right.type === 'Literal' &&
(value.operator === '??' || value.operator === '||')
) {
// `foo ?? null` -=> `foo ?? ''`
// otherwise leave the expression untouched
if (value.right.value === null) {
value = { ...value, right: b.literal('') };
}
}

const is_defined =
value.type === 'BinaryExpression' ||
(value.type === 'UnaryExpression' && value.operator !== 'void') ||
(value.type === 'LogicalExpression' && value.right.type === 'Literal') ||
(value.type === 'Identifier' && value.name === state.analysis.props_id?.name);

expressions.push(value);
if (!is_defined) {
// add `?? ''` where necessary (TODO optimise more cases)
value = b.logical('??', value, b.literal(''));
}

expressions.push(value);

quasi = b.quasi('', i + 1 === values.length);
quasis.push(quasi);
}
Expand All @@ -151,7 +157,10 @@ export function build_template_chunk(
quasi.value.raw = sanitize_template_string(/** @type {string} */ (quasi.value.cooked));
}

const value = b.template(quasis, expressions);
const value =
expressions.length > 0
? b.template(quasis, expressions)
: b.literal(/** @type {string} */ (quasi.value.cooked));

return { value, has_state };
}
Expand Down

0 comments on commit 1e1aea4

Please sign in to comment.