[eslint-patch] Allow for extended shareable ESLint configurations to use any name #3020
Conversation
D4N14L
requested review from
apostolisms,
halfnibble,
iclanton,
octogonz and
sachinjoseph
as code owners
This doesn't seem true to me. We can chat about it offline and then follow up. |
D4N14L
force-pushed
the
user/danade/EslintResolverTweaks
branch
from
1efa4aa to
6f30379
Compare
|
|
||
| // Make sure we actually resolved the module in our call path | ||
| // and not some other spurious dependency. | ||
| if (path.join(eslintrcFolder, 'dist/eslintrc.cjs') === currentModule.filename) { |
There was a problem hiding this comment.
Wouldn't it be more efficient to verify currentModule.filename ends with dist/eslintrc.cjs before calling require.resolve, then verify that the output of path.dirname matches the remainder of currentModule.filename?
|
|
||
| // Make sure we actually resolved the module in our call path | ||
| // and not some other spurious dependency. | ||
| if (path.join(eslintCandidateFolder, 'lib/cli-engine/cli-engine.js') === currentModule.filename) { |
| }) | ||
| ); | ||
|
|
||
| if (path.join(eslintrcFolder, '/lib/config-array-factory.js') == currentModule.filename) { |
There was a problem hiding this comment.
Why the switch to == from ===?
| } | ||
|
|
||
| // Detect the ESLint package version | ||
| const eslintPackageJson = fs.readFileSync(path.join(eslintFolder, 'package.json')).toString(); |
There was a problem hiding this comment.
Does calling toString() provide some advantage over specifying the encoding in the readFileSync call? I'd expect the latter to potentially optimize internally for different buffering behavior.
|
Note: PR #3337 has renamed our GitHub |
|
Superseded by #4353 |
Summary
Allows resolution of extended shareable configurations even when the package they are sourced from is not prefixed with
eslint-config-.Details
In order to allow use of ESLint configurations provided via Heft rigs, we need to allow consumption of configurations from packages not prefixed with
eslint-config-, since Heft rigs obviously do not have the same naming restrictions.The solution first attempts to resolve using the default logic, and falls back to attempting to resolve using the provided extended config name directly, without modification. This is done to allow for backwards-compatibility with intentionally short-named extended configs (cases where the prefix is intentionally omitted), since ESLint prefixes the
eslint-config-to the package name during normalization if it was not found. For example, if@rushstackwas specified as an extends configuration, then it would normally resolve to@rushstack/eslint-config.How it was tested
Tested using resolution logic in Rushstack by moving the ESLint configuration into the rig. A future change will transition Rushstack to use the
@rushstack/eslint-configpackage via@rushstack/heft-node-rigand@rushstack/heft-web-rig.