eslintの導入

[syunto07ka]

前回issue(そんなに書いてないが)

1

公式document

https://eslint.org/

[syunto07ka]

参考

https://eslint.org/docs/user-guide/getting-started

[syunto07ka]

1. npm install eslint --save-dev

2. npx eslint --init

kakiishuntsutomunoMacBook-Pro:jewelry_box syunto$ npx eslint --init
? How would you like to use ESLint? To check syntax and find problems
? What type of modules does your project use? JavaScript modules (import/export)
? Which framework does your project use? React
? Does your project use TypeScript? Yes
? Where does your code run? Browser
? What format do you want your config file to be in? JSON
The config that you've selected requires the following dependencies:

eslint-plugin-react@latest @typescript-eslint/eslint-plugin@latest @typescript-eslint/parser@latest
? Would you like to install them now with npm? Yes
Installing eslint-plugin-react@latest, @typescript-eslint/eslint-plugin@latest, @typescript-eslint/parser@latest
+ eslint-plugin-react@7.19.0
+ @typescript-eslint/parser@2.25.0
+ @typescript-eslint/eslint-plugin@2.25.0
updated 3 packages and audited 931560 packages in 12.445s

50 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

Successfully created .eslintrc.json file in /Users/syunto/development/jewelry_box
kakiishuntsutomunoMacBook-Pro:jewelry_box syunto$

[syunto07ka]

kakiishuntsutomunoMacBook-Pro:jewelry_box syunto$ npx eslint src/App.tsx
Warning: React version not specified in eslint-plugin-react settings. See https://github.com/yannickcr/eslint-plugin-react#configuration .
kakiishuntsutomunoMacBook-Pro:jewelry_box syunto$

こんなエラー

[syunto07ka]

これで解消

"settings": {
    "react": {
        "version": "detect"
    }
}

https://github.com/yannickcr/eslint-plugin-react#configuration

明示的に設定しとかないと反応しないのか。。

[syunto07ka]

動いてはいる

"rules": {
    "indent": ["error", 2]
},

kakiishuntsutomunoMacBook-Pro:jewelry_box syunto$ npx eslint src/App.tsx

/Users/syunto/development/jewelry_box/src/App.tsx
  21:1  error  Expected indentation of 6 spaces but found 4  indent

✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

kakiishuntsutomunoMacBook-Pro:jewelry_box syunto$

[syunto07ka]

package.json の

"eslintConfig": {
    "extends": "react-app"
},

は消さなくても良い？

[piro0919]

@Syunto07ka

package.json の

"eslintConfig": {
    "extends": "react-app"
},

は消さなくても良い？

まずはこの意味から調べるべきかとー。 👍

[syunto07ka]

Configuration Files - use a JavaScript, JSON or YAML file to specify configuration information for an entire directory and all of its subdirectories. This can be in the form of an .eslintrc.* file or an eslintConfig field in a package.json file, both of which ESLint will look for and read automatically, or you can specify a configuration file on the command line.

https://eslint.org/docs/user-guide/configuring

どちらを設定しても同じ、っていうのはこの前理解した 👍 が、どちらとも書いていたらどちらが優先されるんだろう。今両方とも書いてるけど、.eslintrcの方は動いてるっぽい。

[syunto07ka]

へー、マージされてるんだ。 それで、eslintrcがpackage.jsonの設定を上書くと。(stylelintのissueだけど)

https://github.com/stylelint/stylelint/issues/442#issuecomment-150827616

[syunto07ka]

http://eslint.org/docs/user-guide/configuring#configuration-cascading-and-hierarchy

[syunto07ka]

If there is an .eslintrc and a package.json file found in the same directory, .eslintrc will take a priority and package.json file will not be used.

ちゃう、同じディレクトリにあったらpackage.jsonの方は使われないって書いてある。 ので、pakage.jsonで記述されているeslintConfigは必要ない。これが答え

[syunto07ka]

各階層ごとに.eslintrcを置いて、その階層専用のルールを作ることができる、のか

[piro0919]

各階層ごとに.eslintrcを置いて、その階層専用のルールを作ることができる、のか

要するに、モノレポにも対応しているってことですね。

ちゃう、同じディレクトリにあったらpackage.jsonの方は使われないって書いてある。 ので、pakage.jsonで記述されているeslintConfigは必要ない。これが答え

どちらか一方の設定が使われるということは、果たしてどちらに書くべきなのか？という問題にあたります。

各人によって考え方が分かれる部分ですが、自分はpackage.jsonに書く派です。 理由としては、ルートディレクトリはファイルがやたら置かれがちなので、プロジェクトをスッキリさせたいからです。 👍

加えて、普段からpackage.jsonに書いておくようにすると、別ファイルで切り出した場合、別ファイルで切らなければいけない理由があるんだなとパッと見で推察できるようになるのもメリットだと思っています。

ちなみにこれはeslintに限らないと思っています。

[syunto07ka]

どちらか一方の設定が使われるということは、果たしてどちらに書くべきなのか？という問題にあたります。

これはたしかに。決めつけすぎてた。。

各人によって考え方が分かれる部分ですが、自分はpackage.jsonに書く派です。 理由としては、ルートディレクトリはファイルがやたら置かれがちなので、プロジェクトをスッキリさせたいからです。 👍

なるほど👀 個人的には、役割を分割させておきたい考えでファイルを切って書いておきたいです。(ディレクトリ深ぼってeslintのルールを追加したい場合は、eslintrcに書くと思うので、統一をさせるためもある)

まあ、深ぼって設定したくなるときとか、あんまりないんでしょうけど笑

加えて、普段からpackage.jsonに書いておくようにすると、別ファイルで切り出した場合、別ファイルで切らなければいけない理由があるんだなとパッと見で推察できるようになるのもメリットだと思っています。

あーこの考えはなかった。確かに。

[piro0919]

個人的には、役割を分割させておきたい考えでファイルを切って書いておきたいです。

統一性が保たれれば問題ないと思います。 eslintは別ファイルに、babelはpackage.jsonに、などは保守性の観点からNGかと。

[syunto07ka]

eslintは別ファイルに、babelはpackage.jsonに、などは保守性の観点からNGかと。

なるほど👀 ひとまず一緒にしてやってみます。分ける必要があれば途中で分けるでもよいですね 👍

[syunto07ka]

https://github.com/Syunto07ka/jewelry_box/issues/19#issuecomment-607857816

[syunto07ka]

eslint-config-react-app/index.js の中身 見る限りだとwarningレベルでルールを設定している、が大体のルールはここで設定されていそう👀

/**
 * Copyright (c) 2015-present, Facebook, Inc.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

'use strict';

// Inspired by https://github.com/airbnb/javascript but less opinionated.

// We use eslint-loader so even warnings are very visible.
// This is why we only use "WARNING" level for potential errors,
// and we don't use "ERROR" level at all.

// In the future, we might create a separate list of rules for production.
// It would probably be more strict.

// The ESLint browser environment defines all browser globals as valid,
// even though most people don't know some of them exist (e.g. `name` or `status`).
// This is dangerous as it hides accidentally undefined variables.
// We blacklist the globals that we deem potentially confusing.
// To use them, explicitly reference them, e.g. `window.name` or `window.status`.
const restrictedGlobals = require('confusing-browser-globals');

module.exports = {
  root: true,

  parser: 'babel-eslint',

  plugins: ['import', 'flowtype', 'jsx-a11y', 'react', 'react-hooks'],

  env: {
    browser: true,
    commonjs: true,
    es6: true,
    jest: true,
    node: true,
  },

  parserOptions: {
    ecmaVersion: 2018,
    sourceType: 'module',
    ecmaFeatures: {
      jsx: true,
    },
  },

  settings: {
    react: {
      version: 'detect',
    },
  },

  overrides: [
    {
      files: ['**/*.ts?(x)'],
      parser: '@typescript-eslint/parser',
      parserOptions: {
        ecmaVersion: 2018,
        sourceType: 'module',
        ecmaFeatures: {
          jsx: true,
        },

        // typescript-eslint specific options
        warnOnUnsupportedTypeScriptVersion: true,
      },
      plugins: ['@typescript-eslint'],
      // If adding a typescript-eslint version of an existing ESLint rule,
      // make sure to disable the ESLint rule here.
      rules: {
        // TypeScript's `noFallthroughCasesInSwitch` option is more robust (#6906)
        'default-case': 'off',
        // 'tsc' already handles this (https://github.com/typescript-eslint/typescript-eslint/issues/291)
        'no-dupe-class-members': 'off',
        // 'tsc' already handles this (https://github.com/typescript-eslint/typescript-eslint/issues/477)
        'no-undef': 'off',

        // Add TypeScript specific rules (and turn off ESLint equivalents)
        '@typescript-eslint/consistent-type-assertions': 'warn',
        'no-array-constructor': 'off',
        '@typescript-eslint/no-array-constructor': 'warn',
        'no-use-before-define': 'off',
        '@typescript-eslint/no-use-before-define': [
          'warn',
          {
            functions: false,
            classes: false,
            variables: false,
            typedefs: false,
          },
        ],
        'no-unused-expressions': 'off',
        '@typescript-eslint/no-unused-expressions': [
          'error',
          {
            allowShortCircuit: true,
            allowTernary: true,
            allowTaggedTemplates: true,
          },
        ],
        'no-unused-vars': 'off',
        '@typescript-eslint/no-unused-vars': [
          'warn',
          {
            args: 'none',
            ignoreRestSiblings: true,
          },
        ],
        'no-useless-constructor': 'off',
        '@typescript-eslint/no-useless-constructor': 'warn',
      },
    },
  ],

  // NOTE: When adding rules here, you need to make sure they are compatible with
  // `typescript-eslint`, as some rules such as `no-array-constructor` aren't compatible.
  rules: {
    // http://eslint.org/docs/rules/
    'array-callback-return': 'warn',
    'default-case': ['warn', { commentPattern: '^no default$' }],
    'dot-location': ['warn', 'property'],
    eqeqeq: ['warn', 'smart'],
    'new-parens': 'warn',
    'no-array-constructor': 'warn',
    'no-caller': 'warn',
    'no-cond-assign': ['warn', 'except-parens'],
    'no-const-assign': 'warn',
    'no-control-regex': 'warn',
    'no-delete-var': 'warn',
    'no-dupe-args': 'warn',
    'no-dupe-class-members': 'warn',
    'no-dupe-keys': 'warn',
    'no-duplicate-case': 'warn',
    'no-empty-character-class': 'warn',
    'no-empty-pattern': 'warn',
    'no-eval': 'warn',
    'no-ex-assign': 'warn',
    'no-extend-native': 'warn',
    'no-extra-bind': 'warn',
    'no-extra-label': 'warn',
    'no-fallthrough': 'warn',
    'no-func-assign': 'warn',
    'no-implied-eval': 'warn',
    'no-invalid-regexp': 'warn',
    'no-iterator': 'warn',
    'no-label-var': 'warn',
    'no-labels': ['warn', { allowLoop: true, allowSwitch: false }],
    'no-lone-blocks': 'warn',
    'no-loop-func': 'warn',
    'no-mixed-operators': [
      'warn',
      {
        groups: [
          ['&', '|', '^', '~', '<<', '>>', '>>>'],
          ['==', '!=', '===', '!==', '>', '>=', '<', '<='],
          ['&&', '||'],
          ['in', 'instanceof'],
        ],
        allowSamePrecedence: false,
      },
    ],
    'no-multi-str': 'warn',
    'no-native-reassign': 'warn',
    'no-negated-in-lhs': 'warn',
    'no-new-func': 'warn',
    'no-new-object': 'warn',
    'no-new-symbol': 'warn',
    'no-new-wrappers': 'warn',
    'no-obj-calls': 'warn',
    'no-octal': 'warn',
    'no-octal-escape': 'warn',
    // TODO: Remove this option in the next major release of CRA.
    // https://eslint.org/docs/user-guide/migrating-to-6.0.0#-the-no-redeclare-rule-is-now-more-strict-by-default
    'no-redeclare': ['warn', { builtinGlobals: false }],
    'no-regex-spaces': 'warn',
    'no-restricted-syntax': ['warn', 'WithStatement'],
    'no-script-url': 'warn',
    'no-self-assign': 'warn',
    'no-self-compare': 'warn',
    'no-sequences': 'warn',
    'no-shadow-restricted-names': 'warn',
    'no-sparse-arrays': 'warn',
    'no-template-curly-in-string': 'warn',
    'no-this-before-super': 'warn',
    'no-throw-literal': 'warn',
    'no-undef': 'error',
    'no-restricted-globals': ['error'].concat(restrictedGlobals),
    'no-unreachable': 'warn',
    'no-unused-expressions': [
      'error',
      {
        allowShortCircuit: true,
        allowTernary: true,
        allowTaggedTemplates: true,
      },
    ],
    'no-unused-labels': 'warn',
    'no-unused-vars': [
      'warn',
      {
        args: 'none',
        ignoreRestSiblings: true,
      },
    ],
    'no-use-before-define': [
      'warn',
      {
        functions: false,
        classes: false,
        variables: false,
      },
    ],
    'no-useless-computed-key': 'warn',
    'no-useless-concat': 'warn',
    'no-useless-constructor': 'warn',
    'no-useless-escape': 'warn',
    'no-useless-rename': [
      'warn',
      {
        ignoreDestructuring: false,
        ignoreImport: false,
        ignoreExport: false,
      },
    ],
    'no-with': 'warn',
    'no-whitespace-before-property': 'warn',
    'react-hooks/exhaustive-deps': 'warn',
    'require-yield': 'warn',
    'rest-spread-spacing': ['warn', 'never'],
    strict: ['warn', 'never'],
    'unicode-bom': ['warn', 'never'],
    'use-isnan': 'warn',
    'valid-typeof': 'warn',
    'no-restricted-properties': [
      'error',
      {
        object: 'require',
        property: 'ensure',
        message:
          'Please use import() instead. More info: https://facebook.github.io/create-react-app/docs/code-splitting',
      },
      {
        object: 'System',
        property: 'import',
        message:
          'Please use import() instead. More info: https://facebook.github.io/create-react-app/docs/code-splitting',
      },
    ],
    'getter-return': 'warn',

    // https://github.com/benmosher/eslint-plugin-import/tree/master/docs/rules
    'import/first': 'error',
    'import/no-amd': 'error',
    'import/no-webpack-loader-syntax': 'error',

    // https://github.com/yannickcr/eslint-plugin-react/tree/master/docs/rules
    'react/forbid-foreign-prop-types': ['warn', { allowInPropTypes: true }],
    'react/jsx-no-comment-textnodes': 'warn',
    'react/jsx-no-duplicate-props': 'warn',
    'react/jsx-no-target-blank': 'warn',
    'react/jsx-no-undef': 'error',
    'react/jsx-pascal-case': [
      'warn',
      {
        allowAllCaps: true,
        ignore: [],
      },
    ],
    'react/jsx-uses-react': 'warn',
    'react/jsx-uses-vars': 'warn',
    'react/no-danger-with-children': 'warn',
    // Disabled because of undesirable warnings
    // See https://github.com/facebook/create-react-app/issues/5204 for
    // blockers until its re-enabled
    // 'react/no-deprecated': 'warn',
    'react/no-direct-mutation-state': 'warn',
    'react/no-is-mounted': 'warn',
    'react/no-typos': 'error',
    'react/react-in-jsx-scope': 'error',
    'react/require-render-return': 'error',
    'react/style-prop-object': 'warn',

    // https://github.com/evcohen/eslint-plugin-jsx-a11y/tree/master/docs/rules
    'jsx-a11y/accessible-emoji': 'warn',
    'jsx-a11y/alt-text': 'warn',
    'jsx-a11y/anchor-has-content': 'warn',
    'jsx-a11y/anchor-is-valid': [
      'warn',
      {
        aspects: ['noHref', 'invalidHref'],
      },
    ],
    'jsx-a11y/aria-activedescendant-has-tabindex': 'warn',
    'jsx-a11y/aria-props': 'warn',
    'jsx-a11y/aria-proptypes': 'warn',
    'jsx-a11y/aria-role': ['warn', { ignoreNonDOM: true }],
    'jsx-a11y/aria-unsupported-elements': 'warn',
    'jsx-a11y/heading-has-content': 'warn',
    'jsx-a11y/iframe-has-title': 'warn',
    'jsx-a11y/img-redundant-alt': 'warn',
    'jsx-a11y/no-access-key': 'warn',
    'jsx-a11y/no-distracting-elements': 'warn',
    'jsx-a11y/no-redundant-roles': 'warn',
    'jsx-a11y/role-has-required-aria-props': 'warn',
    'jsx-a11y/role-supports-aria-props': 'warn',
    'jsx-a11y/scope': 'warn',

    // https://github.com/facebook/react/tree/master/packages/eslint-plugin-react-hooks
    'react-hooks/rules-of-hooks': 'error',

    // https://github.com/gajus/eslint-plugin-flowtype
    'flowtype/define-flow-type': 'warn',
    'flowtype/require-valid-file-annotation': 'warn',
    'flowtype/use-flow-type': 'warn',
  },
};

[syunto07ka]

@piro0919 "react-app"のみの設定でもよいですか？👀 ベストな設定値がわからず。。

[piro0919]

@Syunto07ka 難しいところですね、自分もベストプラクティスには至っていないので。 ただ、eslint-config-react-appだけでは決して強い設定とは言えないので、そこは都度強めていくべきだとは思います。

このissueは導入が目的なので、一旦CRAのデフォルトでいくのはアリだとは思います。

rel https://github.com/Syunto07ka/jewelry_box/issues/41

[syunto07ka]

続きは #41 にて行うのでこのissueはcloseする