This repository has been archived by the owner on Feb 18, 2024. It is now read-only.
Stop using private ESLint APIs for ESLint config merging #1464
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In #1182, the previous broken ESLint config merging functionality was replaced by using an internal ESLint `merge` function from `config-ops`. Unfortunately in ESLint 6 that function no longer exists in a form that is easy for us to use, so in order to support ESLint 6, I have replaced it with simpler manual merges. These are virtually identical, with one difference - `rules` are now merged by a later rule entry completely replacing the first, rather than updating it. For example, merging these: `'foo': ['error', {'someSetting': false}]` `'foo': 'warn'` ...used to give: `'foo': ['warn', {'someSetting': false}]` ...but now results in: `'foo': 'warn'` I think this difference is a reasonable compromise for not having to rely on ESLint internals and/or implement our own more complex merging. I also believe hitting this will be rare, since: * it doesn't affect merging with rules defined inside an `extends` external file * it won't make a difference for rules that were already at default settings (or that were overridden with custom settings in the later rule definition being merged in) This more simplistic merging behaviour is also what's used by Neutrino 8 (#1182 landed for Neutrino 9 only) - and we still have the other bug fixes included in #1182 that were the primary motivation for refactoring in the first place. Refs #1423.
|
Took a quick peak. Looks good to me... |
timkelty
approved these changes
Sep 9, 2019
|
Thank you for the reviews :-) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #1182, the previous broken ESLint config merging functionality was replaced by using an internal ESLint
mergefunction fromconfig-ops.Unfortunately in ESLint 6 that function no longer exists in a form that is easy for us to use, so in order to support ESLint 6, I have replaced it with simpler manual merges. These are virtually identical, with one difference -
rulesare now merged by a later rule entry completely replacing the first, rather than updating it.For example, merging these:
'foo': ['error', {'someSetting': false}]'foo': 'warn'...used to give:
'foo': ['warn', {'someSetting': false}]...but now results in:
'foo': 'warn'I think this difference is a reasonable compromise for not having to rely on ESLint internals and/or implement our own more complex merging. I also believe hitting this will be rare, since:
extendsexternal fileThis more simplistic merging behaviour is also what's used by Neutrino 8 (#1182 landed for Neutrino 9 only) - and we still have the other bug fixes included in #1182 that were the primary motivation for refactoring in the first place.
Refs #1423.