Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Update eslint-plugin-unicorn to v47 #2146

Merged
merged 10 commits into from May 8, 2023

Conversation

TatsuyaYamamoto
Copy link
Contributor

@TatsuyaYamamoto TatsuyaYamamoto commented May 6, 2023

Related issues: nothing

currently, main branch of eslint-plugin-vue cannot run lint.
Due to a bug in eslint-plugin-unicorn, if eslint-plugin-vue installs ESLint v42.0, eslint-plugin-unicorn must be v46.0.1 or higher.

> eslint-plugin-vue@9.11.0 lint
> eslint . --rulesdir eslint-internal-rules && markdownlint "**/*.md"


Oops! Something went wrong! :(

ESLint: 8.40.0

TypeError: Cannot read properties of undefined (reading 'getAllComments')

ref: sindresorhus/eslint-plugin-unicorn#2076

What I did

  1. update eslint-plugin-unicorn
  2. fix lint error turn off rules

How to fix lint error

  1. precondition => 254 problems (253 errors, 1 warning)
  2. npm run lint:fix => 4 problems (3 errors, 1 warning)
  3. set top-level await manually => 1 problem (0 errors, 1 warning)

@TatsuyaYamamoto
Copy link
Contributor Author

It seems to have another problem of eslint-plugin-vue and ESLint.

If ESLint v8.40 is installed, eslint-plugin-vue’s tests failed.
If ESLint v8.39.1 is installed, the test succeeds

Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Thanks for the dependency update and lint fixes! However, I'm not sure if they are all compatible with the Node versions we support. @ota-meshi which Node versions should we support? If some of the newer language features (importing builtin Node modules via the node: prefix; array .at() function; string .replaceAll() function) are not available in all supported Node versions, we should rather disable the respective lint rules.

Also, I'm not sure what causes the other failing tests.

eslint-internal-rules/no-invalid-meta-docs-categories.js Outdated Show resolved Hide resolved
@ota-meshi
Copy link
Member

ota-meshi commented May 6, 2023

Thank you for this PR.
However, source code changes contain many breaking changes.
Could you change the eslintrc so that we don't have to change the source code? Also, could you revert the changed source code?

@TatsuyaYamamoto
Copy link
Contributor Author

TatsuyaYamamoto commented May 7, 2023

@ota-meshi

Could you change the eslintrc so that we don't have to change the source code?

I turned off some rules

Also, could you revert the changed source code?

I reverted 92305cc, adf77e9

.eslintrc.js Outdated Show resolved Hide resolved
Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Thanks for the changes!

@FloEdelmann FloEdelmann changed the title fix: update eslint-plugin-unicorn to v47.0.0 and fix lint error Update eslint-plugin-unicorn to v47 May 8, 2023
Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

Thank you!

@ota-meshi ota-meshi merged commit d58fb19 into vuejs:master May 8, 2023
14 checks passed
@TatsuyaYamamoto TatsuyaYamamoto deleted the update-eslint-plugin-unicorn branch May 9, 2023 10:01
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.

None yet

3 participants