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

Make the lint command check .jsx files too #333

Merged
merged 1 commit into from Sep 28, 2017
Merged

Make the lint command check .jsx files too #333

merged 1 commit into from Sep 28, 2017

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Sep 27, 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)$/.

This now means there is redundancy in the eslint middleware options, however there isn't a reliable way to turn the test regex into a flat array of file extensions.

In a future major release, perhaps the eslint middleware could drop support for options.test and instead only accept the options.extensions array, which could then be mapped back to the test regex internally. However this would mean less flexible selection of files to be matched, plus mean that there would be inconsistent support for options.test between the various loader middlewares, so not sure whether it's the best way forwards anyway.

Fixes #332.

@edmorley edmorley self-assigned this Sep 27, 2017
@@ -37,6 +37,7 @@ const eslint = require('neutrino-middleware-eslint');
// Usage shows default values
neutrino.use(eslint, {
test: /\.(js|jsx)$/,
extensions: ['js', 'jsx'],
Copy link
Member

Choose a reason for hiding this comment

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

Should these be part of the eslint options?

Copy link
Member Author

Choose a reason for hiding this comment

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

The option will be ignored when used by eslint-loader, but yeah it probably belongs in eslint anyway. I'll update the PR and add a comment.

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.
@edmorley
Copy link
Member Author

The Travis failure appears unrelated.

@eliperelman
Copy link
Member

Thanks @edmorley!

@eliperelman eliperelman merged commit 08e6cac into neutrinojs:master Sep 28, 2017
@eliperelman
Copy link
Member

Released in v6.2.1.

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

Successfully merging this pull request may close these issues.

None yet

2 participants