Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix: Revert config cloning (fixes #13447) (#13449)
* Revert "Fix: Replace Infinity with Number.MAX_SAFE_INTEGER (fixes #13427) (#13435)"

This reverts commit de77c11.

* Revert "Fix: clone config before validating (fixes #12592) (#13034)"

This reverts commit 7fb45cf.
  • Loading branch information
aladdin-add committed Jul 3, 2020
1 parent 0a463db commit 89ee01e
Show file tree
Hide file tree
Showing 8 changed files with 1 addition and 138 deletions.
34 changes: 1 addition & 33 deletions lib/cli-engine/config-array-factory.js
Expand Up @@ -697,38 +697,6 @@ class ConfigArrayFactory {
ctx.matchBasePath
);

/**
* Cloning the rule's config as we are setting `useDefaults` to true`
* which mutates the config with its default value if present. And when we
* refer to a same variable for config for different rules, that referred variable will
* be mutated and it will be used for both.
*
* Example:
*
* const commonRuleConfig = ['error', {}];
*
* Now if we use this variable as a config for rules like this
*
* {
* rules: {
* "a" : commonRuleConfig,
* "b" : commonRuleConfig,
* }
* }
*
* And if these rules have default values in their schema, their
* config will be mutated with default values, the mutated `commonRuleConfig` will be used for `b` as well and it probably
* throw schema voilation errors.
*
* Refer https://github.com/eslint/eslint/issues/12592
*/
const clonedRulesConfig = rules && JSON.parse(
JSON.stringify(
rules,
(key, value) => (value === Infinity ? Number.MAX_SAFE_INTEGER : value)
)
);

// Flatten `extends`.
for (const extendName of extendList.filter(Boolean)) {
yield* this._loadExtends(extendName, ctx);
Expand Down Expand Up @@ -763,7 +731,7 @@ class ConfigArrayFactory {
processor,
reportUnusedDisableDirectives,
root,
rules: clonedRulesConfig,
rules,
settings
};

Expand Down

This file was deleted.

This file was deleted.

4 changes: 0 additions & 4 deletions tests/fixtures/config-file/cloned-config/eslintConfig.js

This file was deleted.

13 changes: 0 additions & 13 deletions tests/fixtures/config-file/cloned-config/eslintConfigFail.js

This file was deleted.

1 change: 0 additions & 1 deletion tests/fixtures/config-file/cloned-config/index.js

This file was deleted.

3 changes: 0 additions & 3 deletions tests/fixtures/config-file/cloned-config/inlineText.js

This file was deleted.

63 changes: 0 additions & 63 deletions tests/lib/cli.js
Expand Up @@ -187,7 +187,6 @@ describe("cli", () => {
});
});


describe("when given a config with environment set to Node.js", () => {
it("should execute without any errors", async () => {
const configPath = getFixturePath("configurations", "env-node.json");
Expand Down Expand Up @@ -1161,66 +1160,4 @@ describe("cli", () => {
});
});

describe("testing the cloned config", () => {
describe("config file and input file", () => {
it("should not modify original configuration object", async () => {
const configPath = getFixturePath("config-file", "cloned-config", "eslintConfig.js");
const filePath = getFixturePath("config-file", "cloned-config", "index.js");
const args = `--config ${configPath} ${filePath}`;

const exit = await cli.execute(args);

assert.strictEqual(exit, 0);
});

it("should exit with 1 as camelcase has wrong property type", async () => {
const configPath = getFixturePath("config-file", "cloned-config", "eslintConfigFail.js");
const filePath = getFixturePath("config-file", "cloned-config", "index.js");
const args = `--config ${configPath} ${filePath}`;

try {
await cli.execute(args);
} catch (error) {
assert.strictEqual(/Configuration for rule "camelcase" is invalid:/u.test(error), true);
}

});

it("should not cause an error when a rule configuration has `Infinity`", async () => {
const configPath = getFixturePath("config-file", "cloned-config", "configWithInfinity.js");
const filePath = getFixturePath("config-file", "cloned-config", "index.js");
const args = `--config ${configPath} ${filePath}`;

const exit = await cli.execute(args);

assert.strictEqual(exit, 0);
});
});

describe("inline config and input file", () => {
it("should not modify original configuration object", async () => {
const filePath = getFixturePath("config-file", "cloned-config", "inlineText.js");
const args = `${filePath}`;

const exit = await cli.execute(args);

assert.strictEqual(exit, 0);
});
});

});

describe("handling circular reference while cloning", () => {
it("should handle circular ref", async () => {
const configPath = getFixturePath("config-file", "cloned-config", "circularRefEslintConfig.js");
const filePath = getFixturePath("config-file", "cloned-config", "index.js");
const args = `--config ${configPath} ${filePath}`;

try {
await cli.execute(args);
} catch (error) {
assert.instanceOf(error, Error);
}
});
});
});

0 comments on commit 89ee01e

Please sign in to comment.