Extending ESLint config not working

[conor-cafferkey-sociomantic]

Describe the bug

Extending ESLint config not working.

Environment

  System:
    OS: macOS 10.14.6
    CPU: (4) x64 Intel(R) Core(TM) i7-7660U CPU @ 2.50GHz
  Binaries:
    Node: 8.15.0 - ~/.nvm/versions/node/v8.15.0/bin/node
    Yarn: 1.17.3 - /usr/local/bin/yarn
    npm: 6.4.1 - ~/.nvm/versions/node/v8.15.0/bin/npm
  Browsers:
    Chrome: 76.0.3809.100
    Firefox: 67.0
    Safari: 12.1.2
  npmPackages:
    react: ^16.9.0 => 16.9.0
    react-dom: ^16.9.0 => 16.9.0
    react-scripts: 3.1.0 => 3.1.0
  npmGlobalPackages:
    create-react-app: Not Found

Steps to reproduce

1. yarn create react-app testapp (or npx create-react-app testapp)
2. cd testapp
3. yarn add eslint-config-airbnb (or npm install eslint-config-airbnb)
4. Add the following to package.json:

"eslintConfig": { "extends": [ "react-app", "airbnb"] } 

5. EXTEND_ESLINT=true yarn start ( or EXTEND_ESLINT=true npm start)

Expected behavior

• eslint-config-airbnb should be used
• app should not compile

Actual behavior

• eslint-config-airbnb is not used
• app compiles (it shouldn’t!)

Problem

webpack.config.js checks thateslintConfig.extends exists:

create-react-app/packages/react-scripts/config/webpack.config.js

Line 355 in c0b4173

eslintConfig.extends &&

and – if it does – that it includes 'react-app':

create-react-app/packages/react-scripts/config/webpack.config.js

Line 356 in c0b4173

eslintConfig.extends.includes('react-app')

It seems that the property extends never exists on eslintConfig(??)

Deleting the above two lines yields the expected behavior (eslint-config-airbnb is picked up and we see lint errors / Failed to compile message)

[HerbCaudill]

I was just coming to file this same issue.

The problem is that we're checking the extends property for react-app, but by this time the config object has been expanded so that it actually contains the contents of eslint-config-react-app and the extends property is no longer present.

This can be confirmed by logging eslintConfig as soon as it's been obtained.

With this eslint config in package.json...

"eslintConfig": {
  "extends": [
    "react-app"
  ],
  "rules": {
    "@typescript-eslint/no-unused-vars": "off"
  }
}

...log eslintConfig after line 348:

create-react-app/packages/react-scripts/config/webpack.config.js

Lines 344 to 348 in c0b4173

	try {
	eslintConfig = eslintCli.getConfigForFile(paths.appIndexJs);
	} catch (e) {
	// A config couldn't be found.
	}

 console.log(eslintConfig) // 🡐 

... then run yarn build. I get something like this:

{
  env: { browser: true, commonjs: true, es6: true, jest: true, node: true },
  globals: {},
  parser: 'C:\\git\\herbcaudill\\screentimebank\\node_modules\\@typescript-eslint\\parser\\dist\\parser.js',
  parserOptions: {
    ecmaVersion: 2018,
    sourceType: 'module',
    ecmaFeatures: { jsx: true },
    warnOnUnsupportedTypeScriptVersion: true
  },
  plugins: [
    'react-hooks',
    'react',// 
    'jsx-a11y',
    'flowtype',
    'import',
    '@typescript-eslint'
  ],
  rules: {
    '@typescript-eslint/no-unused-vars': [ 'off', [Object] ],
    'default-case': [ 'off', [Object] ],
    'no-dupe-class-members': [ 'off' ],
    'no-undef': [ 'off' ],
    '@typescript-eslint/no-angle-bracket-type-assertion': [ 'warn' ],

    // ... 

    'flowtype/require-valid-file-annotation': [ 'warn' ],
    'flowtype/use-flow-type': [ 'warn' ]
  },
  settings: { react: { version: 'detect' } }
}

As you can see, my override of @typescript-eslint/no-unused-vars was successful; but the config object no longer has an extends property, so this check

create-react-app/packages/react-scripts/config/webpack.config.js

Lines 352 to 356 in c0b4173

	if (
	process.env.EXTEND_ESLINT &&
	eslintConfig &&
	eslintConfig.extends &&
	eslintConfig.extends.includes('react-app')

will fail and we'll fall back on the standard eslint-config-react-app.

[HerbCaudill]

I'm guessing that prior to version 6, ESLint included the extends property in the config output. This code, which is new in ESLint 6, explicitly omits it.

https://github.com/eslint/eslint/blob/fbec99ea3e39316791685652c66e522d698f52d8/lib/cli-engine/config-array-factory.js#L559-L578

[ianschmitz]

@mrmckeb I'm seeing eslintCli.getConfigForFile(paths.appIndexJs) return undefined when i have a .eslintrc.json file.

[silverwind]

I'm not sure if the eslint API exposes a way to actually retrieve which extends a config uses. I did find a way to log out some potentially usable data inside eslint's getConfigForFile, but this functionality looks to not be exposed in the API.

console.log(configArrayFactory.getConfigArrayForFile(absolutePath).map(e => e.name));

Which logs this in my case:

ConfigArray [
  '.eslintrc » eslint-config-react-app',
  '.eslintrc » eslint-config-react-app#overrides[0]',
  '.eslintrc » eslint-config-silverwind',
  '.eslintrc',
  '.eslintrc#overrides[0]',
  '.eslintrc#overrides[1]'
]

Maybe cc: @not-an-aardvark

[mrmckeb]

Hi all, I'm currently on a business trip and will be on a plane for the next few hours.

It seems that this may have been broken by the ESLint 6 upgrade, which came after this config change - unfortunately we didn't have a test for that and we obviously needed one.

Maybe @not-an-aardvark can have a look, otherwise I can probably investigate this in the evening today (it'll be London time). Of course if anyone else can have a look and raise a PR, we'll be very grateful.

@ianschmitz, one option is to remove the requirement to extend our config - but that could introduce risk.

[conor-cafferkey-sociomantic]

How about checking for the extends in package.json? Would that work?

[n1ru4l]

It would also be nice to print something to the console instead of swallowing the error if the eslintCli.getConfigForFile(paths.appIndexJs); fails for some reason (other than the file not existing).

Temporary patch-package fix:

patches/react-scripts+3.1.0.patch

diff --git a/node_modules/react-scripts/config/webpack.config.js b/node_modules/react-scripts/config/webpack.config.js
index dad50a3..12fad03 100644
--- a/node_modules/react-scripts/config/webpack.config.js
+++ b/node_modules/react-scripts/config/webpack.config.js
@@ -339,22 +339,16 @@ module.exports = function(webpackEnv) {
                 resolvePluginsRelativeTo: __dirname,
                 // @remove-on-eject-begin
                 baseConfig: (() => {
-                  const eslintCli = new eslint.CLIEngine();
-                  let eslintConfig;
-                  try {
-                    eslintConfig = eslintCli.getConfigForFile(paths.appIndexJs);
-                  } catch (e) {
-                    // A config couldn't be found.
-                  }
+                  if (process.env.EXTEND_ESLINT) {
+                    const eslintCli = new eslint.CLIEngine();
+                    let eslintConfig;
+                    try {
+                      eslintConfig = eslintCli.getConfigForFile(paths.appIndexJs);
+                    } catch (e) {
+                      console.error(e);
+                      process.exit(1);
+                    }
 
-                  // We allow overriding the config, only if it extends our config
-                  // (`extends` can be a string or array of strings).
-                  if (
-                    process.env.EXTEND_ESLINT &&
-                    eslintConfig &&
-                    eslintConfig.extends &&
-                    eslintConfig.extends.includes('react-app')
-                  ) {
                     return eslintConfig;
                   } else {
                     return {

[mrmckeb]

Hi all, apologies for the botched release here - this was entirely my fault.

Unfortunately there was a change in ESLint 6 that broke this, as discussed, and I hadn't added a test for this before upgrading us to ESLint 6. Thanks to @ianschmitz and @iansu for getting a fix out while I was travelling.