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 import/no-cycle perf regression #1944

Merged
merged 1 commit into from Jan 22, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -13,6 +13,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`no-extraneous-dependencies`]: Add package.json cache ([#1948], thanks @fa93hws)
- [`prefer-default-export`]: handle empty array destructuring ([#1965], thanks @ljharb)
- [`no-unused-modules`]: make type imports mark a module as used (fixes #1924) ([#1974], thanks [@cherryblossom000])
- [`import/no-cycle`]: fix perf regression ([#1944], thanks [@Blasz])

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

[#1974]: https://github.com/benmosher/eslint-plugin-import/pull/1974
[#1944]: https://github.com/benmosher/eslint-plugin-import/pull/1944
[#1924]: https://github.com/benmosher/eslint-plugin-import/issues/1924
[#1965]: https://github.com/benmosher/eslint-plugin-import/issues/1965
[#1948]: https://github.com/benmosher/eslint-plugin-import/pull/1948
Expand Down Expand Up @@ -1293,3 +1295,4 @@ for info on changes for earlier releases.
[@straub]: https://github.com/straub
[@andreubotella]: https://github.com/andreubotella
[@cherryblossom000]: https://github.com/cherryblossom000
[@Blasz]: https://github.com/Blasz
27 changes: 22 additions & 5 deletions src/ExportMap.js
Expand Up @@ -22,6 +22,7 @@ let parseConfigFileTextToJson;
const log = debug('eslint-plugin-import:ExportMap');

const exportCache = new Map();
const tsConfigCache = new Map();

export default class ExportMap {
constructor(path) {
Expand Down Expand Up @@ -438,9 +439,11 @@ ExportMap.parse = function (path, content, context) {

const source = makeSourceCode(content, ast);

function isEsModuleInterop() {
function readTsConfig() {
const tsConfigInfo = tsConfigLoader({
cwd: context.parserOptions && context.parserOptions.tsconfigRootDir || process.cwd(),
cwd:
(context.parserOptions && context.parserOptions.tsconfigRootDir) ||
process.cwd(),
getEnv: (key) => process.env[key],
});
try {
Expand All @@ -450,12 +453,26 @@ ExportMap.parse = function (path, content, context) {
// this is because projects not using TypeScript won't have typescript installed
({ parseConfigFileTextToJson } = require('typescript'));
}
const tsConfig = parseConfigFileTextToJson(tsConfigInfo.tsConfigPath, jsonText).config;
return tsConfig.compilerOptions.esModuleInterop;
return parseConfigFileTextToJson(tsConfigInfo.tsConfigPath, jsonText).config;
}
} catch (e) {
return false;
// Catch any errors
}

return null;
}

function isEsModuleInterop() {
const cacheKey = hashObject({
tsconfigRootDir: context.parserOptions && context.parserOptions.tsconfigRootDir,
}).digest('hex');
let tsConfig = tsConfigCache.get(cacheKey);
if (typeof tsConfig === 'undefined') {
tsConfig = readTsConfig();
tsConfigCache.set(cacheKey, tsConfig);
}

return tsConfig !== null ? tsConfig.compilerOptions.esModuleInterop : false;
}

ast.body.forEach(function (n) {
Expand Down
3 changes: 3 additions & 0 deletions tests/files/typescript-export-assign-property.ts
@@ -0,0 +1,3 @@
const AnalyticsNode = { Analytics: {} };

export = AnalyticsNode.Analytics;
44 changes: 40 additions & 4 deletions tests/src/core/getExports.js
@@ -1,6 +1,8 @@
import { expect } from 'chai';
import semver from 'semver';
import sinon from 'sinon';
import eslintPkg from 'eslint/package.json';
import * as tsConfigLoader from 'tsconfig-paths/lib/tsconfig-loader';
import ExportMap from '../../../src/ExportMap';

import * as fs from 'fs';
Expand Down Expand Up @@ -51,7 +53,8 @@ describe('ExportMap', function () {
const differentSettings = Object.assign(
{},
fakeContext,
{ parserPath: 'espree' });
{ parserPath: 'espree' },
);

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

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

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

Expand All @@ -358,8 +361,12 @@ describe('ExportMap', function () {
let imports;
before('load imports', function () {
this.timeout(20000); // takes a long time :shrug:
sinon.spy(tsConfigLoader, 'tsConfigLoader');
imports = ExportMap.get('./typescript.ts', context);
});
after('clear spies', function () {
tsConfigLoader.tsConfigLoader.restore();
});

it('returns something for a TypeScript file', function () {
expect(imports).to.exist;
Expand Down Expand Up @@ -388,9 +395,38 @@ describe('ExportMap', function () {
it('has exported abstract class', function () {
expect(imports.has('Bar')).to.be.true;
});

it('should cache tsconfig until tsconfigRootDir parser option changes', function () {
const customContext = Object.assign(
{},
context,
{
parserOptions: {
tsconfigRootDir: null,
},
},
);
expect(tsConfigLoader.tsConfigLoader.callCount).to.equal(0);
ExportMap.parse('./baz.ts', 'export const baz = 5', customContext);
expect(tsConfigLoader.tsConfigLoader.callCount).to.equal(1);
ExportMap.parse('./baz.ts', 'export const baz = 5', customContext);
expect(tsConfigLoader.tsConfigLoader.callCount).to.equal(1);

const differentContext = Object.assign(
{},
context,
{
parserOptions: {
tsconfigRootDir: process.cwd(),
},
},
);

ExportMap.parse('./baz.ts', 'export const baz = 5', differentContext);
expect(tsConfigLoader.tsConfigLoader.callCount).to.equal(2);
});
});
});

});

// todo: move to utils
Expand Down