From e4c27ef0280cb3f97ff44f9b16a6d55f1eb8addc Mon Sep 17 00:00:00 2001 From: michael faith Date: Sun, 15 Sep 2024 11:12:26 -0500 Subject: [PATCH] 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. --- src/exportMap/builder.js | 6 +++++- tests/src/core/getExports.js | 11 ++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/exportMap/builder.js b/src/exportMap/builder.js index 5348dba37..f7b9006ef 100644 --- a/src/exportMap/builder.js +++ b/src/exportMap/builder.js @@ -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; } diff --git a/tests/src/core/getExports.js b/tests/src/core/getExports.js index 76003410d..da3182a3e 100644 --- a/tests/src/core/getExports.js +++ b/tests/src/core/getExports.js @@ -21,7 +21,7 @@ describe('ExportMap', function () { }, { settings: {}, - parserPath: 'babel-eslint', + parserPath: require.resolve('babel-eslint'), }, ); @@ -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;