-
Notifications
You must be signed in to change notification settings - Fork 25
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
List ESLint and plugins as peerDependencies + use babel-eslint
parser
#8
Conversation
We only need ESLint to lint the eslint-config-expensify project itself, so I'm killing the rest accordingly
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.
🌮
extends: './index.js' | ||
} | ||
extends: './legacy.js' | ||
}; |
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.
why this?
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.
This is saying: When linting this repository (eslint-config-expensify
) we lint it with the ruleset of the "legacy" rules. And since we only write plain vanilla JavaScript, I figured that's alright. We don't need plugins like eslint-plugin-react
or eslint-plugin-jsx-a11y
here.
I am confused by this, like you also mentioned, eslint never supported react out of the box, thats why we use |
Sorry - this is some back and forth after #7.
But when I was trying to install the update to our Web-Expensify repository I noticed that ESLint does not scan subdirectories of
node_modules/
for plugins anymore.So it is not enough to list e.g.
eslint-plugin-react
as a dependency of this package anymore, but we need to actually install it as a separate first-level dependency in Web-Expensify.To do so properly, you'd mention them here as
peerDependency
.Extra note: ESLint is working on undoing that at some point again, the RFC proposal was accepted already: eslint/rfcs#7
But the actual implementation is still pending: eslint/rfcs#14
And I don't really want to hold on that now.
Second thing: We need to use
babel-eslint
as our parser, because (for whatever reason) ESLint seems to have dropped the support to do React out of the box. I couldn't really find the changelog item pointing that out, but with this change everything is working again. Allegedly it has always been like that, but that made me wonder how it has worked in the past for us. Let me know if you have other ideas, but we acn also just merge this as is.