feat: upgrade plugin to v9 #23

d3lm · 2024-06-24T13:52:50Z

This PR updates our eslint plugin and migrates from v8 to v9. This includes some breaking changes specifically around the config since v9 introduced a flat config file format.

I have also improved some of our custom lint rules and fixed a few bugs and did a bunch of cleanup.

Furthermore, I got rid of yarn in favor of using pnpm.

I also added CI via GitHub Actions which now build, lint, and test the project.

For the version I went with a new minor version since we can't just use another minor version. That's because using this updated plugin requires to update the config on the consumer side as well.

Nemikolh

Impressive changes! 🤯

I mostly have questions nothing major

Nemikolh · 2024-06-24T14:03:23Z

src/rules/comment-syntax.ts

@@ -155,7 +154,7 @@ export default createRule<Options, MessageIds>({
              context.report({ node: comment, messageId: 'lineCommentCapital' });
            }

-            if (lastChar === '.') {
+            if (lastChar === '.' && !comment.value.endsWith('etc.') && !comment.value.endsWith(' ...')) {


Woo that's a nice improvements 🤌

Yea indeed. Was annoying that this wasn't allowed cause it's pretty usual to have that.

Nemikolh · 2024-06-24T14:05:02Z

package.json

+    "@typescript-eslint/eslint-plugin": "^8.0.0-alpha.30",
+    "@typescript-eslint/parser": "^8.0.0-alpha.30",
+    "@typescript-eslint/utils": "^8.0.0-alpha.30",


Why do we need the alpha versions?

Yea unfortunately we do need them because only those support the new flat config format.

But I talked to James Henry and they will release the real version very soon, since it's already a release candidate.

Nemikolh · 2024-06-24T14:20:30Z

src/configs/javascript.ts

Do we know if those rules pass on every project that we own that uses this package?

These are the exact same rules as before only pulled into a separate file. But I can check tomorrow if if works on some other projects as well and can come back with my findings.

Nemikolh

Seeing how many files were changed on the TutorialKit repo, I feel like we should merge this ASAP (as well as the one in TutorialKit) and feedback in here can be addressed in future PRs.

Otherwise it'll be an endless series of fixing conflicts on the TutorialKit side 😓

Either that or we publish an "alpha" version of that branch that we use on the TutorialKit side.

What do you think @d3lm?

d3lm · 2024-06-24T17:05:54Z

I mean all those changes in the TK repo are just because it didn't meet our conventions defined in our plugin but it's not specific to the upgrade to eslint v9. But yea, we can release an alpha if you prefer that. I think you didn't have any comments to be addressed other than if we know whether it works on other projects. I do think that it should work because there's no new rules added here and it's just upgrading to a newer version of eslint but worth checking! With a pre-release or alpha we can do that testing and fix any bugs that we encounter along the way. It will be an iterative approach anyways so we can upgrade the repos we have one by one if we want to and it shouldn't be a big bang.

Nemikolh · 2024-06-24T17:41:34Z

Oh I was referring to other reviewers of this PR.

If we want for everyone else to have reviewed it and have their feedback addressed before merging, then I think it would make sense to have an alpha version so that we can move forward on the eslint PR on the TutorialKit repo.

Or we just merge it, release a new version and address the feedback as follow-up PRs which works fine with me.

kirjavascript

looks good to me!

kirjavascript · 2024-06-24T17:47:34Z

pnpm-lock.yaml

what's the reason for the switch to pnpm? is it just performance or something?

I only did that switch since we started to use pnpm in other repos as well and it's a little faster than other package managers and the more repos use pnpm the less disk space is consumed cause it uses symlinks to the global store. But if anyone feels strongly about one or the other then I am happy to discuss and potentially revert back. But yarn v1 is just unmaintained at this point and I wanted to switch to something that is maintained yet gives us good ergonomics and performance.

I just have a preference for consistency, rather than some specific package manager.

This is the first time I've seen it used and it seemed a bit random considering we already use npm and yarn elsewhere, but if we're moving to repos pnpm generally that's fine.

Good point! I think we gotta decide on what we wanna use going forward. TutorialKit uses pnpm and it was chosen because it's gonna be open source and because we maintain a good partnership with the open source community / the Vite ecosystem. But I am a 100% with you on consistency! We gotta decide, but I would personally wanna move away from yarn v1 cause it's unmaintained.

Generally, pnpm has the better workspace support I'd say and gives more options and flexibility in the way it can be configured compared to npm.

d3lm · 2024-06-24T18:05:19Z

I was referring to other reviewers of this PR.

Oh but you did ping me, unless I misinterpreted that 🙉

d3lm · 2024-06-24T18:05:49Z

Oh I get it now 😅

d3lm · 2024-06-24T18:06:16Z

But I am fine either way. We can definitely go with an alpha version for now if we want that. I don't have a strong opinion on this one.

d3lm · 2024-06-24T18:17:11Z

I just tested the new version on our registry proxy and it worked well and caught some issues even.

src/rules/comment-syntax.ts

SamVerschueren

Great work on upgrading this 🙏 !

SamVerschueren

🔥

d3lm · 2024-06-25T08:54:58Z

Ok, I merged #24 and merged main into this branch so it should be good to go.

feat: upgrade plugin to v9

52a861d

d3lm force-pushed the delm/feat/migrate-to-eslint-v9 branch from 6a610cb to 52a861d Compare June 24, 2024 13:54

d3lm requested review from Nemikolh, kirjavascript, SamVerschueren and jrvidal June 24, 2024 13:56

Nemikolh reviewed Jun 24, 2024

View reviewed changes

Nemikolh approved these changes Jun 24, 2024

View reviewed changes

kirjavascript approved these changes Jun 24, 2024

View reviewed changes

fixup: update package.json

7ff651d

SamVerschueren reviewed Jun 25, 2024

View reviewed changes

src/rules/comment-syntax.ts Outdated Show resolved Hide resolved

SamVerschueren approved these changes Jun 25, 2024

View reviewed changes

fixup: add more tests and update comment rule

2a4cd1e

d3lm requested a review from SamVerschueren June 25, 2024 07:58

SamVerschueren approved these changes Jun 25, 2024

View reviewed changes

d3lm mentioned this pull request Jun 25, 2024

feat: enforce newlines after if-statements, add fixers, and adjust comment style #24

Merged

d3lm added 2 commits June 25, 2024 10:51

Merge branch 'main' into delm/feat/migrate-to-eslint-v9

b318c32

fixup: merge main

5fe5bba

d3lm force-pushed the delm/feat/migrate-to-eslint-v9 branch from 3c4e8d6 to 5fe5bba Compare June 25, 2024 08:54

d3lm merged commit d665151 into main Jun 25, 2024
2 checks passed

d3lm deleted the delm/feat/migrate-to-eslint-v9 branch June 25, 2024 11:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: upgrade plugin to v9 #23

feat: upgrade plugin to v9 #23

d3lm commented Jun 24, 2024 •

edited

Loading

Nemikolh left a comment

Nemikolh Jun 24, 2024

d3lm Jun 24, 2024

Nemikolh Jun 24, 2024

d3lm Jun 24, 2024

d3lm Jun 24, 2024

Nemikolh Jun 24, 2024

d3lm Jun 24, 2024

Nemikolh left a comment

d3lm commented Jun 24, 2024

Nemikolh commented Jun 24, 2024

kirjavascript left a comment

kirjavascript Jun 24, 2024

d3lm Jun 24, 2024

kirjavascript Jun 24, 2024 •

edited

Loading

d3lm Jun 24, 2024

d3lm Jun 24, 2024

d3lm commented Jun 24, 2024 •

edited

Loading

d3lm commented Jun 24, 2024

d3lm commented Jun 24, 2024 •

edited

Loading

d3lm commented Jun 24, 2024

SamVerschueren left a comment

SamVerschueren left a comment

d3lm commented Jun 25, 2024

feat: upgrade plugin to v9 #23

feat: upgrade plugin to v9 #23

Conversation

d3lm commented Jun 24, 2024 • edited Loading

Nemikolh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nemikolh left a comment

Choose a reason for hiding this comment

d3lm commented Jun 24, 2024

Nemikolh commented Jun 24, 2024

kirjavascript left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kirjavascript Jun 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d3lm commented Jun 24, 2024 • edited Loading

d3lm commented Jun 24, 2024

d3lm commented Jun 24, 2024 • edited Loading

d3lm commented Jun 24, 2024

SamVerschueren left a comment

Choose a reason for hiding this comment

SamVerschueren left a comment

Choose a reason for hiding this comment

d3lm commented Jun 25, 2024

d3lm commented Jun 24, 2024 •

edited

Loading

kirjavascript Jun 24, 2024 •

edited

Loading

d3lm commented Jun 24, 2024 •

edited

Loading

d3lm commented Jun 24, 2024 •

edited

Loading