Skip to content

Commit

Permalink
fix(utils): export map cache is tainted by unreliable parse results
Browse files Browse the repository at this point in the history
This change addresses and issue observed with the v9 upgrade, where the ExportMap Cache is being tainted with a bad export map, if the parse doesn't yield a `visitorKeys` (which can happen if an incompatible parser is used (e.g. og babel eslint)) for one run of the no-cycle rule.  If a subsequent test is run with a compatible parser, then the bad export map will be found in the cache and used instead of attempting to proceed with the parse.

I also updated the `getExports` test to use a valid parserPath, rather than a mocked one, so that the tests are acting on a valid parserPath.  Otherwise the export map won't be cached following this change.
  • Loading branch information
michaelfaith committed Sep 15, 2024
1 parent f72f207 commit e4c27ef
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
6 changes: 5 additions & 1 deletion src/exportMap/builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,11 @@ export default class ExportMapBuilder {

exportMap.mtime = stats.mtime;

exportCache.set(cacheKey, exportMap);
// If the visitor keys were not populated, then we shouldn't save anything to the cache,
// since the parse results may not be reliable.
if (exportMap.visitorKeys) {
exportCache.set(cacheKey, exportMap);
}
return exportMap;
}

Expand Down
11 changes: 10 additions & 1 deletion tests/src/core/getExports.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('ExportMap', function () {
},
{
settings: {},
parserPath: 'babel-eslint',
parserPath: require.resolve('babel-eslint'),
},
);

Expand All @@ -41,6 +41,15 @@ describe('ExportMap', function () {
.to.exist.and.equal(ExportMapBuilder.get('./named-exports', fakeContext));
});

it('does not return a cached copy if the parse does not yield a visitor keys', function () {
const mockContext = {
...fakeContext,
parserPath: 'not-real',
}
expect(ExportMapBuilder.get('./named-exports', mockContext))
.to.exist.and.not.equal(ExportMapBuilder.get('./named-exports', mockContext));
});

it('does not return a cached copy after modification', (done) => {
const firstAccess = ExportMapBuilder.get('./mutator', fakeContext);
expect(firstAccess).to.exist;
Expand Down

0 comments on commit e4c27ef

Please sign in to comment.