4

I have created an app with create-react-app and I have applied the airbnb rules. The app also contains redux and flow.

The following code is throwing the no-unused-expressions error in eslint:

const reducer = (state: string = '', action: Action) => {
    switch (action.type) {
        // cases
        default:
            (action: empty); // this is throwing eslint/no-unused-expressions
            return state;
    }
};

I am trying to switch off the rule in eslintrc in order to replace it with flowtype/no-unused-expressions

content of my .eslintrc.yml

extends:
    - airbnb
parser: babel-eslint
env:
    browser: true
    jest: true
globals:
    SyntheticEvent: true,
rules:
    no-unused-expressions: off
    react/prefer-stateless-function: off
    react/jsx-filename-extension: off
    react/jsx-one-expression-per-line: off

With this settings, the no-unused-expressions error is not shown anymore in the editor (vscode). However as soon as I compile with npm run start the error is still there:

Expected an assignment or function call and instead saw an expression  no-unused-expressions

causing it to fail to compile.

Of course if I disable locally eslint for that rule for example with

// eslint-disable-line no-unused-expressions

Everything is working in both the editor and the browser. However, as I said I am trying to replace the eslint rule with the flowtype one exactly to avoid to be obliged to disable eslint every time I am using a flow type assertion.

Any idea what I am doing wrong?

package.json content:

{
  "name": "would-you-rather",
  "version": "0.1.0",
  "private": true,
  "dependencies": {
    "eslint-config-airbnb": "17.1.0",
    "eslint-config-flowtype-essential": "^1.0.0",
    "eslint-plugin-flowtype": "^3.2.0",
    "flow-bin": "0.89.0",
    "flow-typed": "2.5.1",
    "immutable": "4.0.0-rc.12",
    "prop-types": "15.6.2",
    "react": "16.6.3",
    "react-dom": "16.6.3",
    "react-icons": "3.2.2",
    "react-redux": "6.0.0",
    "react-redux-loading-bar": "4.1.0",
    "react-router-dom": "4.3.1",
    "react-scripts": "2.1.1",
    "redux": "4.0.1",
    "redux-immutable": "4.0.0",
    "redux-thunk": "2.3.0", 
    "semantic-ui-css": "2.4.1",
    "semantic-ui-react": "0.84.0"
  },
  "devDependencies": {
    "docdash": "1.0.1",
    "jsdoc": "3.5.5"
  },
  "scripts": {
    "start": "react-scripts start",
    "build": "react-scripts build",
    "test": "react-scripts test",
    "eject": "react-scripts eject",
    "jsdoc": "jsdoc --configure jsdoc.conf.json --recurse --private",
    "flow": "$(npm bin)/flow",
    "flow-typed": "$(npm bin)/flow-typed",
    "postinstall": "$(npm bin)/flow-typed install"
  },
  "browserslist": [
    ">0.2%",
    "not dead",
    "not ie <= 11",
    "not op_mini all"
  ]
}

Link of the project on github if you want to play with it

skyboyer
  • 22,209
  • 7
  • 57
  • 64
Luca Borrione
  • 16,324
  • 8
  • 52
  • 66
  • I'm really curious as to the purpose of that `(action: empty)` line. It does appear to be an unused expression. That cast will not change the type of subsequent references to `action` in that block. And it should not be possible to cast the type of a variable to `empty`. Are you trying to force a type error in the `default` case? – Jesse Hallett Jan 02 '19 at 20:20
  • 1
    @JesseHallett Hey, thanks for asking. Using the empty type assertion in the default case validates that every single type of action have been handled as per [flow documentation](https://flow.org/en/docs/react/redux/#toc-typing-redux-reducers). There's a [flowtype no-unused-expressions rule](https://github.com/gajus/eslint-plugin-flowtype/blob/master/.README/rules/no-unused-expressions.md) which ignores type assertion but my understanding is that it requires to switch off the eslint no-unused-expressions rule first, in order to replace it. And I seem unable to switch the latter off. – Luca Borrione Jan 03 '19 at 09:41
  • Can you please show your package.json (specifically the scripts section)? It seems that somehow, ESLint is run differently during `npm run start` vs in your editor. – Platinum Azure Jan 03 '19 at 14:23
  • @LucaBorrione Oh neat! Thanks for the explanation! – Jesse Hallett Jan 03 '19 at 20:53
  • @PlatinumAzure I added the content of my package.json to my question – Luca Borrione Jan 03 '19 at 22:00

3 Answers3

2

The scripts included with react-scripts specifically do not read overrides from eslintrc files. The reasoning is explained in an issue comment:

I don't think it would be a good solution. We aggressively show lint violations (in browser for errors, in console for warnings), and so we didn't include any style rules in the config.

I think style rules should be handled completely separately, before you commit. They shouldn't distract you during development or be loud in browser or terminal.

I think the idea is that you are free to use your own eslint configuration to add style rules specific to your project that you check during development; but the build and start will not look at it, and will stick to the conservative rule set bundled with react-scripts instead. The fact that you have found a case where those conservative rules interfere with your workflow probably deserves an issue report with create-react-app.

I think that the easiest solution is to use the // eslint-disable-line no-unused-expressions line, as you mentioned. But there are a couple of other options. You can modify the expression to convince eslint that it is not unused, or you can use a tool like patch-package to modify react-scripts' Webpack configuration so that it reads your custom eslint configuration.

Convince eslint that the expression is used

The eslint configuration that react-scripts uses is in node_modules/eslint-config-react-app/index.js. You can see that it sets some exceptions to the no-unused-expressions rule:

'no-unused-expressions': [
  'error',
  {
    allowShortCircuit: true,
    allowTernary: true,
    allowTaggedTemplates: true,
  },
],

Ternary expressions are allowed. You can combine the type assertion with a function call (which should never run because action should always be truthy):

(action: empty) || noop();

Patch react-scripts' Webpack configuration

You can see the code that react-scripts uses to run eslint in node_modules/react-scripts/config/webpack.config.dev.js and again in node_modules/react-scripts/config/webpack.config.dev.js:

// First, run the linter.
// It's important to do this before Babel processes the JS.
{
  test: /\.(js|mjs|jsx)$/,
  enforce: 'pre',
  use: [
    {
      options: {
        formatter: require.resolve('react-dev-utils/eslintFormatter'),
        eslintPath: require.resolve('eslint'),
        // @remove-on-eject-begin
        baseConfig: {
          extends: [require.resolve('eslint-config-react-app')],
          settings: { react: { version: '999.999.999' } },
        },
        ignore: false,
        useEslintrc: false,
        // @remove-on-eject-end
      },
      loader: require.resolve('eslint-loader'),
    },
  ],
  include: paths.appSrc,
},

To use your custom configuration you need to change the line useEslintrc: false to useEslintrc: true in both files. Then use patch-package to automatically reapply that change whenever react-scripts is installed or updated. Add this script to the scripts section in package.json:

"scripts": {
  "prepare": "patch-package"
}

Install patch-package, and postinstall-prepare to make sure that yarn runs the that prepare script:

$ yarn add --dev patch-package postinstall-prepare

After editing the Webpack configuration files run this command to save a patch (note that the yarn commands above will have reverted your changes, so make the same changes again before running this step):

$ yarn patch-package react-scripts

That will create a file with a name like patches/react-scripts+2.1.1.patch. You should check this file into version control.

Community
  • 1
  • 1
Jesse Hallett
  • 1,857
  • 17
  • 26
  • Hey thanks for your big effort! I saw the discussion on the [issue](https://github.com/facebook/create-react-app/issues/808#issuecomment-252928970) you mentioned today as well before reading your answer and I am a little bit surprised about this decision honestly. I raised an [issue](https://github.com/facebook/create-react-app/issues/6121) in create-react-app let's see if anyone will suggest something – Luca Borrione Jan 03 '19 at 23:52
  • I tried both your suggestions but 1. (action: empty) || noop(); still throws the same error (I checked node_modules/eslint-config-react-app/index.js and it's the same you posted) 2. I followed your instructions but when I ran yarn patch-package react-scripts it failed with error Error: Unable to find a flow project in the current dir or any of it's parent dirs! Please run this command from within a Flow project. which is strange as I do have a .flowconfig file. – Luca Borrione Jan 03 '19 at 23:52
  • @LucaBorrione That Flow error is strange. Do you have a script in package.json that runs Flow when packages are installed? Perhaps a postinstall, or a prepare script? If so you might try temporarily disabling it while you create the patch. And are you using create-react-app v2 or v1? – Jesse Hallett Jan 04 '19 at 15:17
  • 1
    Yep, you were right: I temporarily disabled the postinstall script, generated the patch, enabled again the postinstall script, removed the node_modules folder and ran npm install again .. it applied the patch and everything is now working! However I strongly believe that this should be done by CRA directly just switching useEslintrc to true in order to allow to load a custom eslintrc instead of obliging us to patch it. I really can't see the reason why they want to impose their styling rules. – Luca Borrione Jan 05 '19 at 12:34
0

As Jesse pointed out in his answer it looks like ignoring .eslintrc is something done intentionally by the react scripts. I have been able to achieve my goal by ejecting and removing the eslintConfig section added by the reject script to the package.json .. however I feel that it would be better to disable eslint inline and avoid ejecting

Luca Borrione
  • 16,324
  • 8
  • 52
  • 66
0

This resolved it for me for react-scripts:4.0.1

  1. Remove the eslintConfig from your package.json file. It should be under your scripts key value.

  2. Now, just add an .eslintrc.js in the root of your project with this configuration in it:

module.exports = {
  extends: ["react-app", "react-app/jest"],
  overrides: [
    {
      "files": ["**/*.js?(x)"],
      "rules": {
          "no-unused-vars": [1, { "vars": "all", "args": "after-used", "ignoreRestSiblings": false }]
      }
    }
  ]
}

You can also do "no-unused-vars": [0] which will turn off linting for unused vars

  1. After adding this, you can restart your server. react-scripts will detect that you have an .eslintrc file in the root of your project and will override the defaults with those in that file.
uinstinct
  • 630
  • 1
  • 7
  • 14