Add requireConfigFile option #743

kaicataldo · 2019-01-17T19:43:36Z

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.

kaicataldo · 2019-01-17T22:49:07Z

lib/analyze-scope.js

@@ -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,


This is already set by https://github.com/babel/babel-eslint/pull/743/files#diff-6d186b954a58d5bb740f73d84fe39073R29

existentialism · 2019-01-18T04:48:19Z

README.md

@@ -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.


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-eslint will not parse any experimental syntax.

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.

loganfsmyth · 2019-01-19T04:42:25Z

lib/parse.js

  let ast;
+
  try {
    ast = parse(code, opts);


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?

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.

loganfsmyth · 2019-01-19T04:43:31Z

README.md

@@ -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.


Should we add some examples of using overrides in the ESLint config to target babel-eslint for specific files?

That's a good idea!

kaicataldo · 2019-01-21T16:05:59Z

Thanks! Will follow up with a few smaller PRs to address the suggested additions above.

* Add requireConfigFile option * Update README.md

kaicataldo force-pushed the requireconfigfile branch 2 times, most recently from 0ce8429 to 8f4afe8 Compare January 17, 2019 22:31

kaicataldo requested a review from loganfsmyth January 17, 2019 22:31

kaicataldo commented Jan 17, 2019

View reviewed changes

Add requireConfigFile option

2e55262

kaicataldo force-pushed the requireconfigfile branch from 810274e to 2e55262 Compare January 17, 2019 22:54

existentialism reviewed Jan 18, 2019

View reviewed changes

Update README.md

6067ddf

kaicataldo force-pushed the requireconfigfile branch from 3b94243 to 6067ddf Compare January 18, 2019 18:59

loganfsmyth reviewed Jan 19, 2019

View reviewed changes

loganfsmyth approved these changes Jan 19, 2019

View reviewed changes

existentialism approved these changes Jan 19, 2019

View reviewed changes

kaicataldo merged commit b9e40ab into babel:master Jan 21, 2019

kaicataldo deleted the requireconfigfile branch January 21, 2019 16:06

This was referenced Jan 22, 2019

Support for mixed JS and TS codebases - do not lint JS files typescript-eslint/typescript-eslint#109

Closed

Docs: Add glob-based configuration example #748

Merged

nicolo-ribaudo pushed a commit to babel/babel that referenced this pull request Nov 14, 2019

Add requireConfigFile option (babel/babel-eslint#743)

37cf65c

* Add requireConfigFile option * Update README.md

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add requireConfigFile option #743

Add requireConfigFile option #743

kaicataldo commented Jan 17, 2019 •

edited

kaicataldo Jan 17, 2019

existentialism Jan 18, 2019

existentialism Jan 18, 2019

kaicataldo Jan 18, 2019

loganfsmyth Jan 19, 2019

kaicataldo Jan 29, 2019

loganfsmyth Jan 19, 2019

kaicataldo Jan 19, 2019

kaicataldo commented Jan 21, 2019

		@@ -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.

Add requireConfigFile option #743

Add requireConfigFile option #743

Conversation

kaicataldo commented Jan 17, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaicataldo commented Jan 21, 2019

kaicataldo commented Jan 17, 2019 •

edited