feature #574 Proposal to replace #504 (ESLint/Vue) (Kocal)

This PR was squashed before being merged into the master branch.

Discussion
----------

Proposal to replace #504 (ESLint/Vue)

This PR add a second argument to `Encore.enableEslintLoader()` that is used to let the ESLint loader lint `.vue` files:

```js
Encore.enableEslintLoader(() => {}, {
    lintVue: true
});
```

Using `lintVue` won't add any ESLint configuration, that the job of the final user (see #504 (comment)).

**EDIT:**

While #657 is being discussed, you can use the following code to:
 - let ESLint process your `.vue` files
 - prevent the error `Use the latest vue-eslint-parser.`, see #656

```js
Encore.enableEslintLoader((options) => {
  delete options.parser;
}, {
  lintVue: true
});
```

**EDIT 2:**

PR #687 has been merged and issue #657 is now resolved. It means that you can use the following code to let eslint-loader handle `.vue` files:
```js
Encore.enableEslintLoader(() => {}, {
    lintVue: true
});
```

Commits
-------

13b0750 chore: remove comment for vue files linting since #687 has been merged
2f1e85b chore: add comment for making .vue files lint working
12b3f77 eslint/vue: add 2nd parameter to Encore#enableEslintLoader
eb85b24 eslint/vue: tweak config-generator
aa782df eslint/vue: implement `getTest()` on ESlint loader helper
bc444ff eslint/vue: tweak `WebpackConfig#enableEslintLoader()`

index.js

@@ -1192,13 +1192,27 @@ class Encore {
      *      options.extends = 'airbnb';
      *      options.emitWarning = false;
      * });
+     *
+     * // configure Encore-specific options
+     * Encore.enableEslintLoader(() => {}, {
+     *     // set optional Encore-specific options, for instance:
+     *
+     *     // lint `.vue` files
+     *     lintVue: true
+     * });
+     * ```
+     *
+     * Supported options:
+     *      * {boolean} lintVue (default=false)
+     *              Configure the loader to lint `.vue` files
      * ```
      *
      * @param {string|object|function} eslintLoaderOptionsOrCallback
+     * @param {object} encoreOptions
      * @returns {Encore}
      */
-    enableEslintLoader(eslintLoaderOptionsOrCallback = () => {}) {
-        webpackConfig.enableEslintLoader(eslintLoaderOptionsOrCallback);
+    enableEslintLoader(eslintLoaderOptionsOrCallback = () => {}, encoreOptions = {}) {
+        webpackConfig.enableEslintLoader(eslintLoaderOptionsOrCallback, encoreOptions);
 
         return this;
     }

lib/WebpackConfig.js

@@ -137,6 +137,9 @@ class WebpackConfig {
         this.vueOptions = {
             useJsx: false,
         };
+        this.eslintOptions = {
+            lintVue: false,
+        };
 
         // Features/Loaders options callbacks
         this.postCssLoaderOptionsCallback = () => {};
@@ -751,31 +754,33 @@ class WebpackConfig {
         this.vueOptions = vueOptions;
     }
 
-    enableEslintLoader(eslintLoaderOptionsOrCallback = () => {}) {
+    enableEslintLoader(eslintLoaderOptionsOrCallback = () => {}, eslintOptions = {}) {
         this.useEslintLoader = true;
 
         if (typeof eslintLoaderOptionsOrCallback === 'function') {
             this.eslintLoaderOptionsCallback = eslintLoaderOptionsOrCallback;
-            return;
-        }
-
-        if (typeof eslintLoaderOptionsOrCallback === 'string') {
+        } else if (typeof eslintLoaderOptionsOrCallback === 'string') {
             logger.deprecation('enableEslintLoader: Extending from a configuration is deprecated, please use a configuration file instead. See https://eslint.org/docs/user-guide/configuring for more information.');
 
             this.eslintLoaderOptionsCallback = (options) => {
                 options.extends = eslintLoaderOptionsOrCallback;
             };
-            return;
-        }
-
-        if (typeof eslintLoaderOptionsOrCallback === 'object') {
+        } else if (typeof eslintLoaderOptionsOrCallback === 'object') {
             this.eslintLoaderOptionsCallback = (options) => {
                 Object.assign(options, eslintLoaderOptionsOrCallback);
             };
-            return;
+        } else {
+            throw new Error('Argument 1 to enableEslintLoader() must be either a string, object or callback function.');
+        }
+
+        // Check allowed keys
+        for (const key of Object.keys(eslintOptions)) {
+            if (!(key in this.eslintOptions)) {
+                throw new Error(`"${key}" is not a valid key for enableEslintLoader(). Valid keys: ${Object.keys(this.eslintOptions).join(', ')}.`);
+            }
         }
 
-        throw new Error('Argument 1 to enableEslintLoader() must be either a string, object or callback function.');
+        this.eslintOptions = eslintOptions;
     }
 
     enableBuildNotifications(enabled = true, notifierPluginOptionsCallback = () => {}) {

lib/config-generator.js

@@ -393,7 +393,7 @@ class ConfigGenerator {
 
         if (this.webpackConfig.useEslintLoader) {
             rules.push(applyRuleConfigurationCallback('eslint', {
-                test: /\.jsx?$/,
+                test: eslintLoaderUtil.getTest(this.webpackConfig),
                 loader: 'eslint-loader',
                 exclude: /node_modules/,
                 enforce: 'pre',

lib/loaders/eslint.js

@@ -69,5 +69,19 @@ Install ${chalk.yellow('babel-eslint')} to prevent potential parsing issues: ${p
         };
 
         return applyOptionsCallback(webpackConfig.eslintLoaderOptionsCallback, eslintLoaderOptions);
+    },
+
+    /**
+     * @param {WebpackConfig} webpackConfig
+     * @return {RegExp} to use for eslint-loader `test` rule
+     */
+    getTest(webpackConfig) {
+        const extensions = ['jsx?'];
+
+        if (webpackConfig.eslintOptions.lintVue) {
+            extensions.push('vue');
+        }
+
+        return new RegExp(`\\.(${extensions.join('|')})$`);
     }
 };

test/WebpackConfig.js

@@ -1395,4 +1395,16 @@ describe('WebpackConfig object', () => {
             expect(() => config.enableIntegrityHashes(true, ['sha1', 'foo', 'sha256'])).to.throw('Invalid hash algorithm "foo"');
         });
     });
+
+    describe('enableEslintLoader', () => {
+        it('Should validate Encore-specific options', () => {
+            const config = createConfig();
+
+            expect(() => {
+                config.enableEslintLoader(() => {}, {
+                    notExisting: false,
+                });
+            }).to.throw('"notExisting" is not a valid key for enableEslintLoader(). Valid keys: lintVue.');
+        });
+    });
 });

test/config-generator.js

@@ -416,6 +416,22 @@ describe('The config-generator function', () => {
             expect(JSON.stringify(actualConfig.module.rules)).to.contain('eslint-loader');
             expect(JSON.stringify(actualConfig.module.rules)).to.contain('extends-name');
         });
+
+        it('enableEslintLoader(() => {}, {lintVue: true})', () => {
+            const config = createConfig();
+            config.addEntry('main', './main');
+            config.publicPath = '/';
+            config.outputPath = '/tmp';
+            config.enableEslintLoader(() => {}, {
+                lintVue: true,
+            });
+
+            const actualConfig = configGenerator(config);
+            expect(JSON.stringify(actualConfig.module.rules)).to.contain('eslint-loader');
+
+            const eslintRule = findRule(/\.(jsx?|vue)$/, actualConfig.module.rules);
+            expect(eslintRule.test.toString()).to.equal(/\.(jsx?|vue)$/.toString());
+        });
     });
 
     describe('addLoader() adds a custom loader', () => {

test/loaders/eslint.js

@@ -74,4 +74,21 @@ describe('loaders/eslint', () => {
         const actualOptions = eslintLoader.getOptions(config);
         expect(actualOptions).to.deep.equals({ foo: true });
     });
+
+    it('getTest() base behavior', () => {
+        const config = createConfig();
+
+        const actualTest = eslintLoader.getTest(config);
+        expect(actualTest.toString()).to.equals(/\.(jsx?)$/.toString());
+    });
+
+    it('getTest() with Vue', () => {
+        const config = createConfig();
+        config.enableEslintLoader(() => {}, {
+            lintVue: true,
+        });
+
+        const actualTest = eslintLoader.getTest(config);
+        expect(actualTest.toString()).to.equals(/\.(jsx?|vue)$/.toString());
+    });
 });