Skip to content

Commit 2390c2c

Browse files
michaelfaithsparten11740
authored andcommitted
[Fix] exportMap: improve cacheKey when using flat config
Discovered this issue in import-js#2996 (comment) This change improves the logic for generating the cache key used for both the exportMap cache and 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 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. The equivalent cacheKey construction when using flat config is to use `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 replace function along with `JSON.stringify` to stringify function properties along with other property types. To ensure that this will work properly with v9, I also tested this against import-js#2996 with v9 installed. Not only does it pass all tests in that branch, but it also removes the need to add this exception: import-js#2996 (comment)
1 parent c64bb4f commit 2390c2c

File tree

4 files changed

+103
-8
lines changed

4 files changed

+103
-8
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
1414
- [`no-named-as-default`]: Allow using an identifier if the export is both a named and a default export ([#3032], thanks [@akwodkiewicz])
1515
- [`export`]: False positive for exported overloaded functions in TS ([#3065], thanks [@liuxingbaoyu])
1616
- `exportMap`: export map cache is tainted by unreliable parse results ([#3062], thanks [@michaelfaith])
17+
- `exportMap`: improve cacheKey when using flat config ([#3072], thanks [@michaelfaith])
1718

1819
### Changed
1920
- [Docs] [`no-relative-packages`]: fix typo ([#3066], thanks [@joshuaobrien])
@@ -1144,6 +1145,7 @@ for info on changes for earlier releases.
11441145

11451146
[`memo-parser`]: ./memo-parser/README.md
11461147

1148+
[#3072]: https://github.com/import-js/eslint-plugin-import/pull/3072
11471149
[#3071]: https://github.com/import-js/eslint-plugin-import/pull/3071
11481150
[#3070]: https://github.com/import-js/eslint-plugin-import/pull/3070
11491151
[#3068]: https://github.com/import-js/eslint-plugin-import/pull/3068

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@
117117
"debug": "^3.2.7",
118118
"doctrine": "^2.1.0",
119119
"eslint-import-resolver-node": "^0.3.9",
120-
"eslint-module-utils": "^2.11.1",
120+
"eslint-module-utils": "^2.12.0",
121121
"hasown": "^2.0.2",
122122
"is-core-module": "^2.15.1",
123123
"is-glob": "^4.0.3",

src/exportMap/childContext.js

+29-6
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
11
import { hashObject } from 'eslint-module-utils/hash';
22

3-
let parserOptionsHash = '';
4-
let prevParserOptions = '';
3+
let optionsHash = '';
4+
let prevOptions = '';
55
let settingsHash = '';
66
let prevSettings = '';
77

8+
// Replacer function helps us with serializing the parser nested within `languageOptions`.
9+
function stringifyReplacerFn(_, value) {
10+
if (typeof value === 'function') {
11+
return String(value);
12+
}
13+
return value;
14+
}
15+
816
/**
917
* don't hold full context object in memory, just grab what we need.
1018
* also calculate a cacheKey, where parts of the cacheKey hash are memoized
@@ -17,13 +25,28 @@ export default function childContext(path, context) {
1725
prevSettings = JSON.stringify(settings);
1826
}
1927

20-
if (JSON.stringify(parserOptions) !== prevParserOptions) {
21-
parserOptionsHash = hashObject({ parserOptions }).digest('hex');
22-
prevParserOptions = JSON.stringify(parserOptions);
28+
// We'll use either a combination of `parserOptions` and `parserPath` or `languageOptions`
29+
// to construct the cache key, depending on whether this is using a flat config or not.
30+
let optionsToken;
31+
if (!parserPath && languageOptions) {
32+
if (JSON.stringify(languageOptions, stringifyReplacerFn) !== prevOptions) {
33+
optionsHash = hashObject({ languageOptions }).digest('hex');
34+
prevOptions = JSON.stringify(languageOptions, stringifyReplacerFn);
35+
}
36+
// For languageOptions, we're just using the hashed options as the options token
37+
optionsToken = optionsHash;
38+
} else {
39+
if (JSON.stringify(parserOptions) !== prevOptions) {
40+
optionsHash = hashObject({ parserOptions }).digest('hex');
41+
prevOptions = JSON.stringify(parserOptions);
42+
}
43+
// When not using flat config, we use a combination of the hashed parserOptions
44+
// and parserPath as the token
45+
optionsToken = String(parserPath) + optionsHash;
2346
}
2447

2548
return {
26-
cacheKey: String(parserPath) + parserOptionsHash + settingsHash + String(path),
49+
cacheKey: optionsToken + settingsHash + String(path),
2750
settings,
2851
parserOptions,
2952
parserPath,

tests/src/exportMap/childContext.js

+71-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { expect } from 'chai';
2+
import { hashObject } from 'eslint-module-utils/hash';
23

34
import childContext from '../../../src/exportMap/childContext';
45

@@ -16,8 +17,13 @@ describe('childContext', () => {
1617
const languageOptions = {
1718
ecmaVersion: 2024,
1819
sourceType: 'module',
19-
parser: {},
20+
parser: {
21+
parseForESLint() { return 'parser1'; },
22+
},
2023
};
24+
const languageOptionsHash = hashObject({ languageOptions }).digest('hex');
25+
const parserOptionsHash = hashObject({ parserOptions }).digest('hex');
26+
const settingsHash = hashObject({ settings }).digest('hex');
2127

2228
// https://github.com/import-js/eslint-plugin-import/issues/3051
2329
it('should pass context properties through, if present', () => {
@@ -48,4 +54,68 @@ describe('childContext', () => {
4854
expect(result.path).to.equal(path);
4955
expect(result.cacheKey).to.be.a('string');
5056
});
57+
58+
it('should construct cache key out of languageOptions if present', () => {
59+
const mockContext = {
60+
settings,
61+
languageOptions,
62+
};
63+
64+
const result = childContext(path, mockContext);
65+
66+
expect(result.cacheKey).to.equal(languageOptionsHash + settingsHash + path);
67+
});
68+
69+
it('should use the same cache key upon multiple calls', () => {
70+
const mockContext = {
71+
settings,
72+
languageOptions,
73+
};
74+
75+
let result = childContext(path, mockContext);
76+
77+
const expectedCacheKey = languageOptionsHash + settingsHash + path;
78+
expect(result.cacheKey).to.equal(expectedCacheKey);
79+
80+
result = childContext(path, mockContext);
81+
expect(result.cacheKey).to.equal(expectedCacheKey);
82+
});
83+
84+
it('should update cacheKey if different languageOptions are passed in', () => {
85+
const mockContext = {
86+
settings,
87+
languageOptions,
88+
};
89+
90+
let result = childContext(path, mockContext);
91+
92+
const firstCacheKey = languageOptionsHash + settingsHash + path;
93+
expect(result.cacheKey).to.equal(firstCacheKey);
94+
95+
// Second run with different parser function
96+
mockContext.languageOptions = {
97+
...languageOptions,
98+
parser: {
99+
parseForESLint() { return 'parser2'; },
100+
},
101+
};
102+
103+
result = childContext(path, mockContext);
104+
105+
const secondCacheKey = hashObject({ languageOptions: mockContext.languageOptions }).digest('hex') + settingsHash + path;
106+
expect(result.cacheKey).to.not.equal(firstCacheKey);
107+
expect(result.cacheKey).to.equal(secondCacheKey);
108+
});
109+
110+
it('should construct cache key out of parserOptions and parserPath if no languageOptions', () => {
111+
const mockContext = {
112+
settings,
113+
parserOptions,
114+
parserPath,
115+
};
116+
117+
const result = childContext(path, mockContext);
118+
119+
expect(result.cacheKey).to.equal(String(parserPath) + parserOptionsHash + settingsHash + path);
120+
});
51121
});

0 commit comments

Comments
 (0)