Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ESLint user-related config #687

Merged
merged 5 commits into from
Mar 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions fixtures/js/eslint-es2018.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const a = { x: 0, y: 1, z: 2 };
const { x, ...b } = a;
6 changes: 1 addition & 5 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1070,18 +1070,14 @@ class Encore {
* // enables the eslint loaded using the default eslint configuration.
* Encore.enableEslintLoader();
*
* // Optionally, you can pass in the configuration eslint should extend.
* Encore.enableEslintLoader('airbnb');
*
* // You can also pass in an object of options
* // that will be passed on to the eslint-loader
* Encore.enableEslintLoader({
* extends: 'airbnb',
* emitWarning: false
* });
*
* // For a more advanced usage you can pass in a callback
* // https://github.com/MoOx/eslint-loader#options
* // https://github.com/webpack-contrib/eslint-loader#options
* Encore.enableEslintLoader((options) => {
* options.extends = 'airbnb';
* options.emitWarning = false;
Expand Down
2 changes: 2 additions & 0 deletions lib/WebpackConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,8 @@ class WebpackConfig {
}

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;
};
Expand Down
1 change: 0 additions & 1 deletion lib/features.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ const features = {
packages: [
{ name: 'eslint' },
{ name: 'eslint-loader', enforce_version: true },
{ name: 'babel-eslint', enforce_version: true },
],
description: 'Enable ESLint checks'
},
Expand Down
43 changes: 42 additions & 1 deletion lib/loaders/eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ const WebpackConfig = require('../WebpackConfig'); //eslint-disable-line no-unus
const loaderFeatures = require('../features');
const applyOptionsCallback = require('../utils/apply-options-callback');

function isMissingConfigError(e) {
if (!e.message || !e.message.includes('No ESLint configuration found')) {
return false;
}

return true;
}

module.exports = {
/**
* @param {WebpackConfig} webpackConfig
Expand All @@ -21,9 +29,42 @@ module.exports = {
getOptions(webpackConfig) {
loaderFeatures.ensurePackagesExistAndAreCorrectVersion('eslint');

const eslint = require('eslint'); // eslint-disable-line node/no-unpublished-require
const engine = new eslint.CLIEngine({
cwd: webpackConfig.runtimeConfig.context,
});

try {
engine.config.getConfigHierarchy(webpackConfig.runtimeConfig.context);
} catch (e) {
if (isMissingConfigError(e)) {
const chalk = require('chalk').default;
const packageHelper = require('../package-helper');

const message = `No ESLint configration has been found.

${chalk.bgGreen.black('', 'FIX', '')} Run command ${chalk.yellow('./node_modules/.bin/eslint --init')} or manually create a ${chalk.yellow('.eslintrc.js')} file at the root of your project.

If you prefer to create a ${chalk.yellow('.eslintrc.js')} file by yourself, here is an example to get you started:

${chalk.yellow(`// .eslintrc.js
module.exports = {
parser: 'babel-eslint',
extends: ['eslint:recommended'],
}
`)}

Install ${chalk.yellow('babel-eslint')} to prevent potential parsing issues: ${packageHelper.getInstallCommand([[{ name: 'babel-eslint' }]])}

`;
throw new Error(message);
}

throw e;
}

const eslintLoaderOptions = {
cache: true,
parser: 'babel-eslint',
emitWarning: true
};

Expand Down
1 change: 1 addition & 0 deletions lib/package-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,5 @@ module.exports = {
getMissingPackageRecommendations,
getInvalidPackageVersionRecommendations,
addPackagesVersionConstraint,
getInstallCommand,
};
5 changes: 5 additions & 0 deletions test/config-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,10 @@ describe('The config-generator function', () => {
});

it('enableEslintLoader("extends-name")', () => {
before(() => {
logger.reset();
});

const config = createConfig();
config.addEntry('main', './main');
config.publicPath = '/';
Expand All @@ -380,6 +384,7 @@ describe('The config-generator function', () => {

const actualConfig = configGenerator(config);

expect(JSON.stringify(logger.getMessages().deprecation)).to.contain('enableEslintLoader: Extending from a configuration is deprecated, please use a configuration file instead. See https://eslint.org/docs/user-guide/configuring for more information.');
expect(JSON.stringify(actualConfig.module.rules)).to.contain('eslint-loader');
expect(JSON.stringify(actualConfig.module.rules)).to.contain('extends-name');
});
Expand Down
68 changes: 68 additions & 0 deletions test/functional.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

'use strict';

const os = require('os');
const chai = require('chai');
chai.use(require('chai-fs'));
const expect = chai.expect;
Expand Down Expand Up @@ -1722,6 +1723,73 @@ module.exports = {
}, true);
});

it('When enabled, eslint checks for linting errors by using configuration from file', (done) => {
const cwd = process.cwd();
after(() => {
process.chdir(cwd);
});

const appDir = testSetup.createTestAppDir();
const config = testSetup.createWebpackConfig(appDir, 'www/build', 'dev');
config.setPublicPath('/build');
config.addEntry('main', './js/eslint-es2018');
config.enableEslintLoader({
// Force eslint-loader to output errors instead of sometimes
// using warnings (see: https://github.com/MoOx/eslint-loader#errors-and-warning)
emitError: true,
});
fs.writeFileSync(
path.join(appDir, '.eslintrc.js'),
`
module.exports = {
parser: 'babel-eslint',
rules: {
'indent': ['error', 2],
'no-unused-vars': ['error', { 'args': 'all' }]
}
} `
);

process.chdir(appDir);

testSetup.runWebpack(config, (webpackAssert, stats) => {
const eslintErrors = stats.toJson().errors[0];

expect(eslintErrors).not.to.contain('Parsing error: Unexpected token ..');
expect(eslintErrors).to.contain('Expected indentation of 0 spaces but found 2');
expect(eslintErrors).to.contain('\'x\' is assigned a value but never used');
expect(eslintErrors).to.contain('\'b\' is assigned a value but never used');

done();
}, true);
});

it('When enabled and without any configuration, ESLint will throw an error and a nice message should be displayed', (done) => {
const cwd = process.cwd();

this.timeout(5000);
setTimeout(() => {
process.chdir(cwd);
done();
}, 4000);

const appDir = testSetup.createTestAppDir(os.tmpdir()); // to prevent issue with Encore's .eslintrc.js
const config = testSetup.createWebpackConfig(appDir, 'www/build', 'dev');
config.setPublicPath('/build');
config.addEntry('main', './js/eslint');
config.enableEslintLoader({
// Force eslint-loader to output errors instead of sometimes
// using warnings (see: https://github.com/MoOx/eslint-loader#errors-and-warning)
emitError: true,
});

process.chdir(appDir);

expect(() => {
testSetup.runWebpack(config, (webpackAssert, stats) => {});
}).to.throw('No ESLint configration has been found.');
});

it('Code splitting with dynamic import', (done) => {
const config = createWebpackConfig('www/build', 'dev');
config.setPublicPath('/build');
Expand Down
4 changes: 2 additions & 2 deletions test/helpers/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ const testFixturesDir = path.join(__dirname, '../', '../', 'fixtures');

let servers = [];

function createTestAppDir() {
const testAppDir = path.join(tmpDir, Math.random().toString(36).substring(7));
function createTestAppDir(rootDir = tmpDir) {
const testAppDir = path.join(rootDir, Math.random().toString(36).substring(7));

// copy the fixtures into this new directory
fs.copySync(testFixturesDir, testAppDir);
Expand Down
3 changes: 0 additions & 3 deletions test/loaders/eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ describe('loaders/eslint', () => {

expect(actualOptions).to.deep.equal({
cache: true,
parser: 'babel-eslint',
emitWarning: true
});
});
Expand All @@ -45,7 +44,6 @@ describe('loaders/eslint', () => {

expect(actualOptions).to.deep.equal({
cache: true,
parser: 'babel-eslint',
emitWarning: true,
extends: 'airbnb'
});
Expand All @@ -61,7 +59,6 @@ describe('loaders/eslint', () => {

expect(actualOptions).to.deep.equal({
cache: true,
parser: 'babel-eslint',
emitWarning: false
});
});
Expand Down