Skip to content

feat: change it to use modern AST, if svelte v5 is installed #437

New issue

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

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

Already on GitHub? # to your account

Merged
merged 14 commits into from
Mar 3, 2024

Conversation

ota-meshi
Copy link
Member

@ota-meshi ota-meshi commented Nov 21, 2023

This PR will be changed to also use Svelte v5's modern AST.
I believe this has performance benefits as it allows we to skip the transformation step to legacy AST.

Copy link

changeset-bot bot commented Nov 21, 2023

🦋 Changeset detected

Latest commit: 2eea457

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte-eslint-parser Minor

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

@coveralls
Copy link

coveralls commented Nov 21, 2023

Pull Request Test Coverage Report for Build 8127822619

Details

  • 348 of 376 (92.55%) changed or added relevant lines in 12 files are covered.
  • 12 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-1.1%) to 90.55%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/context/index.ts 8 9 88.89%
src/parser/converts/element.ts 67 68 98.53%
src/parser/index.ts 1 2 50.0%
src/parser/converts/block.ts 56 59 94.92%
src/parser/html.ts 110 118 93.22%
src/parser/compat.ts 63 77 81.82%
Files with Coverage Reduction New Missed Lines %
src/context/index.ts 1 93.12%
src/parser/converts/attr.ts 3 90.05%
src/parser/converts/element.ts 8 92.21%
Totals Coverage Status
Change from base Build 8127758942: -1.1%
Covered Lines: 2773
Relevant Lines: 2954

💛 - Coveralls

export function getOptionsFromRoot(
svelteAst: SvAST.Ast | SvAST.AstLegacy,
): Compiler.SvelteOptionsRaw | null {
return (svelteAst as any).options?.__raw__ ?? null;
Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. I still don't know the correct way to get the <svelte:options> AST.

@ota-meshi ota-meshi mentioned this pull request Nov 22, 2023
18 tasks
@baseballyama
Copy link
Member

I think it's better to merge this after stable Svelte 5 is released?

@ota-meshi
Copy link
Member Author

I would like to improve the way to get the AST node of <svelte:options> if possible, but I don't think it is necessary to wait for the release of Svelte v5.

#437 (comment)

@ota-meshi ota-meshi force-pushed the use-modern branch 2 times, most recently from bb26940 to be5eecb Compare November 26, 2023 23:54
@baseballyama
Copy link
Member

MEMO: {@const} tag's AST will be change by sveltejs/svelte#9609.

@ota-meshi ota-meshi force-pushed the use-modern branch 3 times, most recently from c87e7a6 to 6ee516a Compare December 3, 2023 12:16
@niemyjski
Copy link

svelte 5 is already included in cli tooling so would be good to have this.

@ota-meshi ota-meshi force-pushed the use-modern branch 2 times, most recently from cfb8c50 to 7b8f3a4 Compare February 24, 2024 12:15
@ota-meshi ota-meshi marked this pull request as ready for review February 24, 2024 13:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants