fix: wait on dependencies of async bindings (#17120)

* fix: wait on dependencies of async bindings

Correctly takes into account blockers for bindings. Also ensures it works for functional bindings.

During that I also noticed a bug in our `nodes_end` assignment logic, which the added test also regresses against.

Fixes #17092
Fixes #17090

* ensure async static props/attributes are awaited

* wait on blockers in binding validation

* await dependencies of style directives

* tidy
pull/17127/head
Simon H 2 weeks ago committed by GitHub
parent 3a4dc7d83d
commit 68d4f9c3b6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: ensure async static props/attributes are awaited

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: wait on dependencies of async bindings

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: await dependencies of style directives

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

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

@ -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]))
)
);
}

@ -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

@ -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 &&

@ -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) {

@ -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
),

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

@ -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') {

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

@ -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);
}
}
/**

@ -210,6 +210,7 @@ export namespace AST {
binding?: Binding | null;
binding_group_name: Identifier;
parent_each_blocks: EachBlock[];
expression: ExpressionMetadata;
};
}

@ -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;
}

@ -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 {

@ -84,6 +84,14 @@ export function flatten(blockers, sync, async, fn) {
}
}
/**
* @param {Array<Promise<void>>} 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`

@ -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<Promise<void>>} blockers
* @param {() => Record<string, any>} 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;
}
});
});
}

@ -0,0 +1,5 @@
<script>
let { value = $bindable() } = $props();
</script>
{value}

@ -0,0 +1,14 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
mode: ['async-server', 'client', 'hydrate'],
ssrHtml: 'value value <div>false</div> <input value="value"> <input value="value">',
async test({ assert, target, logs }) {
await tick();
assert.htmlEqual(target.innerHTML, 'value value <div>true</div> <input> <input>');
assert.deepEqual(logs, [false, 'value', true, 'value']);
}
});

@ -0,0 +1,17 @@
<script>
import Child from './Child.svelte';
await Promise.resolve();
let ref = $state(null);
$effect(() => console.log(!!ref, value))
let value = $state('value');
</script>
<Child bind:value />
<Child bind:value={() => value, v => value = v} />
<div bind:this={ref}>{!!ref}</div>
<input bind:value />
<input bind:value={() => value, v => value = v} />

@ -0,0 +1,5 @@
<script>
let { value } = $props();
</script>
{value}

@ -0,0 +1,11 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
mode: ['async-server', 'client', 'hydrate'],
ssrHtml: 'value <div class="value"></div>',
async test({ assert, target }) {
await tick();
assert.htmlEqual(target.innerHTML, 'value <div class="value"></div>');
}
});

@ -0,0 +1,10 @@
<script>
import Child from './Child.svelte';
await Promise.resolve();
let value = 'value';
</script>
<Child {value} />
<div class={value}></div>

@ -0,0 +1,30 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
mode: ['async-server', 'client', 'hydrate'],
ssrHtml: `
<div style="color: red;"></div>
<div style="width: 100px;"></div>
<button>width</button>
<div style="color: red;"></div>
<div style="width: 100px;"></div>
<button>width</button>
`,
async test({ assert, target }) {
await tick();
assert.htmlEqual(
target.innerHTML,
`
<div style="color: red;"></div>
<div style="width: 100px;"></div>
<button>width</button>
<div style="color: red;"></div>
<div style="width: 100px;"></div>
<button>width</button>
`
);
}
});

@ -0,0 +1,25 @@
<script>
await Promise.resolve();
// static
let color = $state('red');
// dynamic
let width = $state('100px');
</script>
<!-- force them into their own template effects -->
<!-- normal -->
{#if color}
<div style:color={color}></div>
<div style:width={width}></div>
<button onclick={() => width = '1px'}>width</button>
{/if}
<!-- shorthand -->
{#if color}
<div style:color></div>
<div style:width></div>
<button onclick={() => width = '1px'}>width</button>
{/if}
Loading…
Cancel
Save