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

fix: improve consttag ordering in non-runes mode #11908

Merged
merged 23 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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/cold-lamps-accept.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: improve consttag ordering in non-runes mode
14 changes: 14 additions & 0 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,20 @@ const legacy_scope_tweaker = {
binding = state.scope.get(node.name.slice(1));
}

if (
binding?.kind === 'derived' &&
binding.declaration_kind === 'const' &&
binding.node !== node
) {
const const_tag = /** @type {import('../../types/template.js').ConstTag} */ (
path.findLast((n) => n.type === 'ConstTag')
);
if (const_tag !== undefined) {
const_tag.metadata ||= { dependencies: new Set() };
const_tag.metadata.dependencies.add(node.name);
}
}

if (
binding !== null &&
binding.kind === 'normal' &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,7 @@ function create_block(parent, fragment, name, nodes, context) {
nodes,
context.path,
namespace,
context.state.options.runes,
context.state.preserve_whitespace,
context.state.options.preserveComments
);
Expand Down Expand Up @@ -2116,6 +2117,7 @@ export const template_visitors = {
node.fragment.nodes,
context.path,
child_metadata.namespace,
context.state.options.runes,
node.name === 'script' || state.preserve_whitespace,
state.options.preserveComments
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ function create_block(parent, fragment, nodes, context, anchor) {
nodes,
context.path,
namespace,
context.state.options.runes,
context.state.preserve_whitespace,
context.state.options.preserveComments
);
Expand Down Expand Up @@ -1382,6 +1383,7 @@ const template_visitors = {
node.fragment.nodes,
inner_context.path,
metadata.namespace,
context.state.options.runes,
state.preserve_whitespace,
state.options.preserveComments
);
Expand Down
62 changes: 62 additions & 0 deletions packages/svelte/src/compiler/phases/3-transform/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
} from '../patterns.js';
import * as b from '../../utils/builders.js';
import { walk } from 'zimmerframe';
import { extract_identifiers } from '../../utils/ast.js';

/**
* @param {import('estree').Node} node
Expand All @@ -21,6 +22,62 @@ export function is_hoistable_function(node) {
return false;
}

/**
* @param {import("#compiler").SvelteNode[]} nodes
*/
function sort_const_tags(nodes) {
// If we are in a legacy component, attempt to re-order any ConstTag template nodes
// so that they're in topological order given their dependencies.
const const_tags = [];
const deps = new Set();
const tag_ids = new Map();
const other = [];
for (const node of nodes) {
if (node.type === 'ConstTag') {
const ids = extract_identifiers(node.declaration.declarations[0].id);
for (const id of ids) {
deps.add(id.name);
}
tag_ids.set(node, ids);
const_tags.push(node);
} else {
other.push(node);
}
}

if (const_tags.length > 1) {
let attempts = 0;
const new_nodes = [];
while (const_tags.length > 0) {
// Worst case-scenario we prevent an infinite loop
if (attempts > 100) {
new_nodes.push(...const_tags);
break;
}
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
attempts++;
const_loop: for (let i = const_tags.length - 1; i >= 0; i--) {
const node = const_tags[i];
const dependencies = node.metadata?.dependencies;
if (dependencies) {
for (const dep of dependencies) {
if (deps.has(dep)) {
continue const_loop;
}
}
}
const ids = tag_ids.get(node);
for (const id of ids) {
deps.delete(id.name);
}
const_tags.splice(i, 1);
new_nodes.push(node);
}
}
return [...new_nodes, ...other];
}
return nodes;
}

/**
* Extract nodes that are hoisted and trim whitespace according to the following rules:
* - trim leading and trailing whitespace, regardless of surroundings
Expand All @@ -32,6 +89,7 @@ export function is_hoistable_function(node) {
* @param {import('#compiler').SvelteNode[]} nodes
* @param {import('#compiler').SvelteNode[]} path
* @param {import('#compiler').Namespace} namespace
* @param {boolean} runes
* @param {boolean} preserve_whitespace
* @param {boolean} preserve_comments
*/
Expand All @@ -40,9 +98,13 @@ export function clean_nodes(
nodes,
path,
namespace = 'html',
runes = false,
preserve_whitespace,
preserve_comments
) {
if (!runes) {
nodes = sort_const_tags(nodes);
}
/** @type {import('#compiler').SvelteNode[]} */
const hoisted = [];

Expand Down
3 changes: 3 additions & 0 deletions packages/svelte/src/compiler/types/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@ export interface ConstTag extends BaseNode {
declaration: VariableDeclaration & {
declarations: [VariableDeclarator & { id: Pattern; init: Expression }];
};
metadata?: {
dependencies: Set<string>;
};
}

/** A `{@debug ...}` tag */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { test } from '../../test';

export default test({
html: '<h1>Hello worldworld!</h1>'
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<svelte:options runes={false} />
{#if true}
{@const foo = bar}
{@const yoo = foo}
{@const bar = 'world'}
<h1>Hello {bar}{yoo}!</h1>
{/if}
3 changes: 3 additions & 0 deletions packages/svelte/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1448,6 +1448,9 @@ declare module 'svelte/compiler' {
declaration: VariableDeclaration & {
declarations: [VariableDeclarator & { id: Pattern; init: Expression }];
};
metadata?: {
dependencies: Set<string>;
};
}

/** A `{@debug ...}` tag */
Expand Down
Loading