-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Support ESLint v6 #1393
Support ESLint v6 #1393
Conversation
It's also having some sort of whitespace issue with a couple of tests as you can see here - https://ci.appveyor.com/project/benmosher/eslint-plugin-import/builds/25480510/job/i7a5k9x6j6s1hg8a#L3072. I think I've fixed it and then it comes back again haha |
1 similar comment
2 similar comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like you made a bunch of unrelated formatting changes; please revert them.
Are you perhaps running prettier on autosave in your editor or something? If so, disable that - prettier should only be run via eslint autofix, and only on repos that use it. |
src/rules/no-unused-modules.js
Outdated
@@ -15,7 +15,10 @@ try { | |||
var FileEnumerator = require('eslint/lib/cli-engine/file-enumerator').FileEnumerator | |||
listFilesToProcess = function (src) { | |||
var e = new FileEnumerator() | |||
return Array.from(e.iterateFiles(src)) | |||
return Array.from(e.iterateFiles(src), ({ filePath, ignored })=>({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return Array.from(e.iterateFiles(src), ({ filePath, ignored })=>({ | |
return Array.from(e.iterateFiles(src), ({ filePath, ignored }) => ({ |
Sorry about the double quote changes. I've reverted all of them. I have Prettier set to only work when there is a |
I suppose it is eslint in your editor doing that changes. The ci does not lint over |
Added a commit to only run tests for Would you like me to update Travis CI config to use |
Yes, please. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM pending a nit
Includes fixes for the use of `eslint/lib/util/traverser` that is now removed in ESLint 6 Co-Authored-By: golopot <[email protected]>
It's now supported
@ljharb OK I had to do one more thing and make |
Please @ljharb can you merge and release this? This is urgent now with eslint 6 in production everywhere (unfortunately).
|
@frederikhors If |
@ljharb the problem is not mine but it is here: prettier/prettier-vscode#672 |
Their problem is the same; upgrading to eslint v6 prior to peer deps upgrading is invalid. |
@ljharb still same problem today. |
The main issues are:
require.resolve
when passing theparser
option toRuleTester
(https://eslint.org/docs/user-guide/migrating-to-6.0.0#rule-tester-parser)ecmaVersion
whensourceType
ismodule
(https://eslint.org/docs/user-guide/migrating-to-6.0.0#-the-default-parser-now-validates-options-more-strictly)FileEnumerator
into what theimport/no-unused-modules
rule expects@typescript-eslint/parser
is using an internal module (eslint/lib/util/traverser
) which has been removed in ESLint@6 and therefore causing an error. It's been fixed in fix(parser): add simpleTraverse, replaces private ESLint util typescript-eslint/typescript-eslint#628 but not been released yet. (Updated to canary, thanks @golopot)typescript-eslint-parser
but this is an archived package. As I write this I realise I should probably use thetestVersion
function so that tests for this are skipped oneslint>=6
To test:
The only failing tests should be related to the TypeScript issue above.
Related to airbnb/javascript#2036.
Fixes #1362.