From 8f152de6dc1b452d44c3b2ebc7795f2d8f755afc Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Wed, 28 May 2025 23:36:52 +0200 Subject: [PATCH 1/2] fix: falsy attachments on components --- .changeset/fuzzy-taxis-hunt.md | 5 +++++ .../3-transform/client/visitors/shared/component.js | 5 ++++- packages/svelte/src/internal/client/index.js | 2 +- packages/svelte/src/internal/shared/utils.js | 10 ++++++++++ .../samples/attachment-component-falsy/Child.svelte | 5 +++++ .../samples/attachment-component-falsy/_config.js | 5 +++++ .../samples/attachment-component-falsy/main.svelte | 13 +++++++++++++ 7 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 .changeset/fuzzy-taxis-hunt.md create mode 100644 packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/main.svelte diff --git a/.changeset/fuzzy-taxis-hunt.md b/.changeset/fuzzy-taxis-hunt.md new file mode 100644 index 000000000000..5deda96ce53e --- /dev/null +++ b/.changeset/fuzzy-taxis-hunt.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: falsy attachments on components diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index 57402c2b4ac0..3aa7f486ea3a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -261,7 +261,10 @@ export function build_component(node, component_name, context, anchor = context. let expression = /** @type {Expression} */ (context.visit(attribute.expression)); if (attribute.metadata.expression.has_state) { - expression = b.arrow([b.id('$$node')], b.call(expression, b.id('$$node'))); + expression = b.arrow( + [b.id('$$node')], + b.call(b.call('$.safe_call', expression), b.id('$$node')) + ); } push_prop(b.prop('get', b.call('$.attachment'), expression, true)); diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 71c06d7b1b8b..269dcfd17b9f 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -155,7 +155,7 @@ export { } from './dom/operations.js'; export { attr, clsx } from '../shared/attributes.js'; export { snapshot } from '../shared/clone.js'; -export { noop, fallback, to_array } from '../shared/utils.js'; +export { noop, fallback, safe_call, to_array } from '../shared/utils.js'; export { invalid_default_snippet, validate_dynamic_element_tag, diff --git a/packages/svelte/src/internal/shared/utils.js b/packages/svelte/src/internal/shared/utils.js index 10f8597520d9..59f9bbfd0ed3 100644 --- a/packages/svelte/src/internal/shared/utils.js +++ b/packages/svelte/src/internal/shared/utils.js @@ -82,6 +82,16 @@ export function fallback(value, fallback, lazy = false) { : value; } +/** + * @param {*} value + */ +export function safe_call(value) { + if (is_function(value)) { + return value; + } + return noop; +} + /** * When encountering a situation like `let [a, b, c] = $derived(blah())`, * we need to stash an intermediate value that `a`, `b`, and `c` derive diff --git a/packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/Child.svelte b/packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/Child.svelte new file mode 100644 index 000000000000..6760da61faeb --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/Child.svelte @@ -0,0 +1,5 @@ + + +
diff --git a/packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/_config.js b/packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/_config.js new file mode 100644 index 000000000000..27f013d6d179 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/_config.js @@ -0,0 +1,5 @@ +import { test } from '../../test'; + +export default test({ + test() {} +}); diff --git a/packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/main.svelte b/packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/main.svelte new file mode 100644 index 000000000000..993bcdd6a561 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/attachment-component-falsy/main.svelte @@ -0,0 +1,13 @@ + + + + + From a34c2337c07eb1423df82de3ff00b0df1a04edb3 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 28 May 2025 18:49:35 -0400 Subject: [PATCH 2/2] skip the noop if known to be a function --- .../client/visitors/shared/component.js | 7 ++++++- packages/svelte/src/compiler/phases/scope.js | 21 ++++++++++++++++++- packages/svelte/src/internal/client/index.js | 2 +- packages/svelte/src/internal/shared/utils.js | 10 --------- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index 3aa7f486ea3a..ff98d6d3787b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -258,12 +258,17 @@ export function build_component(node, component_name, context, anchor = context. } } } else if (attribute.type === 'AttachTag') { + const evaluated = context.state.scope.evaluate(attribute.expression); + let expression = /** @type {Expression} */ (context.visit(attribute.expression)); if (attribute.metadata.expression.has_state) { expression = b.arrow( [b.id('$$node')], - b.call(b.call('$.safe_call', expression), b.id('$$node')) + b.call( + evaluated.is_function ? expression : b.logical('||', expression, b.id('$.noop')), + b.id('$$node') + ) ); } diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 75a26d487b78..b4cdb6b44673 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -20,6 +20,7 @@ const UNKNOWN = Symbol('unknown'); /** Includes `BigInt` */ export const NUMBER = Symbol('number'); export const STRING = Symbol('string'); +export const FUNCTION = Symbol('string'); /** @type {Record} */ const globals = { @@ -200,6 +201,13 @@ class Evaluation { */ is_number = true; + /** + * True if the value is known to be a function + * @readonly + * @type {boolean} + */ + is_function = true; + /** * @readonly * @type {any} @@ -209,7 +217,7 @@ class Evaluation { /** * * @param {Scope} scope - * @param {Expression} expression + * @param {Expression | FunctionDeclaration} expression * @param {Set} values */ constructor(scope, expression, values) { @@ -500,6 +508,13 @@ class Evaluation { break; } + case 'ArrowFunctionExpression': + case 'FunctionExpression': + case 'FunctionDeclaration': { + this.values.add(FUNCTION); + break; + } + default: { this.values.add(UNKNOWN); } @@ -516,6 +531,10 @@ class Evaluation { this.is_number = false; } + if (value !== FUNCTION) { + this.is_function = false; + } + if (value == null || value === UNKNOWN) { this.is_defined = false; } diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 269dcfd17b9f..71c06d7b1b8b 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -155,7 +155,7 @@ export { } from './dom/operations.js'; export { attr, clsx } from '../shared/attributes.js'; export { snapshot } from '../shared/clone.js'; -export { noop, fallback, safe_call, to_array } from '../shared/utils.js'; +export { noop, fallback, to_array } from '../shared/utils.js'; export { invalid_default_snippet, validate_dynamic_element_tag, diff --git a/packages/svelte/src/internal/shared/utils.js b/packages/svelte/src/internal/shared/utils.js index 59f9bbfd0ed3..10f8597520d9 100644 --- a/packages/svelte/src/internal/shared/utils.js +++ b/packages/svelte/src/internal/shared/utils.js @@ -82,16 +82,6 @@ export function fallback(value, fallback, lazy = false) { : value; } -/** - * @param {*} value - */ -export function safe_call(value) { - if (is_function(value)) { - return value; - } - return noop; -} - /** * When encountering a situation like `let [a, b, c] = $derived(blah())`, * we need to stash an intermediate value that `a`, `b`, and `c` derive