Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Neutrino lint results differ from neutrino build #332

Closed
edmorley opened this issue Sep 27, 2017 · 0 comments
Closed

Neutrino lint results differ from neutrino build #332

edmorley opened this issue Sep 27, 2017 · 0 comments

Comments

@edmorley
Copy link
Member

edmorley commented Sep 27, 2017

As part of upgrading to Neutrino v6, I was going through Treeherder's existing eslint globals configuration, to see if there were any entries that were no longer required. I found that if I commented out the 'React' global, then whilst neutrino lint would report no errors, neutrino build returned:

× Building project failed
./ui/js/reactrevisions.jsx
Module build failed: Module failed because of a eslint error.

C:\Users\Ed\src\treeherder\ui\js\reactrevisions.jsx
  14:11  error  'React' is not defined  no-undef
  57:15  error  'React' is not defined  no-undef
  ...

At first I thought this might be a NODE_ENV difference, however it turns out to actually be due to the file being a .jsx file.

Looking at the source for eslint-loader (which is what runs when using neutrino build) I see it uses eslint's executeOnText(), which just operates on the passed text regardless of file extension (since the webpack loader config already filtered by file extension).

However the neutrino lint command instead uses the eslint API via executeOnFiles() - and whilst that's likely the better choice for this use-case, means that if include is a directory (which it will be for Neutrino, since it will be set to neutrino.options.source), then eslint filters the files by the value for extension, which defaults to ['.js']:
https://eslint.org/docs/developer-guide/nodejs-api#cliengine

As such, the lint command needs to override extensions, setting it to ['.js', '.jsx'].

I'll open a PR.

eliperelman pushed a commit that referenced this issue Sep 28, 2017
Previously the `neutrino lint` command would only check `.js` files,
which was inconsistent with the build/start commands, since they use
`eslint-loader` and so instead use the `test` regex of `/\.(js|jsx)$/`.

Fixes #332.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

1 participant