-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Conversation
🦋 Changeset detectedLatest commit: cba043c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
{@const ...}
tag accept type annotations{@const ...}
tag
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
parser.eat('}', true); | ||
|
||
parser.append( | ||
/** @type {import('#compiler').ConstTag} */ ({ | ||
type: 'ConstTag', | ||
start, | ||
end: parser.index, | ||
expression | ||
declaration |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I don't know why lint is not passed... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
{#if count > 4} | ||
{@const countString: string = count} <!-- ❌ Does not work --> | ||
<!-- ❌ Does not work --> | ||
{@const countString: string = count} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
fix: #9569
It may be considered a breaking change because it changes the structure of the AST of
{@const}
Svelte 5 rewrite
Please note that the Svelte codebase is currently being rewritten for Svelte 5. Changes should target Svelte 5, which lives on the default branch (
main
).If your PR concerns Svelte 4 (including updates to svelte.dev.docs), please ensure the base branch is
svelte-4
and notmain
.Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint