Uh oh!
There was an error while loading. Please reload this page.
Update top-level lint deps - #1544
Conversation
There was a problem hiding this comment.
Let's just remove this one? I don't think we ever enabled it in the end, and there are still some issues preventing it from working well with eslint-loader (specifically, fixing the exporting file doesn't "recheck" the importing file).
There was a problem hiding this comment.
We use the plugin in our eslint config so I don't think we can drop it, it's also a peer dep.
p.s. these packages are the top-level ones and strictly used for CI linting, they don't persist to the end-user in any way.
There was a problem hiding this comment.
Ah, I forgot about ''import/no-webpack-loader-syntax''. Okay
There was a problem hiding this comment.
Should we enable ESLint's native support for async/await?
See note in https://github.com/babel/babel-eslint/releases/tag/v7.0.0.
There was a problem hiding this comment.
This is a peerDep of our config, and we can't switch to ESLint's native support until import() lands.
There was a problem hiding this comment.
See import-js/eslint-plugin-import#700.
The babel-eslint parser is required for it to be parsed properly (syntax parsing is not the job of an eslint plugin).
There was a problem hiding this comment.
Will unpin, what about our preset (eslint-config-react-app)? Lerna doesn't bump this automatically.
There was a problem hiding this comment.
I'm not sure why do we even have it in root package.json?
There was a problem hiding this comment.
Our CI currently runs lint in e2e-simple, see https://travis-ci.org/facebookincubator/create-react-app/jobs/201147269#L204-L234. We don't treat warnings as failures though, would you like to?
There was a problem hiding this comment.
Oh okay. Yeah I think we should fail on them.
gaearon
commented
Feb 13, 2017
To answer your earlier question—we unpinned ESLint config in #1110. |
gaearon
left a comment
There was a problem hiding this comment.
Looks fine (but please unpin the ESLint plugin).
See other notes for optional changes.
There was a problem hiding this comment.
/home/travis/build/facebookincubator/create-react-app/packages/react-dev-utils/InterpolateHtmlPlugin.js
18:1 warning 'use strict' is unnecessary inside of modules strict
I can ignore it (like I did to the CLI), I was just listening to eslint. 😛 Was waiting to see if CI passed on node 4.
There was a problem hiding this comment.
I'm confused why it thinks it's a module. We're not using import syntax anywhere.
There was a problem hiding this comment.
We tell it so, I'll put the stricts back.
parserOptions: {
ecmaVersion: 6,
> sourceType: 'module',
ecmaFeatures: {
jsx: true,
generators: true,
experimentalObjectRestSpread: true
}
},
There was a problem hiding this comment.
We could override extending our own config, but I'm not sure if that's necessary.
gaearon
commented
Feb 13, 2017
We actually need to add stricts to Node-targeted code. Like |
Timer
commented
Feb 13, 2017
Most of these configs contain |
This is weird (and I think it's wrong). |
Timer
commented
Feb 13, 2017
Sorry, I didn't mean to state that as fact -- I derived it's been a problem before from the conversation on eslint/eslint#4832, but nothing I change seems to be making it catch and require the |
gaearon
commented
Mar 5, 2017
Instead let's do #1729. |
No description provided.