fix: take async into account for bindings/transitions/animations/attachments (#17198)

* fix: take async into account for bindings/transitions/animations/attachments

- block on async work
- error at compile time on await expressions. Right now it gives confusing errors later at compile time or at runtime

Fixes #17194

* this was weird
pull/17204/head
Simon H 18 hours ago committed by GitHub
parent a17dc3c302
commit 91486fa807
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: error at compile time instead of at runtime on await expressions inside bindings/transitions/animations/attachments

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: take async blockers into account for bindings/transitions/animations/attachments

@ -561,6 +561,12 @@ Cannot use `await` in deriveds and template expressions, or at the top level of
`$host()` can only be used inside custom element component instances `$host()` can only be used inside custom element component instances
``` ```
### illegal_await_expression
```
`use:`, `transition:` and `animate:` directives, attachments and bindings do not support await expressions
```
### illegal_element_attribute ### illegal_element_attribute
``` ```

@ -231,6 +231,10 @@ The same applies to components:
> Expected whitespace > Expected whitespace
## illegal_await_expression
> `use:`, `transition:` and `animate:` directives, attachments and bindings do not support await expressions
## illegal_element_attribute ## illegal_element_attribute
> `<%name%>` does not support non-event attributes or spread attributes > `<%name%>` does not support non-event attributes or spread attributes

@ -1148,6 +1148,15 @@ export function expected_whitespace(node) {
e(node, 'expected_whitespace', `Expected whitespace\nhttps://svelte.dev/e/expected_whitespace`); e(node, 'expected_whitespace', `Expected whitespace\nhttps://svelte.dev/e/expected_whitespace`);
} }
/**
* `use:`, `transition:` and `animate:` directives, attachments and bindings do not support await expressions
* @param {null | number | NodeLike} node
* @returns {never}
*/
export function illegal_await_expression(node) {
e(node, 'illegal_await_expression', `\`use:\`, \`transition:\` and \`animate:\` directives, attachments and bindings do not support await expressions\nhttps://svelte.dev/e/illegal_await_expression`);
}
/** /**
* `<%name%>` does not support non-event attributes or spread attributes * `<%name%>` does not support non-event attributes or spread attributes
* @param {null | number | NodeLike} node * @param {null | number | NodeLike} node

@ -654,8 +654,7 @@ function read_attribute(parser) {
} }
} }
/** @type {AST.Directive} */ const directive = /** @type {AST.Directive} */ ({
const directive = {
start, start,
end, end,
type, type,
@ -664,7 +663,7 @@ function read_attribute(parser) {
metadata: { metadata: {
expression: new ExpressionMetadata() expression: new ExpressionMetadata()
} }
}; });
// @ts-expect-error we do this separately from the declaration to avoid upsetting typescript // @ts-expect-error we do this separately from the declaration to avoid upsetting typescript
directive.modifiers = modifiers; directive.modifiers = modifiers;

@ -24,6 +24,7 @@ import { extract_svelte_ignore } from '../../utils/extract_svelte_ignore.js';
import { ignore_map, ignore_stack, pop_ignore, push_ignore } from '../../state.js'; import { ignore_map, ignore_stack, pop_ignore, push_ignore } from '../../state.js';
import { ArrowFunctionExpression } from './visitors/ArrowFunctionExpression.js'; import { ArrowFunctionExpression } from './visitors/ArrowFunctionExpression.js';
import { AssignmentExpression } from './visitors/AssignmentExpression.js'; import { AssignmentExpression } from './visitors/AssignmentExpression.js';
import { AnimateDirective } from './visitors/AnimateDirective.js';
import { AttachTag } from './visitors/AttachTag.js'; import { AttachTag } from './visitors/AttachTag.js';
import { Attribute } from './visitors/Attribute.js'; import { Attribute } from './visitors/Attribute.js';
import { AwaitBlock } from './visitors/AwaitBlock.js'; import { AwaitBlock } from './visitors/AwaitBlock.js';
@ -142,6 +143,7 @@ const visitors = {
pop_ignore(); pop_ignore();
} }
}, },
AnimateDirective,
ArrowFunctionExpression, ArrowFunctionExpression,
AssignmentExpression, AssignmentExpression,
AttachTag, AttachTag,

@ -0,0 +1,15 @@
/** @import { Context } from '../types' */
/** @import { AST } from '#compiler'; */
import * as e from '../../../errors.js';
/**
* @param {AST.AnimateDirective} node
* @param {Context} context
*/
export function AnimateDirective(node, context) {
context.next({ ...context.state, expression: node.metadata.expression });
if (node.metadata.expression.has_await) {
e.illegal_await_expression(node);
}
}

@ -1,7 +1,7 @@
/** @import { AST } from '#compiler' */ /** @import { AST } from '#compiler' */
/** @import { Context } from '../types' */ /** @import { Context } from '../types' */
import { mark_subtree_dynamic } from './shared/fragment.js'; import { mark_subtree_dynamic } from './shared/fragment.js';
import * as e from '../../../errors.js';
/** /**
* @param {AST.AttachTag} node * @param {AST.AttachTag} node
@ -10,4 +10,8 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
export function AttachTag(node, context) { export function AttachTag(node, context) {
mark_subtree_dynamic(context.path); mark_subtree_dynamic(context.path);
context.next({ ...context.state, expression: node.metadata.expression }); context.next({ ...context.state, expression: node.metadata.expression });
if (node.metadata.expression.has_await) {
e.illegal_await_expression(node);
}
} }

@ -161,6 +161,7 @@ export function BindDirective(node, context) {
const [get, set] = node.expression.expressions; const [get, set] = node.expression.expressions;
// We gotta jump across the getter/setter functions to avoid the expression metadata field being reset to null // We gotta jump across the getter/setter functions to avoid the expression metadata field being reset to null
// as we want to collect the functions' blocker/async info
context.visit(get.type === 'ArrowFunctionExpression' ? get.body : get, { context.visit(get.type === 'ArrowFunctionExpression' ? get.body : get, {
...context.state, ...context.state,
expression: node.metadata.expression expression: node.metadata.expression
@ -169,6 +170,11 @@ export function BindDirective(node, context) {
...context.state, ...context.state,
expression: node.metadata.expression expression: node.metadata.expression
}); });
if (node.metadata.expression.has_await) {
e.illegal_await_expression(node);
}
return; return;
} }
@ -267,4 +273,8 @@ export function BindDirective(node, context) {
} }
context.next({ ...context.state, expression: node.metadata.expression }); context.next({ ...context.state, expression: node.metadata.expression });
if (node.metadata.expression.has_await) {
e.illegal_await_expression(node);
}
} }

@ -1,5 +1,6 @@
/** @import { AST } from '#compiler' */ /** @import { AST } from '#compiler' */
/** @import { Context } from '../types' */ /** @import { Context } from '../types' */
import * as e from '../../../errors.js';
import { mark_subtree_dynamic } from './shared/fragment.js'; import { mark_subtree_dynamic } from './shared/fragment.js';
@ -10,5 +11,9 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
export function TransitionDirective(node, context) { export function TransitionDirective(node, context) {
mark_subtree_dynamic(context.path); mark_subtree_dynamic(context.path);
context.next(); context.next({ ...context.state, expression: node.metadata.expression });
if (node.metadata.expression.has_await) {
e.illegal_await_expression(node);
}
} }

@ -1,6 +1,7 @@
/** @import { AST } from '#compiler' */ /** @import { AST } from '#compiler' */
/** @import { Context } from '../types' */ /** @import { Context } from '../types' */
import { mark_subtree_dynamic } from './shared/fragment.js'; import { mark_subtree_dynamic } from './shared/fragment.js';
import * as e from '../../../errors.js';
/** /**
* @param {AST.UseDirective} node * @param {AST.UseDirective} node
@ -8,5 +9,10 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
*/ */
export function UseDirective(node, context) { export function UseDirective(node, context) {
mark_subtree_dynamic(context.path); mark_subtree_dynamic(context.path);
context.next();
context.next({ ...context.state, expression: node.metadata.expression });
if (node.metadata.expression.has_await) {
e.illegal_await_expression(node);
}
} }

@ -15,14 +15,24 @@ export function AnimateDirective(node, context) {
: b.thunk(/** @type {Expression} */ (context.visit(node.expression))); : b.thunk(/** @type {Expression} */ (context.visit(node.expression)));
// in after_update to ensure it always happens after bind:this // in after_update to ensure it always happens after bind:this
context.state.after_update.push( let statement = b.stmt(
b.stmt( b.call(
b.call( '$.animation',
'$.animation', context.state.node,
context.state.node, b.thunk(/** @type {Expression} */ (context.visit(parse_directive_name(node.name)))),
b.thunk(/** @type {Expression} */ (context.visit(parse_directive_name(node.name)))), expression
expression
)
) )
); );
if (node.metadata.expression.is_async()) {
statement = b.stmt(
b.call(
'$.run_after_blockers',
node.metadata.expression.blockers(),
b.thunk(b.block([statement]))
)
);
}
context.state.after_update.push(statement);
} }

@ -9,6 +9,18 @@ import { build_expression } from './shared/utils.js';
*/ */
export function AttachTag(node, context) { export function AttachTag(node, context) {
const expression = build_expression(context, node.expression, node.metadata.expression); const expression = build_expression(context, node.expression, node.metadata.expression);
context.state.init.push(b.stmt(b.call('$.attach', context.state.node, b.thunk(expression)))); let statement = b.stmt(b.call('$.attach', context.state.node, b.thunk(expression)));
if (node.metadata.expression.is_async()) {
statement = b.stmt(
b.call(
'$.run_after_blockers',
node.metadata.expression.blockers(),
b.thunk(b.block([statement]))
)
);
}
context.state.init.push(statement);
context.next(); context.next();
} }

@ -25,5 +25,17 @@ export function TransitionDirective(node, context) {
} }
// in after_update to ensure it always happens after bind:this // in after_update to ensure it always happens after bind:this
context.state.after_update.push(b.stmt(b.call('$.transition', ...args))); let statement = b.stmt(b.call('$.transition', ...args));
if (node.metadata.expression.is_async()) {
statement = b.stmt(
b.call(
'$.run_after_blockers',
node.metadata.expression.blockers(),
b.thunk(b.block([statement]))
)
);
}
context.state.after_update.push(statement);
} }

@ -32,6 +32,18 @@ export function UseDirective(node, context) {
} }
// actions need to run after attribute updates in order with bindings/events // actions need to run after attribute updates in order with bindings/events
context.state.init.push(b.stmt(b.call('$.action', ...args))); let statement = b.stmt(b.call('$.action', ...args));
if (node.metadata.expression.is_async()) {
statement = b.stmt(
b.call(
'$.run_after_blockers',
node.metadata.expression.blockers(),
b.thunk(b.block([statement]))
)
);
}
context.state.init.push(statement);
context.next(); context.next();
} }

@ -296,6 +296,9 @@ export function build_component(node, component_name, context) {
); );
} }
// TODO also support await expressions here?
memoizer.check_blockers(attribute.metadata.expression);
push_prop(b.prop('init', b.call('$.attachment'), expression, true)); push_prop(b.prop('init', b.call('$.attachment'), expression, true));
} }
} }

@ -142,6 +142,10 @@ export function build_inline_component(node, expression, context) {
true true
); );
} }
} else if (attribute.type === 'AttachTag') {
// While we don't run attachments on the server, on the client they might generate a surrounding blocker function which generates
// extra comments, and to prevent hydration mismatches we therefore have to account for them here to generate similar comments on the server.
optimiser.check_blockers(attribute.metadata.expression);
} }
} }

@ -194,6 +194,10 @@ export namespace AST {
name: string; name: string;
/** The y in `animate:x={y}` */ /** The y in `animate:x={y}` */
expression: null | Expression; expression: null | Expression;
/** @internal */
metadata: {
expression: ExpressionMetadata;
};
} }
/** A `bind:` directive */ /** A `bind:` directive */
@ -285,6 +289,10 @@ export namespace AST {
intro: boolean; intro: boolean;
/** True if this is a `transition:` or `out:` directive */ /** True if this is a `transition:` or `out:` directive */
outro: boolean; outro: boolean;
/** @internal */
metadata: {
expression: ExpressionMetadata;
};
} }
/** A `use:` directive */ /** A `use:` directive */
@ -294,6 +302,10 @@ export namespace AST {
name: string; name: string;
/** The 'y' in `use:x={y}` */ /** The 'y' in `use:x={y}` */
expression: null | Expression; expression: null | Expression;
/** @internal */
metadata: {
expression: ExpressionMetadata;
};
} }
interface BaseElement extends BaseNode { interface BaseElement extends BaseNode {

@ -1,13 +1,7 @@
/** @import { AnimateFn, Animation, AnimationConfig, EachItem, Effect, TransitionFn, TransitionManager } from '#client' */ /** @import { AnimateFn, Animation, AnimationConfig, EachItem, Effect, TransitionFn, TransitionManager } from '#client' */
import { noop, is_function } from '../../../shared/utils.js'; import { noop, is_function } from '../../../shared/utils.js';
import { effect } from '../../reactivity/effects.js'; import { effect } from '../../reactivity/effects.js';
import { import { active_effect, untrack } from '../../runtime.js';
active_effect,
active_reaction,
set_active_effect,
set_active_reaction,
untrack
} from '../../runtime.js';
import { loop } from '../../loop.js'; import { loop } from '../../loop.js';
import { should_intro } from '../../render.js'; import { should_intro } from '../../render.js';
import { current_each_item } from '../blocks/each.js'; import { current_each_item } from '../blocks/each.js';

@ -26,6 +26,7 @@ import {
} from './deriveds.js'; } from './deriveds.js';
import { aborted } from './effects.js'; import { aborted } from './effects.js';
import { hydrate_next, hydrating, set_hydrate_node, skip_nodes } from '../dom/hydration.js'; import { hydrate_next, hydrating, set_hydrate_node, skip_nodes } from '../dom/hydration.js';
import { current_each_item, set_current_each_item } from '../dom/blocks/each.js';
/** /**
* @param {Array<Promise<void>>} blockers * @param {Array<Promise<void>>} blockers
@ -89,7 +90,11 @@ export function flatten(blockers, sync, async, fn) {
* @param {(values: Value[]) => any} fn * @param {(values: Value[]) => any} fn
*/ */
export function run_after_blockers(blockers, fn) { export function run_after_blockers(blockers, fn) {
flatten(blockers, [], [], fn); var each_item = current_each_item; // TODO should this be part of capture?
flatten(blockers, [], [], (v) => {
set_current_each_item(each_item);
fn(v);
});
} }
/** /**

@ -0,0 +1,11 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
mode: ['client', 'hydrate'],
async test({ assert, logs }) {
await tick();
assert.deepEqual(logs, ['ready']);
}
});

@ -0,0 +1,13 @@
<script>
// Wait a macrotask to make sure the effect doesn't run before the microtask-Promise.resolve() resolves, masking a bug
await new Promise(r => setTimeout(r));
function run(_, arg) {
console.log(arg);
}
let value = $state('ready');
</script>
<div use:run={value}></div>

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

@ -0,0 +1,11 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
mode: ['client', 'hydrate'],
async test({ assert, logs }) {
await tick();
assert.deepEqual(logs, ['ready', 'ready']);
}
});

@ -0,0 +1,17 @@
<script>
import Child from "./Child.svelte";
// Wait a macrotask to make sure the effect doesn't run before the microtask-Promise.resolve() resolves, masking a bug
await new Promise(r => setTimeout(r));
function createAttachment(value) {
return () => {
console.log(value);
};
}
let attachment = $state('ready');
</script>
<div {@attach createAttachment(attachment)}></div>
<Child {@attach createAttachment(attachment)} />

@ -0,0 +1,11 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
mode: ['client', 'hydrate'],
async test({ assert, logs }) {
await tick();
assert.deepEqual(logs, ['ready']);
}
});

@ -0,0 +1,13 @@
<script>
// Wait a macrotask to make sure the effect doesn't run before the microtask-Promise.resolve() resolves, masking a bug
await new Promise(r => setTimeout(r));
function custom(_, value) {
console.log(value);
return { duration: 0 };
}
let params = $state('ready');
</script>
<div transition:custom={params}></div>
Loading…
Cancel
Save