webpack-contrib / eslint-loader

[DEPRECATED] A ESlint loader for webpack
MIT License
1.06k stars 121 forks source link

eslint-loader doesn't check if additional .eslintrc exists #129

Closed hinok closed 7 years ago

hinok commented 7 years ago

eslint gives ability to overrides rules for specific folders, for example:

- app/
- app/index.js
- app/.eslintrc // (a) Overrides rules from (b)
- index.js
- .eslintrc // (b)

In my project when I use eslint by eslint-cli I don't get any errors but when I develop and use webpack + eslint-loader, then I get errors.

ERROR in ./src/components/Icons/IconSearch.js

/a/b/c/e/f/g/h/src/components/Icons/IconSearch.js
  8:1  error  Line 8 exceeds the maximum line length of 100  max-len

✖ 1 problem (1 error, 0 warnings)

 @ ./src/views/home/index.js 13:0-61
 @ ./src/app/routes.js
 @ ./src/app/index.js
 @ ./src/index.js
 @ multi app

I don't want append

/* eslint-disable max-len */

in each file that contains SVG because I have a lot of icons... Is there any way to instrument this loader to check if additional .eslintrc files exists?

peternewnham commented 7 years ago

eslint-loader doesn't load the eslintrc files - it uses an instance of eslint.CLIEngine which the eslint-cli also uses and that should resolve the correct rules for each file itself. I've just tried nested .eslintrc files and it seemed to be working correctly for me.

Do you have the eslint-loader cache option enabled? If so then you may be experiencing the problem described in #124 where if you change the eslint rules the previously cached results are still being output. Try setting cache to false and if that fixes the issue you can delete node_modules/.cache/eslint-loader to reset the cache properly.

hinok commented 7 years ago

Do you have the eslint-loader cache option enabled?

Yes. If I disable cache, it doesn't change anything. I'm using webpack 2.1.0-beta.27 with this configuration:

{
    module: {
      loaders: [
        {
          enforce: 'pre',
          test: /\.js$/,
          loader: 'eslint-loader',
          exclude: /node_modules/,
        },
      ],
    },
    plugins: [
      new webpack.LoaderOptionsPlugin({
        test: /\.js$/,
        options: {
          eslint: {
            configFile: utils.root('.eslintrc'), // this is my helper for resolving paths
            cache: false,
          }
        },
      }),
    ],
  }

I'm using also eslint-config-airbnb but this probably is not related. I will try reproduce this issue and create an example repository that could show this bug.

hinok commented 7 years ago

Update: If I remove configFile property, it works - so weird


@wrakky https://github.com/hinok/webpack2-eslint-loader

npm install

# Run eslint by eslint-cli
npm run lint

# Run webpack2 + eslint-loader
npm start

Repository contains demo.gif added in README.md that shows recorded behaviour. Look at additional .eslintrc inside subfolder.

peternewnham commented 7 years ago

If you use the -c option in the npm run lint command (eslint . --ext .js -c .eslintrc) which is the equivalent to the eslint-loader configFile option you get the same output from both eslint cli and weback.

The file passed to theconfigFile option is not extendable which is why you are having the problem. You probably want to either remove the option if possible or maybe the baseConfig option will work for you instead.

Either way this isn't an eslint-loader issue. Hope that helps!

hinok commented 7 years ago

Yep, that's it. Sorry for the confussion! When I found yesterday that removing configFile solved the problem, I was pretty sure that let's say it's the eslint's fault not eslint-loader - if we could name it as a fault.

I close this issue and maybe it's worth to update README? I could add PR if you want.

MoOx commented 7 years ago

PR welcome!

hinok commented 7 years ago

Added PR https://github.com/MoOx/eslint-loader/pull/130

nmccready commented 6 years ago

I am having a similar problem, can you post your actual config files which resolved your issue?

configs:

nmccready commented 6 years ago

NVM:

https://github.com/webpack-contrib/eslint-loader/pull/183/files

Use eslintPath with the CLIEngine class wrapper .

Seems like defining CLIEngine is overkill. Why is this not wrapped within the code/eslint-loader library itself?

class CLIEngine {
  executeOnText() {
    return require(eslintPath);
  }
}
dualeoo commented 6 years ago

The problem can be fixed by adding the option configFile to webpack.module.rules.Rule

{
  test: /\.(js|vue)$/,
  loader: 'eslint-loader',
  enforce: 'pre',
  include: [resolve('src'), resolve('test')],
  options: {
    formatter: require('eslint-friendly-formatter'),
    emitWarning: !config.dev.showEslintErrorsInOverlay,
    configFile: '.eslintrc.js'
}
midori0507 commented 5 years ago

@dualeoo: I still got a same issue with latest eslint-loader and eslint, even when configFile is there

dualeoo commented 5 years ago

@midori0507 sr but

midori0507 commented 5 years ago

@dualeoo: Issues: Eslint-loader didn't show any error when build, but when you run node_modules/.bin/eslint it show multiple lint errors, Eslint-loader does work, just not 100%. Version: eslint-loader@2.1.2 eslint 5.14.0 Config:

module: {
    rules: [
      {
        enforce: "pre",
        test: /\.js$/,
        exclude: /node_modules/,
        options: {
           babelPath: require.resolve('eslint'),
           configFile: path.resolve(__dirname, '../.eslintrc'),
        },
        loader: "eslint-loader"
      },
      {
        test: /\.js$/,
        exclude: /node_modules/,
        loader: "babel-loader"
      }
    ]
  }
  // ...

.eslintrc

{
"extends": "airbnb",
"env": {
"browser": true,
"mocha": true,
"node": true
},
"globals": {
"SETTINGS": true
},
"parser": "babel-eslint",
"plugins": [
"import",
"react"
],
"settings": {
"import/resolver": {
"webpack": {
"config": "./config/webpack.common.js"
}
}
},
"rules": {
"class-methods-use-this": "off",
"import/no-extraneous-dependencies": "off",
"import/order": [
"error",
{
"groups": [
"builtin",
"external",
[
"internal",
"parent",
"sibling",
"index"
]
],
"newlines-between": "always"
}
],
"import/prefer-default-export": "off",
"new-cap": [
"error",
{
"capIsNew": false,
"newIsCap": true
}
],
"no-underscore-dangle": [
"error",
{
"allow": [
"_paq"
]
}
],
"no-unused-expressions": "off",
"no-use-before-define": [
"error",
"nofunc"
],
"react/forbid-prop-types": "off",
"react/jsx-filename-extension": "off",
"react/jsx-sort-props": [
"error",
{
"ignoreCase": true
}
],
"react/no-string-refs": "off",
"react/no-unused-prop-types": [
"error",
{
"skipShapeProps": true
}
],
"react/prefer-stateless-function": "off",
"linebreak-style": 0,
"no-restricted-globals": [
"error",
"history"
],
"prefer-destructuring": "off",
"react/destructuring-assignment": "off",
"react/require-default-props": "off",
"jsx-a11y/click-events-have-key-events": "off",
"jsx-a11y/anchor-is-valid": "off",
"jsx-a11y/label-has-for": "off",
"react/no-array-index-key": "off"
}
}
oceandrama commented 5 years ago

@midori0507 maybe webpack doesn't include files with error in the build. Is it realy used in code?

Venryx commented 4 years ago

In my case the issue was fixed by changing from the old webpack.module.rules.Rule.query key to webpack.module.rules.Rule.options.