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

Breaking: eslint:recommended changes (fixes #10768) #11518

Merged
merged 2 commits into from Apr 26, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -15,7 +15,7 @@ module.exports = {
docs: {
description: "disallow using an async function as a Promise executor",
category: "Possible Errors",
recommended: false,
recommended: true,
url: "https://eslint.org/docs/rules/no-async-promise-executor"
},

@@ -22,7 +22,7 @@ module.exports = {
docs: {
description: "disallow the use of `console`",
category: "Possible Errors",
recommended: true,
recommended: false,
This conversation was marked as resolved by aladdin-add

This comment has been minimized.

Copy link
@ljharb

ljharb Mar 18, 2019

Contributor

this isn’t a breaking change; could it land separately?

This comment has been minimized.

Copy link
@not-an-aardvark

not-an-aardvark Mar 18, 2019

Member

I think it is a breaking change; projects that want to disallow console can currently rely on eslint:recommended to do so. By removing the rule we would no longer be providing that guarantee, and those projects would need to be told that they should migrate by configuring no-console explicitly.

We do sometimes loosen the default options for particular rules in a minor release, but in those cases I would say the impact of a false negative is usually lower.

url: "https://eslint.org/docs/rules/no-console"
},

@@ -106,7 +106,7 @@ module.exports = {
docs: {
description: "disallow characters which are made with multiple code points in character class syntax",
category: "Possible Errors",
recommended: false,
recommended: true,
url: "https://eslint.org/docs/rules/no-misleading-character-class"
},

@@ -15,7 +15,7 @@ module.exports = {
docs: {
description: "disallow calling some `Object.prototype` methods directly on objects",
category: "Possible Errors",
recommended: false,
recommended: true,
url: "https://eslint.org/docs/rules/no-prototype-builtins"
},

@@ -28,7 +28,7 @@ module.exports = {
docs: {
description: "disallow identifiers from shadowing restricted names",
category: "Variables",
recommended: false,
recommended: true,
url: "https://eslint.org/docs/rules/no-shadow-restricted-names"
},

@@ -16,7 +16,7 @@ module.exports = {
docs: {
description: "disallow unnecessary `catch` clauses",
category: "Best Practices",
recommended: false,
recommended: true,
url: "https://eslint.org/docs/rules/no-useless-catch"
},

@@ -16,7 +16,7 @@ module.exports = {
docs: {
description: "disallow `with` statements",
category: "Best Practices",
recommended: false,
recommended: true,
url: "https://eslint.org/docs/rules/no-with"
},

@@ -17,7 +17,7 @@ module.exports = {
docs: {
description: "disallow assignments that can lead to race conditions due to usage of `await` or `yield`",
category: "Possible Errors",
recommended: false,
recommended: true,
url: "https://eslint.org/docs/rules/require-atomic-updates"
},

@@ -50,7 +50,6 @@ rules:
new-parens: "error"
no-alert: "error"
no-array-constructor: "error"
no-async-promise-executor: "error"
no-buffer-constructor: "error"
no-caller: "error"
no-confusing-arrow: "error"
@@ -87,7 +86,6 @@ rules:
no-path-concat: "error"
This conversation was marked as resolved by not-an-aardvark

This comment has been minimized.

Copy link
@not-an-aardvark

not-an-aardvark Mar 17, 2019

Member

Can we add no-console since it was removed?

This comment has been minimized.

Copy link
@aladdin-add

aladdin-add Mar 18, 2019

Author Member

It was at Line 57.

seems it has been inconsistent here --- some recommended rules were included, while some were not.

no-process-exit: "error"
no-proto: "error"
no-prototype-builtins: "error"
no-redeclare: "error"
no-restricted-properties: [
"error",
@@ -104,7 +102,6 @@ rules:
no-self-compare: "error"
no-sequences: "error"
no-shadow: "error"
no-shadow-restricted-names: "error"
no-tabs: "error"
no-throw-literal: "error"
no-trailing-spaces: "error"
@@ -118,15 +115,13 @@ rules:
no-unused-vars: ["error", {vars: "all", args: "after-used"}]
no-use-before-define: "error"
no-useless-call: "error"
no-useless-catch: "error"
no-useless-computed-key: "error"
no-useless-concat: "error"
no-useless-constructor: "error"
no-useless-escape: "error"
no-useless-rename: "error"
no-useless-return: "error"
no-whitespace-before-property: "error"
no-with: "error"
no-var: "error"
object-curly-newline: ["error", { "consistent": true, "multiline": true }]
object-curly-spacing: ["error", "always"]
@@ -158,7 +153,6 @@ rules:
quotes: ["error", "double", {avoidEscape: true}]
quote-props: ["error", "as-needed"]
radix: "error"
require-atomic-updates: "error"
require-jsdoc: "error"
require-unicode-regexp: "error"
rest-spread-spacing: "error"
@@ -467,7 +467,7 @@ describe("cli", () => {

cli.execute(`--no-eslintrc --config ./conf/eslint-recommended.js --no-ignore ${files.join(" ")}`);

assert.strictEqual(log.info.args[0][0].split("\n").length, 11);
assert.strictEqual(log.info.args[0][0].split("\n").length, 10);
});
});

@@ -366,7 +366,7 @@ describe("configInitializer", () => {

it("should extend and not disable recommended rules", () => {
assert.strictEqual(config.extends, "eslint:recommended");
assert.notProperty(config.rules, "no-console");
assert.notProperty(config.rules, "no-debugger");
});

it("should support new ES features if using later ES version", () => {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.