Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
- [`no-named-as-default`]: Allow using an identifier if the export is both a named and a default export ([#3032], thanks [@akwodkiewicz])
- [`export`]: False positive for exported overloaded functions in TS ([#3065], thanks [@liuxingbaoyu])
- `exportMap`: export map cache is tainted by unreliable parse results ([#3062], thanks [@michaelfaith])
- `exportMap`: improve cacheKey when using flat config ([#3072], thanks [@michaelfaith])

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

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

[#3072]: https://github.com/import-js/eslint-plugin-import/pull/3072
[#3071]: https://github.com/import-js/eslint-plugin-import/pull/3071
[#3070]: https://github.com/import-js/eslint-plugin-import/pull/3070
[#3068]: https://github.com/import-js/eslint-plugin-import/pull/3068
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@
"debug": "^3.2.7",
"doctrine": "^2.1.0",
"eslint-import-resolver-node": "^0.3.9",
"eslint-module-utils": "^2.11.1",
"eslint-module-utils": "^2.12.0",
"hasown": "^2.0.2",
"is-core-module": "^2.15.1",
"is-glob": "^4.0.3",
Expand Down
35 changes: 29 additions & 6 deletions src/exportMap/childContext.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import { hashObject } from 'eslint-module-utils/hash';

let parserOptionsHash = '';
let prevParserOptions = '';
let optionsHash = '';
let prevOptions = '';
let settingsHash = '';
let prevSettings = '';

// Replacer function helps us with serializing the parser nested within `languageOptions`.
function stringifyReplacerFn(_, value) {
if (typeof value === 'function') {
return String(value);
}
return value;
}

/**
* don't hold full context object in memory, just grab what we need.
* also calculate a cacheKey, where parts of the cacheKey hash are memoized
Expand All @@ -17,13 +25,28 @@ export default function childContext(path, context) {
prevSettings = JSON.stringify(settings);
}

if (JSON.stringify(parserOptions) !== prevParserOptions) {
parserOptionsHash = hashObject({ parserOptions }).digest('hex');
prevParserOptions = JSON.stringify(parserOptions);
// We'll use either a combination of `parserOptions` and `parserPath` or `languageOptions`
// to construct the cache key, depending on whether this is using a flat config or not.
let optionsToken;
if (!parserPath && languageOptions) {
if (JSON.stringify(languageOptions, stringifyReplacerFn) !== prevOptions) {
optionsHash = hashObject({ languageOptions }).digest('hex');
prevOptions = JSON.stringify(languageOptions, stringifyReplacerFn);
}
// For languageOptions, we're just using the hashed options as the options token
optionsToken = optionsHash;
} else {
if (JSON.stringify(parserOptions) !== prevOptions) {
optionsHash = hashObject({ parserOptions }).digest('hex');
prevOptions = JSON.stringify(parserOptions);
}
// When not using flat config, we use a combination of the hashed parserOptions
// and parserPath as the token
optionsToken = String(parserPath) + optionsHash;
}

return {
cacheKey: String(parserPath) + parserOptionsHash + settingsHash + String(path),
cacheKey: optionsToken + settingsHash + String(path),
ljharb marked this conversation as resolved.
Show resolved Hide resolved
settings,
parserOptions,
parserPath,
Expand Down
72 changes: 71 additions & 1 deletion tests/src/exportMap/childContext.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expect } from 'chai';
import { hashObject } from 'eslint-module-utils/hash';

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

Expand All @@ -16,8 +17,13 @@ describe('childContext', () => {
const languageOptions = {
ecmaVersion: 2024,
sourceType: 'module',
parser: {},
parser: {
parseForESLint() { return 'parser1'; },
},
};
const languageOptionsHash = hashObject({ languageOptions }).digest('hex');
const parserOptionsHash = hashObject({ parserOptions }).digest('hex');
const settingsHash = hashObject({ settings }).digest('hex');

// https://github.com/import-js/eslint-plugin-import/issues/3051
it('should pass context properties through, if present', () => {
Expand Down Expand Up @@ -48,4 +54,68 @@ describe('childContext', () => {
expect(result.path).to.equal(path);
expect(result.cacheKey).to.be.a('string');
});

it('should construct cache key out of languageOptions if present', () => {
const mockContext = {
settings,
languageOptions,
};

const result = childContext(path, mockContext);

expect(result.cacheKey).to.equal(languageOptionsHash + settingsHash + path);
});

it('should use the same cache key upon multiple calls', () => {
const mockContext = {
settings,
languageOptions,
};

let result = childContext(path, mockContext);

const expectedCacheKey = languageOptionsHash + settingsHash + path;
expect(result.cacheKey).to.equal(expectedCacheKey);

result = childContext(path, mockContext);
expect(result.cacheKey).to.equal(expectedCacheKey);
});

it('should update cacheKey if different languageOptions are passed in', () => {
const mockContext = {
settings,
languageOptions,
};

let result = childContext(path, mockContext);

const firstCacheKey = languageOptionsHash + settingsHash + path;
expect(result.cacheKey).to.equal(firstCacheKey);

// Second run with different parser function
mockContext.languageOptions = {
...languageOptions,
parser: {
parseForESLint() { return 'parser2'; },
},
};

result = childContext(path, mockContext);

const secondCacheKey = hashObject({ languageOptions: mockContext.languageOptions }).digest('hex') + settingsHash + path;
expect(result.cacheKey).to.not.equal(firstCacheKey);
expect(result.cacheKey).to.equal(secondCacheKey);
});

it('should construct cache key out of parserOptions and parserPath if no languageOptions', () => {
const mockContext = {
settings,
parserOptions,
parserPath,
};

const result = childContext(path, mockContext);

expect(result.cacheKey).to.equal(String(parserPath) + parserOptionsHash + settingsHash + path);
});
});
6 changes: 6 additions & 0 deletions utils/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange

## Unreleased

## v2.12.0 - 2024-09-26

### Added
- `hash`: add support for hashing functions ([#3072], thanks [@michaelfaith])

## v2.11.1 - 2024-09-23

### Fixed
Expand Down Expand Up @@ -172,6 +177,7 @@ Yanked due to critical issue with cache key resulting from #839.
### Fixed
- `unambiguous.test()` regex is now properly in multiline mode

[#3072]: https://github.com/import-js/eslint-plugin-import/pull/3072
[#3061]: https://github.com/import-js/eslint-plugin-import/pull/3061
[#3057]: https://github.com/import-js/eslint-plugin-import/pull/3057
[#3049]: https://github.com/import-js/eslint-plugin-import/pull/3049
Expand Down
2 changes: 2 additions & 0 deletions utils/hash.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ function hashify(value, hash) {

if (Array.isArray(value)) {
hashArray(value, hash);
} else if (typeof value === 'function') {
hash.update(String(value));
} else if (value instanceof Object) {
hashObject(value, hash);
} else {
Expand Down
2 changes: 1 addition & 1 deletion utils/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "eslint-module-utils",
"version": "2.11.1",
"version": "2.12.0",
"description": "Core utilities to support eslint-plugin-import and other module-related plugins.",
"engines": {
"node": ">=4"
Expand Down