Config json cloning is a BREAKING CHANGE!

[JounQin]

Related: #13034 (comment)

Tell us about your environment

• ESLint Version: 7.3.1
• Node Version: v14.4.0
• npm Version: 6.14.4

What parser (default, Babel-ESLint, etc.) are you using?

eslint-mdx

Please show your full configuration:

Configuration

module.exports = {
  extends: 'plugin:unicorn/recommended',
  rules: {
    'unicorn/filename-case': [
      2,
      {
        cases: {
          kebabCase: true,
          pascalCase: true,
        },
        // ignore UPPER_CASE markdown filenames
        ignore: [/^[A-Z](([\dA-Z]+_)*[\dA-Z]+)?\.mdx?$/],
      },
    ]
  }
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

<!-- README.md -->

eslint README.md

What did you expect to happen?
no error

What actually happened? Please include the actual, raw output from ESLint.

1:2  error  Filename is not in kebab case or pascal case. Rename it to `readme.md` or `Readme.md`  unicorn/filename-case

The error occurres because the ESLint config is json cloned since v7.3.0, and this breaks Regxp option of unicorn/filename-case, I don't think this is an invalid usage.

[anikethsaha]

You can try regex in string ? in your case, '/^[A-Z](([\dA-Z]+_)*[\dA-Z]+)?\.mdx?$/', though the flags wont work here, there can be work-around in the unicorn plugin to remove the regex instance and go with full string , even string for flags in a array

like

ignore: [ '^FOOBAR\\.js$', ['^FOOBAR\\.js$', 'i'] ]

but yeah , this was a breaking change.

[aladdin-add]

what do you think revert the change? we can reconsider introducing it in a major release(eslint v8)

[anikethsaha]

I did mention that earlier, #13034 (comment).

+1 for that

[kaicataldo]

I personally think this change should be reverted (even though I understand it was considered a bug fix, because the intention was always to have ESLint configs be serializable). @eslint/eslint-tsc I know there was other discussion of this while I was on vacation. Would someone like to fill us in on what the final decision was?

[nzakas]

The discussion was to wait and see how many bug reports we received before deciding whether to revert or not. We had just one report of the use of Infinity, and given that was clearly a bug and could be easily replaced with a valid number to work the same, we decided to add a patch to allow that isolated case.

This case is more concerning because it involves encouraging end users to use regular expressions in their configs (vs one isolated instance of Infinity in one config).

With this new data, I’m in favor of reverting the change.

[aladdin-add]

well, seems we have reached a consensus to revert it. will make a PR later!

[kaicataldo]

@btmills @mysticatea Would love to hear your opinions as well :)

[mysticatea]

👍 for reverting.

[btmills]

Agreed with @nzakas - this is the sort of end-user case we were looking for that would indicate we need to revert. A couple things we could do in addition to reverting:

• Add custom replacer functions that log non-fatal warnings when non-serializable values are found in the cache and --print-config option.
• Open an issue with eslint-plugin-unicorn to start a discussion around alternatives.

[ljharb]

Note that the cloning isn't the issue; it's using JSON to clone that's the issue. A proper deep cloning algorithm would work fine and preserve the fix that cloning originally solved.

[kaicataldo]

Marking as accepted since the entire TSC is in favor of reverting.

[kaicataldo]

@ljharb Yeah, that was also explored. I believe the reason that implementation was not chosen was due to challenges with potential circular references.

[ljharb]

@kaicataldo yeah, that gets tricky, but i'd expect that to be handleable with a WeakMap, and mapping "original object" to "cloned object", prior to iterating over the original object to clone copies into the cloned object, and always looking up a cloned object in the map before creating a new one.

[anikethsaha]

similar to #13034 (comment) ?