diff --git a/.changeset/eleven-cases-sing.md b/.changeset/eleven-cases-sing.md new file mode 100644 index 0000000000..fa6edda180 --- /dev/null +++ b/.changeset/eleven-cases-sing.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure async static props/attributes are awaited diff --git a/.changeset/five-coats-travel.md b/.changeset/five-coats-travel.md new file mode 100644 index 0000000000..f1235043df --- /dev/null +++ b/.changeset/five-coats-travel.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: wait on dependencies of async bindings diff --git a/.changeset/huge-walls-hang.md b/.changeset/huge-walls-hang.md new file mode 100644 index 0000000000..63dd6c21c4 --- /dev/null +++ b/.changeset/huge-walls-hang.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: await dependencies of style directives diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js index 38eab5d36e..aeae121278 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js @@ -159,6 +159,16 @@ export function BindDirective(node, context) { mark_subtree_dynamic(context.path); + const [get, set] = node.expression.expressions; + // We gotta jump across the getter/setter functions to avoid the expression metadata field being reset to null + context.visit(get.type === 'ArrowFunctionExpression' ? get.body : get, { + ...context.state, + expression: node.metadata.expression + }); + context.visit(set.type === 'ArrowFunctionExpression' ? set.body : set, { + ...context.state, + expression: node.metadata.expression + }); return; } @@ -247,7 +257,8 @@ export function BindDirective(node, context) { node.metadata = { binding_group_name: group_name, - parent_each_blocks: each_blocks + parent_each_blocks: each_blocks, + expression: node.metadata.expression }; } @@ -255,5 +266,5 @@ export function BindDirective(node, context) { w.bind_invalid_each_rest(binding.node, binding.node.name); } - context.next(); + context.next({ ...context.state, expression: node.metadata.expression }); } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js index 9699d3c03b..8f4036cf89 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js @@ -23,6 +23,9 @@ export function StyleDirective(node, context) { if (binding.kind !== 'normal') { node.metadata.expression.has_state = true; } + if (binding.blocker) { + node.metadata.expression.dependencies.add(binding); + } } } else { context.next(); @@ -30,9 +33,7 @@ export function StyleDirective(node, context) { for (const chunk of get_attribute_chunks(node.value)) { if (chunk.type !== 'ExpressionTag') continue; - node.metadata.expression.has_state ||= chunk.metadata.expression.has_state; - node.metadata.expression.has_call ||= chunk.metadata.expression.has_call; - node.metadata.expression.has_await ||= chunk.metadata.expression.has_await; + node.metadata.expression.merge(chunk.metadata.expression); } } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js index 9a264facce..12118d9f69 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js @@ -250,10 +250,13 @@ export function BindDirective(node, context) { let statement = defer ? b.stmt(b.call('$.effect', b.thunk(call))) : b.stmt(call); - // TODO this doesn't account for function bindings - if (node.metadata.binding?.blocker) { + if (node.metadata.expression.is_async()) { statement = b.stmt( - b.call(b.member(node.metadata.binding.blocker, b.id('then')), b.thunk(b.block([statement]))) + b.call( + '$.run_after_blockers', + node.metadata.expression.blockers(), + b.thunk(b.block([statement])) + ) ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 3998770a71..971f93e1d8 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -484,21 +484,6 @@ function setup_select_synchronization(value_binding, context) { ); } -/** - * @param {ExpressionMetadata} target - * @param {ExpressionMetadata} source - */ -function merge_metadata(target, source) { - target.has_assignment ||= source.has_assignment; - target.has_await ||= source.has_await; - target.has_call ||= source.has_call; - target.has_member_expression ||= source.has_member_expression; - target.has_state ||= source.has_state; - - for (const r of source.references) target.references.add(r); - for (const b of source.dependencies) target.dependencies.add(b); -} - /** * @param {AST.ClassDirective[]} class_directives * @param {ComponentContext} context @@ -514,7 +499,7 @@ export function build_class_directives_object( const metadata = new ExpressionMetadata(); for (const d of class_directives) { - merge_metadata(metadata, d.metadata.expression); + metadata.merge(d.metadata.expression); const expression = /** @type Expression */ (context.visit(d.expression)); properties.push(b.init(d.name, expression)); @@ -541,7 +526,7 @@ export function build_style_directives_object( const metadata = new ExpressionMetadata(); for (const d of style_directives) { - merge_metadata(metadata, d.metadata.expression); + metadata.merge(d.metadata.expression); const expression = d.value === true 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 e343662ed5..2b57f61c77 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 @@ -171,8 +171,6 @@ export function build_component(node, component_name, context) { attribute.value, context, (value, metadata) => { - if (!metadata.has_state && !metadata.has_await) return value; - // When we have a non-simple computation, anything other than an Identifier or Member expression, // then there's a good chance it needs to be memoized to avoid over-firing when read within the // child component (e.g. `active={i === index}`) @@ -198,7 +196,12 @@ export function build_component(node, component_name, context) { push_prop(b.init(attribute.name, value)); } } else if (attribute.type === 'BindDirective') { - const expression = /** @type {Expression} */ (context.visit(attribute.expression)); + const expression = /** @type {Expression} */ ( + context.visit(attribute.expression, { ...context.state, memoizer }) + ); + + // Bindings are a bit special: we don't want to add them to (async) deriveds but we need to check if they have blockers + memoizer.check_blockers(attribute.metadata.expression); if ( dev && diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js index 436d262d3a..58ed88855f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js @@ -122,7 +122,7 @@ export function build_attribute_value(value, context, memoize = (value) => value return { value: memoize(expression, chunk.metadata.expression), - has_state: chunk.metadata.expression.has_state || chunk.metadata.expression.has_await + has_state: chunk.metadata.expression.has_state || chunk.metadata.expression.is_async() }; } @@ -170,7 +170,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c if (class_directives.length) { next = build_class_directives_object(class_directives, context); has_state ||= class_directives.some( - (d) => d.metadata.expression.has_state || d.metadata.expression.has_await + (d) => d.metadata.expression.has_state || d.metadata.expression.is_async() ); if (has_state) { @@ -240,7 +240,7 @@ export function build_set_style(node_id, attribute, style_directives, context) { if (style_directives.length) { next = build_style_directives_object(style_directives, context); has_state ||= style_directives.some( - (d) => d.metadata.expression.has_state || d.metadata.expression.has_await + (d) => d.metadata.expression.has_state || d.metadata.expression.is_async() ); if (has_state) { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js index 83bc5da3e3..e7a4f6059b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js @@ -31,11 +31,7 @@ export class Memoizer { * @param {boolean} memoize_if_state */ add(expression, metadata, memoize_if_state = false) { - for (const binding of metadata.dependencies) { - if (binding.blocker) { - this.#blockers.add(binding.blocker); - } - } + this.check_blockers(metadata); const should_memoize = metadata.has_call || metadata.has_await || (memoize_if_state && metadata.has_state); @@ -52,6 +48,17 @@ export class Memoizer { return id; } + /** + * @param {ExpressionMetadata} metadata + */ + check_blockers(metadata) { + for (const binding of metadata.dependencies) { + if (binding.blocker) { + this.#blockers.add(binding.blocker); + } + } + } + apply() { return [...this.#sync, ...this.#async].map((memo, i) => { memo.id.name = `$${i}`; @@ -348,6 +355,7 @@ export function validate_binding(state, binding, expression) { b.call( '$.validate_binding', b.literal(state.analysis.source.slice(binding.start, binding.end)), + binding.metadata.expression.blockers(), b.thunk( state.store_to_invalidate ? b.sequence([b.call('$.mark_store_binding'), obj]) : obj ), diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js index 83a5dcf3ec..70c790fc57 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js @@ -107,6 +107,9 @@ export function build_inline_component(node, expression, context) { push_prop(b.prop('init', b.key(attribute.name), value)); } else if (attribute.type === 'BindDirective' && attribute.name !== 'this') { + // Bindings are a bit special: we don't want to add them to (async) deriveds but we need to check if they have blockers + optimiser.check_blockers(attribute.metadata.expression); + if (attribute.expression.type === 'SequenceExpression') { const [get, set] = /** @type {SequenceExpression} */ (context.visit(attribute.expression)) .expressions; diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js index b7607f3fb8..31ffe19139 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js @@ -121,6 +121,8 @@ export function build_element_attributes(node, context, transform) { expression = b.call(expression.expressions[0]); } + expression = transform(expression, attribute.metadata.expression); + if (is_content_editable_binding(attribute.name)) { content = expression; } else if (attribute.name === 'value' && node.name === 'textarea') { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js index 6bc0ab3233..4736a7c5da 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js @@ -347,16 +347,11 @@ export class PromiseOptimiser { #blockers = new Set(); /** - * * @param {Expression} expression * @param {ExpressionMetadata} metadata */ transform = (expression, metadata) => { - for (const binding of metadata.dependencies) { - if (binding.blocker) { - this.#blockers.add(binding.blocker); - } - } + this.check_blockers(metadata); if (metadata.has_await) { this.has_await = true; @@ -368,6 +363,17 @@ export class PromiseOptimiser { return expression; }; + /** + * @param {ExpressionMetadata} metadata + */ + check_blockers(metadata) { + for (const binding of metadata.dependencies) { + if (binding.blocker) { + this.#blockers.add(binding.blocker); + } + } + } + apply() { if (this.expressions.length === 0) { return b.empty; diff --git a/packages/svelte/src/compiler/phases/nodes.js b/packages/svelte/src/compiler/phases/nodes.js index 2c7c3fb66b..9d1a8f1890 100644 --- a/packages/svelte/src/compiler/phases/nodes.js +++ b/packages/svelte/src/compiler/phases/nodes.js @@ -115,6 +115,21 @@ export class ExpressionMetadata { is_async() { return this.has_await || this.#get_blockers().size > 0; } + + /** + * @param {ExpressionMetadata} source + */ + merge(source) { + this.has_state ||= source.has_state; + this.has_call ||= source.has_call; + this.has_await ||= source.has_await; + this.has_member_expression ||= source.has_member_expression; + this.has_assignment ||= source.has_assignment; + this.#blockers = null; // so that blockers are recalculated + + for (const r of source.references) this.references.add(r); + for (const b of source.dependencies) this.dependencies.add(b); + } } /** diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 9cf498d187..f9bbe28333 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -210,6 +210,7 @@ export namespace AST { binding?: Binding | null; binding_group_name: Identifier; parent_each_blocks: EachBlock[]; + expression: ExpressionMetadata; }; } diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js index c2443fda7a..2ad0538b43 100644 --- a/packages/svelte/src/internal/client/dom/template.js +++ b/packages/svelte/src/internal/client/dom/template.js @@ -20,7 +20,7 @@ import { TEMPLATE_USE_MATHML, TEMPLATE_USE_SVG } from '../../../constants.js'; -import { COMMENT_NODE, DOCUMENT_FRAGMENT_NODE, TEXT_NODE } from '#client/constants'; +import { COMMENT_NODE, DOCUMENT_FRAGMENT_NODE, EFFECT_RAN, TEXT_NODE } from '#client/constants'; /** * @param {TemplateNode} start @@ -344,7 +344,13 @@ export function comment() { */ export function append(anchor, dom) { if (hydrating) { - /** @type {Effect} */ (active_effect).nodes_end = hydrate_node; + var effect = /** @type {Effect} */ (active_effect); + // When hydrating and outer component and an inner component is async, i.e. blocked on a promise, + // then by the time the inner resolves we have already advanced to the end of the hydrated nodes + // of the parent component. Check for defined for that reason to avoid rewinding the parent's end marker. + if ((effect.f & EFFECT_RAN) === 0 || effect.nodes_end === null) { + effect.nodes_end = hydrate_node; + } hydrate_next(); return; } diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 9b5bc54740..3807c63c20 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -102,7 +102,8 @@ export { for_await_track_reactivity_loss, run, save, - track_reactivity_loss + track_reactivity_loss, + run_after_blockers } from './reactivity/async.js'; export { eager, flushSync as flush } from './reactivity/batch.js'; export { diff --git a/packages/svelte/src/internal/client/reactivity/async.js b/packages/svelte/src/internal/client/reactivity/async.js index 04e544759d..e48aff3d7f 100644 --- a/packages/svelte/src/internal/client/reactivity/async.js +++ b/packages/svelte/src/internal/client/reactivity/async.js @@ -84,6 +84,14 @@ export function flatten(blockers, sync, async, fn) { } } +/** + * @param {Array>} blockers + * @param {(values: Value[]) => any} fn + */ +export function run_after_blockers(blockers, fn) { + flatten(blockers, [], [], fn); +} + /** * Captures the current effect context so that we can restore it after * some asynchronous work has happened (so that e.g. `await a + b` diff --git a/packages/svelte/src/internal/client/validate.js b/packages/svelte/src/internal/client/validate.js index ec3d805447..0daecdb480 100644 --- a/packages/svelte/src/internal/client/validate.js +++ b/packages/svelte/src/internal/client/validate.js @@ -5,6 +5,7 @@ import { FILENAME } from '../../constants.js'; import { render_effect } from './reactivity/effects.js'; import * as w from './warnings.js'; import { capture_store_binding } from './reactivity/store.js'; +import { run_after_blockers } from './reactivity/async.js'; /** * @param {() => any} collection @@ -40,44 +41,47 @@ export function validate_each_keys(collection, key_fn) { /** * @param {string} binding + * @param {Array>} blockers * @param {() => Record} get_object * @param {() => string} get_property * @param {number} line * @param {number} column */ -export function validate_binding(binding, get_object, get_property, line, column) { - var warned = false; +export function validate_binding(binding, blockers, get_object, get_property, line, column) { + run_after_blockers(blockers, () => { + var warned = false; - var filename = dev_current_component_function?.[FILENAME]; + var filename = dev_current_component_function?.[FILENAME]; - render_effect(() => { - if (warned) return; + render_effect(() => { + if (warned) return; - var [object, is_store_sub] = capture_store_binding(get_object); + var [object, is_store_sub] = capture_store_binding(get_object); - if (is_store_sub) return; + if (is_store_sub) return; - var property = get_property(); + var property = get_property(); - var ran = false; + var ran = false; - // by making the (possibly false, but it would be an extreme edge case) assumption - // that a getter has a corresponding setter, we can determine if a property is - // reactive by seeing if this effect has dependencies - var effect = render_effect(() => { - if (ran) return; + // by making the (possibly false, but it would be an extreme edge case) assumption + // that a getter has a corresponding setter, we can determine if a property is + // reactive by seeing if this effect has dependencies + var effect = render_effect(() => { + if (ran) return; - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - object[property]; - }); + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + object[property]; + }); - ran = true; + ran = true; - if (effect.deps === null) { - var location = `${filename}:${line}:${column}`; - w.binding_property_non_reactive(binding, location); + if (effect.deps === null) { + var location = `${filename}:${line}:${column}`; + w.binding_property_non_reactive(binding, location); - warned = true; - } + warned = true; + } + }); }); } diff --git a/packages/svelte/tests/runtime-runes/samples/async-binding-after-await/Child.svelte b/packages/svelte/tests/runtime-runes/samples/async-binding-after-await/Child.svelte new file mode 100644 index 0000000000..6cb93933ec --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-binding-after-await/Child.svelte @@ -0,0 +1,5 @@ + + +{value} diff --git a/packages/svelte/tests/runtime-runes/samples/async-binding-after-await/_config.js b/packages/svelte/tests/runtime-runes/samples/async-binding-after-await/_config.js new file mode 100644 index 0000000000..3068cace2d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-binding-after-await/_config.js @@ -0,0 +1,14 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + mode: ['async-server', 'client', 'hydrate'], + ssrHtml: 'value value
false
', + + async test({ assert, target, logs }) { + await tick(); + + assert.htmlEqual(target.innerHTML, 'value value
true
'); + assert.deepEqual(logs, [false, 'value', true, 'value']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-binding-after-await/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-binding-after-await/main.svelte new file mode 100644 index 0000000000..9e4096581e --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-binding-after-await/main.svelte @@ -0,0 +1,17 @@ + + + + value, v => value = v} /> +
{!!ref}
+ + value, v => value = v} /> \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/async-static-prop-after-await/Child.svelte b/packages/svelte/tests/runtime-runes/samples/async-static-prop-after-await/Child.svelte new file mode 100644 index 0000000000..e93bbe3dc0 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-static-prop-after-await/Child.svelte @@ -0,0 +1,5 @@ + + +{value} diff --git a/packages/svelte/tests/runtime-runes/samples/async-static-prop-after-await/_config.js b/packages/svelte/tests/runtime-runes/samples/async-static-prop-after-await/_config.js new file mode 100644 index 0000000000..eff988e0d6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-static-prop-after-await/_config.js @@ -0,0 +1,11 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + mode: ['async-server', 'client', 'hydrate'], + ssrHtml: 'value
', + async test({ assert, target }) { + await tick(); + assert.htmlEqual(target.innerHTML, 'value
'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-static-prop-after-await/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-static-prop-after-await/main.svelte new file mode 100644 index 0000000000..1083882e1f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-static-prop-after-await/main.svelte @@ -0,0 +1,10 @@ + + + +
diff --git a/packages/svelte/tests/runtime-runes/samples/async-style-after-await/_config.js b/packages/svelte/tests/runtime-runes/samples/async-style-after-await/_config.js new file mode 100644 index 0000000000..472e7076c3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-style-after-await/_config.js @@ -0,0 +1,30 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + mode: ['async-server', 'client', 'hydrate'], + ssrHtml: ` +
+
+ +
+
+ + `, + + async test({ assert, target }) { + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` +
+
+ +
+
+ + ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-style-after-await/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-style-after-await/main.svelte new file mode 100644 index 0000000000..7c1abeda2c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-style-after-await/main.svelte @@ -0,0 +1,25 @@ + + + + + +{#if color} +
+
+ +{/if} + + +{#if color} +
+
+ +{/if}