import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.57k stars 1.57k forks source link

[Fix] `exportMap`: improve `cacheKey` when using flat config #3072

Closed michaelfaith closed 1 month ago

michaelfaith commented 1 month ago

Discovered this issue in https://github.com/import-js/eslint-plugin-import/pull/2996#issuecomment-2372522774.

This change improves the logic for generating the cache key used for both the ExportMap cache and the resolver cache when using flat config. Prior to this change, the cache key was a combination of the parserPath, a hash of the parserOptions, a hash of the plugin settings, and the path of the file. When using flat config, parserPath isn't provided. So, there's the possibility of incorrect / stale cache objects being used if someone ran with this plugin using different parsers within the same lint execution, if parserOptions and settings are the same.

For flat config, we can achieve similar results by constructing the cacheKey using languageOptions as a component of the key, rather than parserPath and parserOptions. One caveat is that the parser property of languageOptions is an object that oftentimes has the same two functions (parse and parseForESLint). This won't be reliably distinct when using the base JSON.stringify function to detect changes. So, this implementation uses a replacer function along with JSON.stringify to stringify function properties along with other property types in order to detect different parsers.

To ensure that this works properly with v9, I also tested this against https://github.com/import-js/eslint-plugin-import/pull/2996 with v9 installed. Not only does it pass all tests in that branch, but it also removes the need to add this exception: https://github.com/import-js/eslint-plugin-import/pull/2996#discussion_r1774135785

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.95%. Comparing base (61f02a2) to head (250571e).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3072 +/- ## ========================================== - Coverage 95.39% 94.95% -0.45% ========================================== Files 82 82 Lines 3560 3565 +5 Branches 1244 1246 +2 ========================================== - Hits 3396 3385 -11 - Misses 164 180 +16 ``` | [Flag](https://app.codecov.io/gh/import-js/eslint-plugin-import/pull/3072/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=import-js) | Coverage Δ | | |---|---|---| | [](https://app.codecov.io/gh/import-js/eslint-plugin-import/pull/3072/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=import-js) | `94.95% <100.00%> (-0.45%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=import-js#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

michaelfaith commented 1 month ago

Note: I broke the change to utils/hash out as its own commit, so that it can be released separately.

ljharb commented 1 month ago

Published v2.12.0 of eslint-module-utils already; this PR will be merged once CI passes.