Skip to content

Commit e4c27ef

Browse files
committed
fix(utils): export map cache is tainted by unreliable parse results
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.
1 parent f72f207 commit e4c27ef

File tree

2 files changed

+15
-2
lines changed

2 files changed

+15
-2
lines changed

src/exportMap/builder.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,11 @@ export default class ExportMapBuilder {
9292

9393
exportMap.mtime = stats.mtime;
9494

95-
exportCache.set(cacheKey, exportMap);
95+
// If the visitor keys were not populated, then we shouldn't save anything to the cache,
96+
// since the parse results may not be reliable.
97+
if (exportMap.visitorKeys) {
98+
exportCache.set(cacheKey, exportMap);
99+
}
96100
return exportMap;
97101
}
98102

tests/src/core/getExports.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe('ExportMap', function () {
2121
},
2222
{
2323
settings: {},
24-
parserPath: 'babel-eslint',
24+
parserPath: require.resolve('babel-eslint'),
2525
},
2626
);
2727

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

44+
it('does not return a cached copy if the parse does not yield a visitor keys', function () {
45+
const mockContext = {
46+
...fakeContext,
47+
parserPath: 'not-real',
48+
}
49+
expect(ExportMapBuilder.get('./named-exports', mockContext))
50+
.to.exist.and.not.equal(ExportMapBuilder.get('./named-exports', mockContext));
51+
});
52+
4453
it('does not return a cached copy after modification', (done) => {
4554
const firstAccess = ExportMapBuilder.get('./mutator', fakeContext);
4655
expect(firstAccess).to.exist;

0 commit comments

Comments
 (0)