Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable animate: directive for snippets #14796

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 7 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/sixty-grapes-dream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': minor
---

feat: enable `animate:` directive for snippets
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ An element can only have one 'animate' directive
### animation_invalid_placement

```
An element that uses the `animate:` directive must be the only child of a keyed `{#each ...}` block
An element that uses the `animate:` directive must be the only child of a keyed `{#each ...}` block, or an only child of a snippet
```

### animation_missing_key
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/messages/compile-errors/template.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

## animation_invalid_placement

> An element that uses the `animate:` directive must be the only child of a keyed `{#each ...}` block
> An element that uses the `animate:` directive must be the only child of a keyed `{#each ...}` block, or an only child of a snippet

## animation_missing_key

Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/compiler/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -625,12 +625,12 @@ export function animation_duplicate(node) {
}

/**
* An element that uses the `animate:` directive must be the only child of a keyed `{#each ...}` block
* An element that uses the `animate:` directive must be the only child of a keyed `{#each ...}` block, or an only child of a snippet
* @param {null | number | NodeLike} node
* @returns {never}
*/
export function animation_invalid_placement(node) {
e(node, "animation_invalid_placement", `An element that uses the \`animate:\` directive must be the only child of a keyed \`{#each ...}\` block\nhttps://svelte.dev/e/animation_invalid_placement`);
e(node, "animation_invalid_placement", `An element that uses the \`animate:\` directive must be the only child of a keyed \`{#each ...}\` block, or an only child of a snippet\nhttps://svelte.dev/e/animation_invalid_placement`);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,13 @@ export function RenderTag(node, context) {
*/
let resolved = callee.type === 'Identifier' && is_resolved_snippet(binding);

/** @type {AST.SnippetBlock | undefined} */
let snippet

if (binding?.initial?.type === 'SnippetBlock') {
// if this render tag unambiguously references a local snippet, our job is easy
node.metadata.snippets.add(binding.initial);
snippet = binding.initial
node.metadata.snippets.add(snippet);
}

context.state.analysis.snippet_renderers.set(node, resolved);
Expand All @@ -49,8 +53,31 @@ export function RenderTag(node, context) {
) {
e.render_tag_invalid_call_expression(node);
}

const parent = context.path.at(-2);
const is_animated = snippet?.body.nodes.some(is_animate_directive);

if(is_animated) {
if (parent?.type !== 'EachBlock') {
e.animation_invalid_placement(node);
}
else if (!parent.key) {
e.animation_missing_key(parent);
}
}

mark_subtree_dynamic(context.path);

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

/** @param {AST.Text | AST.Tag | AST.ElementLike | AST.Comment | AST.Block} child */
function is_animate_directive(child) {
Copy link
Author

@Refzlund Refzlund Dec 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is used twice as it is also in phases/3-transform/client/visitors/EachBlock.js#L347. Do I put it anywhere to re-use it across the two phases?

if (child.type === 'RenderTag') {
for (const snippet_block of child.metadata.snippets) {
return snippet_block.body.nodes.some(is_animate_directive);
}
}
if (child.type !== 'RegularElement' && child.type !== 'SvelteElement') return false;
return child.attributes.some((attr) => attr.type === 'AnimateDirective');
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ export function validate_element(node, context) {
validate_attribute_name(attribute);
} else if (attribute.type === 'AnimateDirective') {
const parent = context.path.at(-2);
if (parent?.type !== 'EachBlock') {
if (parent?.type !== 'EachBlock' && parent?.type !== 'SnippetBlock') {
e.animation_invalid_placement(attribute);
} else if (!parent.key) {
} else if (parent.type === 'EachBlock' && !parent.key) {
e.animation_missing_key(attribute);
} else if (
parent.body.nodes.filter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,7 @@ export function EachBlock(node, context) {
// Since `animate:` can only appear on elements that are the sole child of a keyed each block,
// we can determine at compile time whether the each block is animated or not (in which
// case it should measure animated elements before and after reconciliation).
if (
node.key &&
node.body.nodes.some((child) => {
if (child.type !== 'RegularElement' && child.type !== 'SvelteElement') return false;
return child.attributes.some((attr) => attr.type === 'AnimateDirective');
})
) {
if (node.key && node.body.nodes.some(is_animate_directive)) {
flags |= EACH_IS_ANIMATED;
}

Expand Down Expand Up @@ -348,3 +342,14 @@ function collect_transitive_dependencies(binding, seen = new Set()) {

return [...seen];
}

/** @param {AST.Text | AST.Tag | AST.ElementLike | AST.Comment | AST.Block} child */
function is_animate_directive(child) {
if (child.type === 'RenderTag') {
for (const snippet_block of child.metadata.snippets) {
return snippet_block.body.nodes.some(is_animate_directive);
}
}
if (child.type !== 'RegularElement' && child.type !== 'SvelteElement') return false;
return child.attributes.some((attr) => attr.type === 'AnimateDirective');
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[
{
"code": "animation_invalid_placement",
"message": "An element that uses the `animate:` directive must be the only child of a keyed `{#each ...}` block",
"message": "An element that uses the `animate:` directive must be the only child of a keyed `{#each ...}` block, or an only child of a snippet",
"start": {
"line": 5,
"column": 5
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[
{
"code": "animation_invalid_placement",
"message": "An element that uses the `animate:` directive must be the only child of a keyed `{#each ...}` block",
"message": "An element that uses the `animate:` directive must be the only child of a keyed `{#each ...}` block, or an only child of a snippet",
"start": {
"line": 6,
"column": 6
Expand Down
Loading