Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/itchy-forks-greet.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: error at compile time instead of at runtime on await expressions inside bindings/transitions/animations/attachments
5 changes: 5 additions & 0 deletions .changeset/plain-bikes-smile.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: take async blockers into account for bindings/transitions/animations/attachments
6 changes: 6 additions & 0 deletions documentation/docs/98-reference/.generated/compile-errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```

### illegal_await_expression

```
`use:`, `transition:` and `animate:` directives, attachments and bindings do not support await expressions
```

### illegal_element_attribute

```
Expand Down
4 changes: 4 additions & 0 deletions packages/svelte/messages/compile-errors/template.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,10 @@ The same applies to components:

> Expected whitespace
## illegal_await_expression

> `use:`, `transition:` and `animate:` directives, attachments and bindings do not support await expressions
## illegal_element_attribute

> `<%name%>` does not support non-event attributes or spread attributes
Expand Down
9 changes: 9 additions & 0 deletions packages/svelte/src/compiler/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1148,6 +1148,15 @@ export function expected_whitespace(node) {
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
* @param {null | number | NodeLike} node
Expand Down
5 changes: 2 additions & 3 deletions packages/svelte/src/compiler/phases/1-parse/state/element.js
Original file line number Diff line number Diff line change
Expand Up @@ -654,8 +654,7 @@ function read_attribute(parser) {
}
}

/** @type {AST.Directive} */
const directive = {
const directive = /** @type {AST.Directive} */ ({
start,
end,
type,
Expand All @@ -664,7 +663,7 @@ function read_attribute(parser) {
metadata: {
expression: new ExpressionMetadata()
}
};
});

// @ts-expect-error we do this separately from the declaration to avoid upsetting typescript
directive.modifiers = modifiers;
Expand Down
2 changes: 2 additions & 0 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 { ArrowFunctionExpression } from './visitors/ArrowFunctionExpression.js';
import { AssignmentExpression } from './visitors/AssignmentExpression.js';
import { AnimateDirective } from './visitors/AnimateDirective.js';
import { AttachTag } from './visitors/AttachTag.js';
import { Attribute } from './visitors/Attribute.js';
import { AwaitBlock } from './visitors/AwaitBlock.js';
Expand Down Expand Up @@ -142,6 +143,7 @@ const visitors = {
pop_ignore();
}
},
AnimateDirective,
ArrowFunctionExpression,
AssignmentExpression,
AttachTag,
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/** @import { AST } from '#compiler' */
/** @import { Context } from '../types' */

import { mark_subtree_dynamic } from './shared/fragment.js';
import * as e from '../../../errors.js';

/**
* @param {AST.AttachTag} node
Expand All @@ -10,4 +10,8 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
export function AttachTag(node, context) {
mark_subtree_dynamic(context.path);
context.next({ ...context.state, expression: node.metadata.expression });

if (node.metadata.expression.has_await) {
e.illegal_await_expression(node);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ export function BindDirective(node, context) {

const [get, set] = node.expression.expressions;
// 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.state,
expression: node.metadata.expression
Expand All @@ -169,6 +170,11 @@ export function BindDirective(node, context) {
...context.state,
expression: node.metadata.expression
});

if (node.metadata.expression.has_await) {
e.illegal_await_expression(node);
}

return;
}

Expand Down Expand Up @@ -267,4 +273,8 @@ export function BindDirective(node, context) {
}

context.next({ ...context.state, expression: node.metadata.expression });

if (node.metadata.expression.has_await) {
e.illegal_await_expression(node);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/** @import { AST } from '#compiler' */
/** @import { Context } from '../types' */
import * as e from '../../../errors.js';

import { mark_subtree_dynamic } from './shared/fragment.js';

Expand All @@ -10,5 +11,9 @@ import { mark_subtree_dynamic } from './shared/fragment.js';
export function TransitionDirective(node, context) {
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);
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
/** @import { AST } from '#compiler' */
/** @import { Context } from '../types' */
import { mark_subtree_dynamic } from './shared/fragment.js';
import * as e from '../../../errors.js';

/**
* @param {AST.UseDirective} node
* @param {Context} context
*/
export function UseDirective(node, context) {
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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,24 @@ export function AnimateDirective(node, context) {
: b.thunk(/** @type {Expression} */ (context.visit(node.expression)));

// in after_update to ensure it always happens after bind:this
context.state.after_update.push(
b.stmt(
b.call(
'$.animation',
context.state.node,
b.thunk(/** @type {Expression} */ (context.visit(parse_directive_name(node.name)))),
expression
)
let statement = b.stmt(
b.call(
'$.animation',
context.state.node,
b.thunk(/** @type {Expression} */ (context.visit(parse_directive_name(node.name)))),
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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@ import { build_expression } from './shared/utils.js';
*/
export function AttachTag(node, context) {
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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,17 @@ export function TransitionDirective(node, context) {
}

// 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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ export function UseDirective(node, context) {
}

// 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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ export function build_inline_component(node, expression, context) {
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);
}
}

Expand Down
12 changes: 12 additions & 0 deletions packages/svelte/src/compiler/types/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ export namespace AST {
name: string;
/** The y in `animate:x={y}` */
expression: null | Expression;
/** @internal */
metadata: {
expression: ExpressionMetadata;
};
}

/** A `bind:` directive */
Expand Down Expand Up @@ -285,6 +289,10 @@ export namespace AST {
intro: boolean;
/** True if this is a `transition:` or `out:` directive */
outro: boolean;
/** @internal */
metadata: {
expression: ExpressionMetadata;
};
}

/** A `use:` directive */
Expand All @@ -294,6 +302,10 @@ export namespace AST {
name: string;
/** The 'y' in `use:x={y}` */
expression: null | Expression;
/** @internal */
metadata: {
expression: ExpressionMetadata;
};
}

interface BaseElement extends BaseNode {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
/** @import { AnimateFn, Animation, AnimationConfig, EachItem, Effect, TransitionFn, TransitionManager } from '#client' */
import { noop, is_function } from '../../../shared/utils.js';
import { effect } from '../../reactivity/effects.js';
import {
active_effect,
active_reaction,
set_active_effect,
set_active_reaction,
untrack
} from '../../runtime.js';
import { active_effect, untrack } from '../../runtime.js';
import { loop } from '../../loop.js';
import { should_intro } from '../../render.js';
import { current_each_item } from '../blocks/each.js';
Expand Down
7 changes: 6 additions & 1 deletion packages/svelte/src/internal/client/reactivity/async.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
} from './deriveds.js';
import { aborted } from './effects.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
Expand Down Expand Up @@ -89,7 +90,11 @@ export function flatten(blockers, sync, async, fn) {
* @param {(values: Value[]) => any} 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);
});
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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']);
}
});
Original file line number Diff line number Diff line change
@@ -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>
Loading
Loading