Add requireConfigFile option #743
Conversation
kaicataldo
force-pushed
the
requireconfigfile
branch
2 times, most recently
from
0ce8429
to
8f4afe8
Compare
| @@ -323,7 +323,7 @@ module.exports = function(ast, parserOptions) { | |||
| parserOptions.ecmaFeatures.globalReturn) === true, | |||
| impliedStrict: false, | |||
| sourceType: ast.sourceType, | |||
| ecmaVersion: parserOptions.ecmaVersion || 2018, | |||
| ecmaVersion: parserOptions.ecmaVersion, | |||
There was a problem hiding this comment.
kaicataldo
force-pushed
the
requireconfigfile
branch
from
810274e
to
2e55262
Compare
README.md
Outdated
| @@ -53,6 +53,7 @@ With the parser set, your configuration can be configured as described in the [C | |||
|
|
|||
| Additional configuration options can be set in your ESLint configuration under the `parserOptions` key. Please note that the `ecmaFeatures` config property may still be required for ESLint to work properly with features not in ECMAScript 5 by default. | |||
|
|
|||
| - `requireConfigFile` (default `true`) can be set to `false` to allow babel-eslint to run on files that do not have a Babel configuration associated with them. This can be useful for linting files that are not transformed by Babel (such as configuration files), though using [glob-based configuration](https://eslint.org/docs/user-guide/configuring#configuration-based-on-glob-patterns) to use the default parser for these files is recommended. Note that when no configuration file is found that babel-eslint will not parse any experimental syntax. | |||
There was a problem hiding this comment.
Note that when no configuration file is found that babel-eslint will not parse any experimental syntax.
that -> then?
Or maybe better as:
Note, when no configuration file is found,
babel-eslintwill not parse any experimental syntax.
There was a problem hiding this comment.
Also, maybe:
This can be useful for linting files that are not transformed by Babel (such as configuration files), though we recommend using the default parser via glob-based configuration.
kaicataldo
force-pushed
the
requireconfigfile
branch
from
3b94243
to
6067ddf
Compare
| let ast; | ||
|
|
||
| try { | ||
| ast = parse(code, opts); |
There was a problem hiding this comment.
Not related to this PR, but do we want to throw a helpful error if this ends up returning null? Otherwise I assume it will throw if someone tries to run ESLint on a file that is in the user's .babelrc?
There was a problem hiding this comment.
Question about this: It looks like babelCore#parseSync returns null when there isn't a config, but since we have a default configuration we pass through if a file isn't found with requireConfig: false and it warns when a config isn't found with requireConfig: true, I'm not seeing how babel-eslint can get into this state.
| @@ -53,6 +53,7 @@ With the parser set, your configuration can be configured as described in the [C | |||
|
|
|||
| Additional configuration options can be set in your ESLint configuration under the `parserOptions` key. Please note that the `ecmaFeatures` config property may still be required for ESLint to work properly with features not in ECMAScript 5 by default. | |||
|
|
|||
| - `requireConfigFile` (default `true`) can be set to `false` to allow babel-eslint to run on files that do not have a Babel configuration associated with them. This can be useful for linting files that are not transformed by Babel (such as tooling configuration files), though we recommend using the default parser via [glob-based configuration](https://eslint.org/docs/user-guide/configuring#configuration-based-on-glob-patterns). Note: babel-eslint will not parse any experimental syntax when no configuration file is found. | |||
There was a problem hiding this comment.
Should we add some examples of using overrides in the ESLint config to target babel-eslint for specific files?
|
Thanks! Will follow up with a few smaller PRs to address the suggested additions above. |
* Add requireConfigFile option * Update README.md
fixes #738
I'm not entirely sure what the best way to go about testing this would be. We don't have tests for a lot of the behavior surrounding config options at the moment, and I'd love to figure out a way to do it.
As I see it, it seems like a feasible way to test this would be to create fixture dirs for all the various possible configuration variations, run the linter on the fixture dir, and make assertions around that. One thing I'm not entirely sure about is how to test for the absence of a config file (since there's one in
test/that will be the default if another isn't supplied). I'm hoping there's a way that doesn't feel quite so clunky and repetitive.Do we want to merge this as is since this stuff hasn't been tested in the past and try to see if we can improve testing in the future with #739? Happy to defer to the team's consensus on this.