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: support type annotations in {@const ...} tag #9609

Merged
merged 9 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from 8 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/seven-ravens-check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': minor
---

feat: support type definition in {@const}
6 changes: 4 additions & 2 deletions documentation/docs/05-misc/03-typescript.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,11 @@ You cannot use TypeScript in your template's markup. For example, the following
let count = 10;
</script>

<h1>Count as string: {count as string}!</h1> <!-- ❌ Does not work -->
<!-- ❌ Does not work -->
<h1>Count as string: {count as string}!</h1>
{#if count > 4}
{@const countString: string = count} <!-- ❌ Does not work -->
<!-- ❌ Does not work -->
{@const countString: string = count}
Copy link
Member

Choose a reason for hiding this comment

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

Can we revert the doc changes? We're going to rewrite most of these anyway, so no point in doing this now and risk merge conflicts in case we want to port some V4 changes to the main branch.

Copy link
Member Author

Choose a reason for hiding this comment

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

I also want to revert this but I can not pass the lint ci.
Is it better to add this file to .prettierignore?

Copy link
Member Author

Choose a reason for hiding this comment

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

Once I tried to update it.
cba043c

Copy link
Member

Choose a reason for hiding this comment

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

I'll look into on Monday more closely. If it's only related to prettier then we might need to add it. Although I don't understand how this wasn't failing before then

Copy link
Member Author

@baseballyama baseballyama Nov 25, 2023

Choose a reason for hiding this comment

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

I think because Svelte compiler without this PR can not parse the code due to type in {@const} tag.

{countString}
{/if}
```
Expand Down
27 changes: 27 additions & 0 deletions packages/svelte/src/compiler/legacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,33 @@ export function convert(source, ast) {
};
},
// @ts-ignore
ConstTag(node) {
if (
/** @type {import('./types/legacy-nodes.js').LegacyConstTag} */ (node).expression !==
undefined
) {
return node;
}

const modern_node = /** @type {import('#compiler').ConstTag} */ (node);
const { id: left } = { ...modern_node.declaration.declarations[0] };
// @ts-ignore
delete left.typeAnnotation;
return {
type: 'ConstTag',
start: modern_node.start,
end: node.end,
expression: {
type: 'AssignmentExpression',
start: (modern_node.declaration.start ?? 0) + 'const '.length,
end: modern_node.declaration.end ?? 0,
operator: '=',
left,
right: modern_node.declaration.declarations[0].init
}
};
},
// @ts-ignore
KeyBlock(node, { visit }) {
remove_surrounding_whitespace_nodes(node.fragment.nodes);
return {
Expand Down
43 changes: 38 additions & 5 deletions packages/svelte/src/compiler/phases/1-parse/state/tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import read_context from '../read/context.js';
import read_expression from '../read/expression.js';
import { error } from '../../../errors.js';
import { create_fragment } from '../utils/create.js';
import { parse_expression_at } from '../acorn.js';
import { walk } from 'zimmerframe';
import { parse } from '../acorn.js';

const regex_whitespace_with_closing_curly_brace = /^\s*}/;

Expand Down Expand Up @@ -532,21 +532,54 @@ function special(parser) {
// {@const a = b}
parser.require_whitespace();

const expression = read_expression(parser);
const CONST_LENGTH = 'const '.length;
parser.index = parser.index - CONST_LENGTH;

let end_index = parser.index;
/** @type {import('estree').VariableDeclaration | undefined} */
let declaration = undefined;

if (!(expression.type === 'AssignmentExpression' && expression.operator === '=')) {
const dummy_spaces = parser.template.substring(0, parser.index).replace(/[^\n]/g, ' ');
while (true) {
end_index = parser.template.indexOf('}', end_index + 1);
if (end_index === -1) break;
try {
const node = parse(
dummy_spaces + parser.template.substring(parser.index, end_index),
parser.ts
).body[0];
if (node?.type === 'VariableDeclaration') {
declaration = node;
break;
}
} catch (e) {
continue;
}
}

if (
declaration === undefined ||
declaration.declarations.length !== 1 ||
declaration.declarations[0].init === undefined
) {
error(start, 'invalid-const');
}

parser.allow_whitespace();
parser.index = end_index;
parser.eat('}', true);

const id = declaration.declarations[0].id;
if (id.type === 'Identifier') {
// Tidy up some stuff left behind by acorn-typescript
id.end = (id.start ?? 0) + id.name.length;
}

parser.append(
/** @type {import('#compiler').ConstTag} */ ({
type: 'ConstTag',
start,
end: parser.index,
expression
declaration
Copy link
Member

Choose a reason for hiding this comment

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

Mhhm it probably makes sense to make this a VariableDeclaration instead of an AssignmentExpression, and as such rename the property. For backwards compatibility this should be handled in legacy.js to transform it back to what it was in the Svelte 4 AST.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ohhhhh I forgot to consider about the legacy mode.

I think I can copy this. (Once already I did simmilar thing)
5e30745#diff-07a9d6689d63c982b2e3a45ae4bd624240d27824e8c5bdce8a539b4d2a7c36f1R576-R583

Copy link
Member Author

Choose a reason for hiding this comment

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

})
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1611,19 +1611,20 @@ export const template_visitors = {
);
},
ConstTag(node, { state, visit }) {
const declaration = node.declaration.declarations[0];
// TODO we can almost certainly share some code with $derived(...)
if (node.expression.left.type === 'Identifier') {
if (declaration.id.type === 'Identifier') {
state.init.push(
b.const(
node.expression.left,
declaration.id,
b.call(
'$.derived',
b.thunk(/** @type {import('estree').Expression} */ (visit(node.expression.right)))
b.thunk(/** @type {import('estree').Expression} */ (visit(declaration.init)))
)
)
);
} else {
const identifiers = extract_identifiers(node.expression.left);
const identifiers = extract_identifiers(declaration.id);
const tmp = b.id(state.scope.generate('computed_const'));

// Make all identifiers that are declared within the following computed regular
Expand All @@ -1639,8 +1640,8 @@ export const template_visitors = {
[],
b.block([
b.const(
/** @type {import('estree').Pattern} */ (visit(node.expression.left)),
/** @type {import('estree').Expression} */ (visit(node.expression.right))
/** @type {import('estree').Pattern} */ (visit(declaration.id)),
/** @type {import('estree').Expression} */ (visit(declaration.init))
),
b.return(b.object(identifiers.map((node) => b.prop('init', node, node))))
])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1049,8 +1049,9 @@ const template_visitors = {
state.template.push(t_expression(id));
},
ConstTag(node, { state, visit }) {
const pattern = /** @type {import('estree').Pattern} */ (visit(node.expression.left));
const init = /** @type {import('estree').Expression} */ (visit(node.expression.right));
const declaration = node.declaration.declarations[0];
const pattern = /** @type {import('estree').Pattern} */ (visit(declaration.id));
const init = /** @type {import('estree').Expression} */ (visit(declaration.init));
state.init.push(b.declaration('const', pattern, init));
},
DebugTag(node, { state, visit }) {
Expand Down
13 changes: 10 additions & 3 deletions packages/svelte/src/compiler/phases/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -437,15 +437,21 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
next();
},

VariableDeclaration(node, { state, next }) {
VariableDeclaration(node, { state, path, next }) {
const is_parent_const_tag = path.at(-1)?.type === 'ConstTag';
for (const declarator of node.declarations) {
/** @type {import('#compiler').Binding[]} */
const bindings = [];

state.scope.declarators.set(declarator, bindings);

for (const id of extract_identifiers(declarator.id)) {
const binding = state.scope.declare(id, 'normal', node.kind, declarator.init);
const binding = state.scope.declare(
id,
is_parent_const_tag ? 'derived' : 'normal',
node.kind,
declarator.init
);
bindings.push(binding);
}
}
Expand Down Expand Up @@ -594,7 +600,8 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
},

ConstTag(node, { state, next }) {
for (const identifier of extract_identifiers(node.expression.left)) {
const declaration = node.declaration.declarations[0];
for (const identifier of extract_identifiers(declaration.id)) {
state.scope.declare(
/** @type {import('estree').Identifier} */ (identifier),
'derived',
Expand Down
7 changes: 7 additions & 0 deletions packages/svelte/src/compiler/types/legacy-nodes.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { StyleDirective as LegacyStyleDirective, Text } from '#compiler';
import type {
ArrayExpression,
AssignmentExpression,
Expression,
Identifier,
MemberExpression,
Expand Down Expand Up @@ -168,6 +169,11 @@ export interface LegacyTitle extends BaseElement {
name: 'title';
}

export interface LegacyConstTag extends BaseNode {
type: 'ConstTag';
expression: AssignmentExpression;
}

export interface LegacyTransition extends BaseNode {
type: 'Transition';
/** The 'x' in `transition:x` */
Expand Down Expand Up @@ -215,6 +221,7 @@ export type LegacyElementLike =
| LegacyWindow;

export type LegacySvelteNode =
| LegacyConstTag
| LegacyElementLike
| LegacyAttributeLike
| LegacyAttributeShorthand
Expand Down
7 changes: 5 additions & 2 deletions packages/svelte/src/compiler/types/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import type { Binding } from '#compiler';
import type {
ArrayExpression,
ArrowFunctionExpression,
AssignmentExpression,
VariableDeclaration,
VariableDeclarator,
Expression,
FunctionDeclaration,
FunctionExpression,
Expand Down Expand Up @@ -130,7 +131,9 @@ export interface Comment extends BaseNode {
/** A `{@const ...}` tag */
export interface ConstTag extends BaseNode {
type: 'ConstTag';
expression: AssignmentExpression;
declaration: VariableDeclaration & {
declarations: [VariableDeclarator & { id: Identifier; init: Expression }];
};
}

/** 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: '<p>10 * 10 = 100</p><p>20 * 20 = 400</p>'
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script lang="ts">
const boxes = [ { width: 10, height: 10 }, { width: 20, height: 20 } ];
</script>

{#each boxes as box}
{@const area: number = box.width * box.height}
<p>{box.width} * {box.height} = {area}</p>
{/each}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { test } from '../../test';

export default test({
html: '<p>{}</p>'
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script lang="ts">
</script>

{@const name: string = "{}"}
<p>{name}</p>