Skip to content
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

Jump to bottom

feat: upgrade @typescript-eslint/utils to v6 #1508

Merged
merged 11 commits into from
Mar 22, 2024
Merged

feat: upgrade @typescript-eslint/utils to v6 #1508

merged 11 commits into from
Mar 22, 2024

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Feb 16, 2024

Really the changes needed were just type and testing based, though I'm not sure if we can support v5 without a breaking change anyway because we'd have to switch to requiring it as a peer dependency which is arguably still breaking?

v6 dropped support for Node v14 which we still support.

Closes #1401

(this is meant to be the min. required for the upgrade - I'll do other stuff like updating the linting plugins in another PR)

@G-Rath G-Rath requested a review from SimenB February 16, 2024 22:13
@G-Rath G-Rath force-pushed the update-typescript-eslint branch from 3dca273 to b8d120d Compare March 21, 2024 19:48
SimenB
SimenB reviewed Mar 21, 2024
View reviewed changes
package.json
@@ -95,7 +95,7 @@
]
},
"dependencies": {
"@typescript-eslint/utils": "^5.10.0"
"@typescript-eslint/utils": "^6.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

image

as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we're still supporting @typescript-eslint/eslint-plugin v5 and don't use a lot of the actual rules so I don't think there's a strong advantage - I'll double check that but I don't think it should block landing this right now

Copy link
Member

Choose a reason for hiding this comment

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

I think we can drop it - no reason too keep support when bumping version (when we're landing as major regardless)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok but can we do that in a PR after this so it gets a dedicated changelog entry?

Copy link
Member

@SimenB SimenB Mar 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

i.e. I think it makes sense to add support for further majors as a minor, but if we're doing a major release regardless, I don't think it's super valuable (beyond this special case when we're 2 majors behind 😅 )

Copy link
Member

Choose a reason for hiding this comment

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

for sure!

Copy link
Member

Choose a reason for hiding this comment

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

(sorry, I had the tab open and GH isn't super good with live updates 😅 let's blame rails!)

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: update this to the last v6 version.
That way you're enforcing that package managers always give you the latest version, rather than trying to resolve to some random one with fewer bug fixes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tend to favor not doing that to enable better deduplication downstream, given we consider this a very stable dependency

SimenB
SimenB reviewed Mar 21, 2024
View reviewed changes
src/rules/__tests__/unbound-method.test.ts
'Promise.resolve().then(console.log);',
"['1', '2', '3'].map(Number.parseInt);",
'[5.2, 7.1, 3.6].map(Math.floor);',
'const x = console.log;',
...(parseInt(rawTypeScriptESLintPluginVersion.split('.')[0], 10) >= 6
Copy link
Member

Choose a reason for hiding this comment

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

won't this always be true?

Copy link
Member

Choose a reason for hiding this comment

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

(also, we should use semver module for this)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No because we still support and test against @typescript-eslint/eslint-plugin v5

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll not replace this with semver since it'll just go away right after this PR when we drop support for v5

@G-Rath G-Rath requested a review from SimenB March 21, 2024 20:39
bradzacher
bradzacher reviewed Mar 21, 2024
View reviewed changes
src/rules/prefer-lowercase-title.ts
@@ -70,11 +68,14 @@ export default createRule<
ignore: {
type: 'array',
items: {
type: 'string',
// for some reason TypeScript thinks this _must_ be a read-only
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably need to update our parameter types to be const parameters to avoid weirdness like this.

@G-Rath G-Rath force-pushed the update-typescript-eslint branch from bf802b2 to e1024e7 Compare March 22, 2024 00:34
SimenB
SimenB approved these changes Mar 22, 2024
View reviewed changes
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

👍

@G-Rath G-Rath merged commit dc6e8cd into next Mar 22, 2024
38 checks passed
@G-Rath G-Rath deleted the update-typescript-eslint branch March 22, 2024 19:32
github-actions bot pushed a commit that referenced this pull request Mar 22, 2024
@semantic-release-bot
chore(release): 28.0.0-next.3 [skip ci]
9899ddf 
# [28.0.0-next.3](v28.0.0-next.2...v28.0.0-next.3) (2024-03-22)

### Features

* upgrade `@typescript-eslint/utils` to v6 ([#1508](#1508)) ([dc6e8cd](dc6e8cd))
@github-actions GitHub Actions
Copy link

🎉 This PR is included in version 28.0.0-next.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this pull request Apr 6, 2024
@semantic-release-bot
chore(release): 28.0.0 [skip ci]
5bc0b88 
# [28.0.0](v27.9.0...v28.0.0) (2024-04-06)

### Bug Fixes

* allow ESLint 9 as peer dependency ([#1547](#1547)) ([3c5e167](3c5e167))
* drop support for Node 19 ([#1548](#1548)) ([c87e388](c87e388))
* **no-large-snapshots:** avoid `instanceof RegExp` check for ESLint v9 compatibility ([#1542](#1542)) ([af4a9c9](af4a9c9))

### Features

* drop support for `@typescript-eslint/eslint-plugin` v5 ([#1530](#1530)) ([150e355](150e355))
* drop support for Node v14 ([#1527](#1527)) ([df5e580](df5e580))
* remove `no-if` rule ([#1528](#1528)) ([f976fc8](f976fc8))
* remove `snapshot` processor and `flat/snapshot` config ([#1532](#1532)) ([98087f9](98087f9))
* upgrade `@typescript-eslint/utils` to v6 ([#1508](#1508)) ([dc6e8cd](dc6e8cd))

### BREAKING CHANGES

* Node v19 is no longer supported
* removed unneeded `snapshot` processor and `flat/snapshot` config
* dropped support for `@typescript-eslint/eslint-plugin` v5
* dropped support for Node v14
* removed `no-if` in favor of `no-conditional-in-test`
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 6, 2024

🎉 This PR is included in version 28.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@SimenB SimenB mentioned this pull request Apr 6, 2024
Merged
This was referenced Jun 19, 2024
Merged
Merged
Open
Open
@Sec32fun32 Sec32fun32 mentioned this pull request Sep 2, 2024
Open
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@bradzacher bradzacher bradzacher left review comments

@SimenB SimenB SimenB approved these changes

Assignees
No one assigned
Labels
bug enhancement released on @next released rule: consistent-test-it rule: expect-expect rule: max-expects rule: max-nested-describe rule: no-alias-methods rule: no-commented-out-tests rule: no-conditional-expect rule: no-conditional-in-test rule: no-confusing-set-timeout rule: no-deprecated-functions rule: no-disabled-tests rule: no-done-callback rule: no-duplicate-hooks rule: no-export rule: no-focused-tests rule: no-hooks rule: no-identical-title rule: no-if rule: no-interpolation-in-snapshots rule: no-jasmine-globals rule: no-large-snapshots rule: no-mocks-import rule: no-restricted-jest-methods rule: no-restricted-matchers rule: no-standalone-expect rule: no-test-prefixes rule: no-test-return-statement rule: no-untyped-mock-factory rule: prefer-called-with rule: prefer-comparison-matcher rule: prefer-each rule: prefer-equality-matcher rule: prefer-expect-assertions rule: prefer-expect-resolves rule: prefer-hooks-in-order rule: prefer-hooks-on-top rule: prefer-lowercase-title rule: prefer-mock-promise-shorthand rule: prefer-snapshot-hint rule: prefer-spy-on rule: prefer-strict-equal rule: prefer-to-be rule: prefer-to-contain rule: prefer-to-have-length rule: prefer-todo rule: require-hook rule: require-to-throw-message rule: require-top-level-describe rule: unbound-method rule: valid-describe-callback rule: valid-expect rule: valid-expect-in-promise rule: valid-title
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@G-Rath @SimenB @bradzacher @dubzzz