Skip to content

Conversation

@JoshuaKGoldberg
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg commented Jul 6, 2024

Description

👋 Hi! Coming over from typescript-eslint/typescript-eslint#9141: we're working on typescript-eslint v8 and would like to try the beta out on repos like this one.

I'm sending this draft PR as a reference to see what issues we might give you in upgrading and to try to be helpful. If this is annoying noise to you, please forgive me, I'll withdraw the PR. But I hope this is useful on your end - and please let me know if you have any feedback! ❤️

I've put inline comments on any changes. There shouldn't be anything user-facing (this isn't a feat: or fix: PR).

@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

'@typescript-eslint/no-empty-object-type': [
'error',
{ allowInterfaces: 'with-single-extends' }, // maybe we should turn this on in a new PR
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

'@typescript-eslint/no-inferrable-types': 'off',
'@typescript-eslint/no-unused-vars': 'off', // maybe we should turn this on in a new PR
'@typescript-eslint/no-var-requires': 'off',
'@typescript-eslint/no-require-imports': 'off',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@typescript-eslint/no-var-requires was merged into @typescript-eslint/no-require-imports: typescript-eslint/typescript-eslint#8092

'no-extra-semi': 'off',
'@typescript-eslint/no-extra-semi': 'off', // conflicts with prettier
'@typescript-eslint/no-inferrable-types': 'off',
'@typescript-eslint/no-unused-expressions': 'off', // maybe we should turn this on in a new PR
Copy link
Contributor Author

Choose a reason for hiding this comment

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

): ResultPromise<EO & (keyof EO extends 'stdio' ? {} : { stdio: 'inherit' })> {
): ResultPromise<
EO & (keyof EO extends 'stdio' ? object : { stdio: 'inherit' })
> {
Copy link
Contributor Author

@JoshuaKGoldberg JoshuaKGoldberg Jul 6, 2024

Choose a reason for hiding this comment

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

https://v8--typescript-eslint.netlify.app/rules/ban-types: this is now covered by @typescript-eslint/no-empty-object-type. This particular object is better described as object, I think.

@bluwy
Copy link
Member

bluwy commented Jul 24, 2024

The changes look great to me. I personally don't mind trying the beta here. Is there anything else you plan to do as it's in draft?

@JoshuaKGoldberg
Copy link
Contributor Author

Is there anything else you plan to do as it's in draft?

Nothing while we're still in beta. Once we release as stable -tentatively in a week!- I was going to bump the version to stable & un-draft this & other beta testing PRs.

@bluwy
Copy link
Member

bluwy commented Jul 24, 2024

Awesome! Looking forward to the release

@nilsingwersen
Copy link
Contributor

This can be changed from draft now since typescript-eslint 8 got released right? @JoshuaKGoldberg @bluwy

@josephearl
Copy link

In template-react-ts, should the eslint config be updated to use projectService instead of project?

@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review August 4, 2024 21:53
@JoshuaKGoldberg
Copy link
Contributor Author

In template-react-ts, should the eslint config be updated to use projectService instead of project?

That sounds very reasonable and good to me, but I'm not a maintainer on this project and this PR is just an internal chore. Sounds like a good feature request / suggestion to file separately!

@JoshuaKGoldberg JoshuaKGoldberg requested a review from bluwy August 4, 2024 21:53
antfu
antfu previously approved these changes Aug 4, 2024
Copy link
Member

@antfu antfu left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM, thanks for the explanations!

@bluwy
Copy link
Member

bluwy commented Aug 5, 2024

I'll merge this in for now.

Re projectService, it looks like a much better default behaviour. If there aren't any drawbacks to enabling it, I think it sounds fine to do so.

@bluwy bluwy merged commit d1891fd into vitejs:main Aug 5, 2024
@JoshuaKGoldberg JoshuaKGoldberg deleted the typescript-eslint-8 branch August 5, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants