Skip to content

Commit 2ae68c1

Browse files
mblaszczyk-atlassianljharb
authored andcommitted
[Fix] import/no-cycle: fix perf regression
Fixes #1943.
1 parent 462b016 commit 2ae68c1

File tree

4 files changed

+68
-9
lines changed

4 files changed

+68
-9
lines changed

Diff for: CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
1313
- [`no-extraneous-dependencies`]: Add package.json cache ([#1948], thanks @fa93hws)
1414
- [`prefer-default-export`]: handle empty array destructuring ([#1965], thanks @ljharb)
1515
- [`no-unused-modules`]: make type imports mark a module as used (fixes #1924) ([#1974], thanks [@cherryblossom000])
16+
- [`import/no-cycle`]: fix perf regression ([#1944], thanks [@Blasz])
1617

1718
## [2.22.1] - 2020-09-27
1819
### Fixed
@@ -741,6 +742,7 @@ for info on changes for earlier releases.
741742
[`memo-parser`]: ./memo-parser/README.md
742743

743744
[#1974]: https://github.com/benmosher/eslint-plugin-import/pull/1974
745+
[#1944]: https://github.com/benmosher/eslint-plugin-import/pull/1944
744746
[#1924]: https://github.com/benmosher/eslint-plugin-import/issues/1924
745747
[#1965]: https://github.com/benmosher/eslint-plugin-import/issues/1965
746748
[#1948]: https://github.com/benmosher/eslint-plugin-import/pull/1948
@@ -1293,3 +1295,4 @@ for info on changes for earlier releases.
12931295
[@straub]: https://github.com/straub
12941296
[@andreubotella]: https://github.com/andreubotella
12951297
[@cherryblossom000]: https://github.com/cherryblossom000
1298+
[@Blasz]: https://github.com/Blasz

Diff for: src/ExportMap.js

+22-5
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ let parseConfigFileTextToJson;
2222
const log = debug('eslint-plugin-import:ExportMap');
2323

2424
const exportCache = new Map();
25+
const tsConfigCache = new Map();
2526

2627
export default class ExportMap {
2728
constructor(path) {
@@ -438,9 +439,11 @@ ExportMap.parse = function (path, content, context) {
438439

439440
const source = makeSourceCode(content, ast);
440441

441-
function isEsModuleInterop() {
442+
function readTsConfig() {
442443
const tsConfigInfo = tsConfigLoader({
443-
cwd: context.parserOptions && context.parserOptions.tsconfigRootDir || process.cwd(),
444+
cwd:
445+
(context.parserOptions && context.parserOptions.tsconfigRootDir) ||
446+
process.cwd(),
444447
getEnv: (key) => process.env[key],
445448
});
446449
try {
@@ -450,12 +453,26 @@ ExportMap.parse = function (path, content, context) {
450453
// this is because projects not using TypeScript won't have typescript installed
451454
({ parseConfigFileTextToJson } = require('typescript'));
452455
}
453-
const tsConfig = parseConfigFileTextToJson(tsConfigInfo.tsConfigPath, jsonText).config;
454-
return tsConfig.compilerOptions.esModuleInterop;
456+
return parseConfigFileTextToJson(tsConfigInfo.tsConfigPath, jsonText).config;
455457
}
456458
} catch (e) {
457-
return false;
459+
// Catch any errors
458460
}
461+
462+
return null;
463+
}
464+
465+
function isEsModuleInterop() {
466+
const cacheKey = hashObject({
467+
tsconfigRootDir: context.parserOptions && context.parserOptions.tsconfigRootDir,
468+
}).digest('hex');
469+
let tsConfig = tsConfigCache.get(cacheKey);
470+
if (typeof tsConfig === 'undefined') {
471+
tsConfig = readTsConfig();
472+
tsConfigCache.set(cacheKey, tsConfig);
473+
}
474+
475+
return tsConfig !== null ? tsConfig.compilerOptions.esModuleInterop : false;
459476
}
460477

461478
ast.body.forEach(function (n) {

Diff for: tests/files/typescript-export-assign-property.ts

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
const AnalyticsNode = { Analytics: {} };
2+
3+
export = AnalyticsNode.Analytics;

Diff for: tests/src/core/getExports.js

+40-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { expect } from 'chai';
22
import semver from 'semver';
3+
import sinon from 'sinon';
34
import eslintPkg from 'eslint/package.json';
5+
import * as tsConfigLoader from 'tsconfig-paths/lib/tsconfig-loader';
46
import ExportMap from '../../../src/ExportMap';
57

68
import * as fs from 'fs';
@@ -51,7 +53,8 @@ describe('ExportMap', function () {
5153
const differentSettings = Object.assign(
5254
{},
5355
fakeContext,
54-
{ parserPath: 'espree' });
56+
{ parserPath: 'espree' },
57+
);
5558

5659
expect(ExportMap.get('./named-exports', differentSettings))
5760
.to.exist.and
@@ -338,11 +341,11 @@ describe('ExportMap', function () {
338341
// ['string form', { 'typescript-eslint-parser': '.ts' }],
339342
];
340343

341-
if (semver.satisfies(eslintPkg.version, '>5.0.0')) {
344+
if (semver.satisfies(eslintPkg.version, '>5')) {
342345
configs.push(['array form', { '@typescript-eslint/parser': ['.ts', '.tsx'] }]);
343346
}
344347

345-
if (semver.satisfies(eslintPkg.version, '<6.0.0')) {
348+
if (semver.satisfies(eslintPkg.version, '<6')) {
346349
configs.push(['array form', { 'typescript-eslint-parser': ['.ts', '.tsx'] }]);
347350
}
348351

@@ -358,8 +361,12 @@ describe('ExportMap', function () {
358361
let imports;
359362
before('load imports', function () {
360363
this.timeout(20000); // takes a long time :shrug:
364+
sinon.spy(tsConfigLoader, 'tsConfigLoader');
361365
imports = ExportMap.get('./typescript.ts', context);
362366
});
367+
after('clear spies', function () {
368+
tsConfigLoader.tsConfigLoader.restore();
369+
});
363370

364371
it('returns something for a TypeScript file', function () {
365372
expect(imports).to.exist;
@@ -388,9 +395,38 @@ describe('ExportMap', function () {
388395
it('has exported abstract class', function () {
389396
expect(imports.has('Bar')).to.be.true;
390397
});
398+
399+
it('should cache tsconfig until tsconfigRootDir parser option changes', function () {
400+
const customContext = Object.assign(
401+
{},
402+
context,
403+
{
404+
parserOptions: {
405+
tsconfigRootDir: null,
406+
},
407+
},
408+
);
409+
expect(tsConfigLoader.tsConfigLoader.callCount).to.equal(0);
410+
ExportMap.parse('./baz.ts', 'export const baz = 5', customContext);
411+
expect(tsConfigLoader.tsConfigLoader.callCount).to.equal(1);
412+
ExportMap.parse('./baz.ts', 'export const baz = 5', customContext);
413+
expect(tsConfigLoader.tsConfigLoader.callCount).to.equal(1);
414+
415+
const differentContext = Object.assign(
416+
{},
417+
context,
418+
{
419+
parserOptions: {
420+
tsconfigRootDir: process.cwd(),
421+
},
422+
},
423+
);
424+
425+
ExportMap.parse('./baz.ts', 'export const baz = 5', differentContext);
426+
expect(tsConfigLoader.tsConfigLoader.callCount).to.equal(2);
427+
});
391428
});
392429
});
393-
394430
});
395431

396432
// todo: move to utils

0 commit comments

Comments
 (0)