feat: Support runes #425

baseballyama · 2023-11-14T15:04:47Z

No description provided.

changeset-bot · 2023-11-14T15:04:51Z

🦋 Changeset detected

Latest commit: 0a61e91

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 · 2023-11-14T15:06:01Z

Pull Request Test Coverage Report for Build 6898873227

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

39 of 43 (90.7%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.07%) to 91.045%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/parser/typescript/analyze/index.ts	33	37	89.19%

Totals
Change from base Build 6863793483:	0.07%
Covered Lines:	2276
Relevant Lines:	2415

💛 - Coveralls

baseballyama · 2023-11-14T15:09:45Z

I could not figure out how to test this because the globals are different for Svelte4 and Svelte5. Once I created the expected files for Svelte5, but there are too many additional lines. Any suggestions?

ota-meshi · 2023-11-15T01:06:34Z

Wow! I noticed this PR after opening #426. This PR seems to be a little more advanced than #426 as type information for runes is added.

I could not figure out how to test this because the globals are different for Svelte4 and Svelte5. Once I created the expected files for Svelte5, but there are too many additional lines. Any suggestions?

In #426, the idea is to explicitly opt in to runes mode using an option to make it work.
https://github.com/sveltejs/svelte-eslint-parser/blob/runes-vars/README.md#parseroptionssveltefeaturesrunes

baseballyama · 2023-11-15T01:20:11Z

In #426, the idea is to explicitly opt in to runes mode using an option to make it work.

From the user's point of view, using Svelte5 = being able to use runes, so if there is a way to not force users to configure it, that would be good.

About test, how about to extract variables part and prepare only this part for Svelte4 and Svelte5 separately.
Then we can prevent to make duplicated expected files.

https://github.com/sveltejs/svelte-eslint-parser/pull/425/files#diff-8907e5126cad5dd0dd35d0025fdd3086e01a29d3cb72fd9bb5f37b6352ba9048R3-R52

One question, my understanding is that the Svelte compiler is not bundled with svelte-eslint-parser so it always uses the version installed by the user, correct?

baseballyama · 2023-11-15T01:22:52Z

I think my todo is there.

Reduce duplicated expected files
Add tests for runes types
Handle $derived (I need to understand about this issue at first)

baseballyama · 2023-11-15T01:25:42Z

Oh but your PR supports .svelte.js files.
So somehow we need to combine these PRs.

ota-meshi · 2023-11-15T01:26:38Z

One question, my understanding is that the Svelte compiler is not bundled with svelte-eslint-parser so it always uses the version installed by the user, correct?

Yeah, we specifying svelte in peerDependencies and peerDependenciesMeta, so I expect the user to be able to use whatever version they want to use.

baseballyama · 2023-11-15T01:34:24Z

What do you think is the best way to do Next Action?

My opinion is that runes should not be configurable and should be reconize automatically by the parser, but since you are more professional in this field than me, please make a decision.
As for testing, it is possible to eliminate duplicates and I can handle that.
Regarding support for the .svelte.js file, would it be better to merge your PR after this PR is merged? Or we can collaborate on this branch and put them together as one PR, but I have no opinion here.

ota-meshi · 2023-11-15T02:24:09Z

My opinion is that runes should not be configurable and should be reconize automatically by the parser, but since you are more professional in this field than me, please make a decision.

I think we can consider what our options are after Svelte v5 is released. I believe we can remove options or even change the defaults for options.
However, it currently needs to be marked as an experimental feature, so I think it makes sense to provide it as an option for now.

would it be better to merge your PR after this PR is merged?

Yeah, I would like to change and merge #426 after this PR is merged.

ota-meshi · 2023-11-15T02:30:21Z

However, it currently needs to be marked as an experimental feature, so I think it makes sense to provide it as an option for now.

Oh, I think it's a good idea to judge by svelte version rather than using options. Installing svelte v5 means the user is explicitly using pre-release.

https://github.com/sveltejs/svelte-eslint-parser/pull/425/files#diff-cc6f76d59c9667a8e87dc3efb2b017199c77faed21c60201fc2633a0725c8217R15

ota-meshi · 2023-11-15T02:45:00Z

src/parser/globals.ts

+  "$state",
+  "$derived",
+  "$effect",
+  "$effect.pre",


Since $effect.pre is a member of $effect, I don't think it is necessary to define it here.

Suggested change

"$effect.pre",

baseballyama · 2023-11-15T15:37:48Z

Tomorrow I will add more tests for runes.

ota-meshi

It looks mostly good, but I did make one comment 😃

ota-meshi · 2023-11-17T00:33:08Z

src/parser/typescript/analyze/index.ts

+          if (
+            scopeManager.globalScope!.through.some(
+              (reference) => reference.identifier.name === svelte5Global,
+            )
+          ) {


By moving this judgment before the switch, I think we can eliminate the four same implementations.

Ohhhhh...
0a61e91

ota-meshi

LGTM!

baseballyama added 3 commits November 14, 2023 23:03

wip

e530a0b

add expected files for svelte5

c9fe482

register globals

82b0679

remove unnecessary files

e5ae173

baseballyama added 2 commits November 15, 2023 00:06

add changeset

c72d063

format

cacef02

update fixture

fd8ffa7

ota-meshi requested changes Nov 15, 2023

View reviewed changes

baseballyama added 11 commits November 15, 2023 21:18

add rune tests (wip)

dc6a5f0

reduce duplicated expected files

fc5ccaa

fix test

ab28e8e

revert

2bf76a9

update ci

8c334b3

fix ci

18775dd

co -> checkout

33031fb

add rune tests

1d9e7ab

fix lint

81b5cfa

remove duplicated

17f11fd

fix

1affdb3

baseballyama added 6 commits November 15, 2023 23:37

implement json sorter

cd492b6

how about now

8cc981c

remove more duplicated json

90ef27a

more

2a2f332

more

ef863fc

more

658dee1

add more rune ts tests

f18d155

baseballyama marked this pull request as ready for review November 16, 2023 15:00

baseballyama requested a review from ota-meshi November 16, 2023 15:00

can i skip?

01dfa17

ota-meshi requested changes Nov 17, 2023

View reviewed changes

ota-meshi mentioned this pull request Nov 17, 2023

Support for Svelte v5 #423

Open

16 tasks

dry out

0a61e91

baseballyama requested a review from ota-meshi November 17, 2023 01:55

ota-meshi approved these changes Nov 17, 2023

View reviewed changes

ota-meshi merged commit ff242c4 into main Nov 17, 2023
14 checks passed

ota-meshi deleted the support-runes branch November 17, 2023 03:05

github-actions bot mentioned this pull request Nov 17, 2023

chore: release svelte-eslint-parser (next) #428

Merged

github-actions bot mentioned this pull request Apr 7, 2024

chore: release svelte-eslint-parser #500

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support runes #425

feat: Support runes #425

baseballyama commented Nov 14, 2023

changeset-bot bot commented Nov 14, 2023 •

edited

coveralls commented Nov 14, 2023 •

edited by github-actions bot

baseballyama commented Nov 14, 2023

ota-meshi commented Nov 15, 2023

baseballyama commented Nov 15, 2023

baseballyama commented Nov 15, 2023 •

edited

baseballyama commented Nov 15, 2023

ota-meshi commented Nov 15, 2023

baseballyama commented Nov 15, 2023

ota-meshi commented Nov 15, 2023

ota-meshi commented Nov 15, 2023

ota-meshi Nov 15, 2023

baseballyama Nov 15, 2023

baseballyama commented Nov 15, 2023

ota-meshi left a comment

ota-meshi Nov 17, 2023

baseballyama Nov 17, 2023

ota-meshi left a comment

feat: Support runes #425

feat: Support runes #425

Conversation

baseballyama commented Nov 14, 2023

changeset-bot bot commented Nov 14, 2023 • edited

🦋 Changeset detected

coveralls commented Nov 14, 2023 • edited by github-actions bot

Pull Request Test Coverage Report for Build 6898873227

Warning: This coverage report may be inaccurate.

💛 - Coveralls

baseballyama commented Nov 14, 2023

ota-meshi commented Nov 15, 2023

baseballyama commented Nov 15, 2023

baseballyama commented Nov 15, 2023 • edited

baseballyama commented Nov 15, 2023

ota-meshi commented Nov 15, 2023

baseballyama commented Nov 15, 2023

ota-meshi commented Nov 15, 2023

ota-meshi commented Nov 15, 2023

ota-meshi Nov 15, 2023

Choose a reason for hiding this comment

baseballyama Nov 15, 2023

Choose a reason for hiding this comment

baseballyama commented Nov 15, 2023

ota-meshi left a comment

Choose a reason for hiding this comment

ota-meshi Nov 17, 2023

Choose a reason for hiding this comment

baseballyama Nov 17, 2023

Choose a reason for hiding this comment

ota-meshi left a comment

Choose a reason for hiding this comment

changeset-bot bot commented Nov 14, 2023 •

edited

coveralls commented Nov 14, 2023 •

edited by github-actions bot

baseballyama commented Nov 15, 2023 •

edited