From fe3121a2d74db1c2178d1ab189ef59b80c5b90c4 Mon Sep 17 00:00:00 2001 From: Sukka Date: Thu, 11 Jul 2024 18:11:41 +0800 Subject: [PATCH] perf: make `ExportMap` util and `no-cycle` rule faster (#109) --- .changeset/heavy-kangaroos-relax.md | 5 + .changeset/wicked-mangos-suffer.md | 5 + src/rules/no-cycle.ts | 11 +- src/utils/export-map.ts | 232 ++++++++++++++-------------- src/utils/ignore.ts | 13 +- src/utils/import-type.ts | 5 - src/utils/lazy-value.ts | 32 ++++ src/utils/module-cache.ts | 6 +- src/utils/resolve.ts | 5 +- 9 files changed, 178 insertions(+), 136 deletions(-) create mode 100644 .changeset/heavy-kangaroos-relax.md create mode 100644 .changeset/wicked-mangos-suffer.md diff --git a/.changeset/heavy-kangaroos-relax.md b/.changeset/heavy-kangaroos-relax.md new file mode 100644 index 000000000..5ef3e4430 --- /dev/null +++ b/.changeset/heavy-kangaroos-relax.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-import-x": patch +--- + +Make `eslint-plugin-import-x` overall faster by refactoring the `ExportMap` util diff --git a/.changeset/wicked-mangos-suffer.md b/.changeset/wicked-mangos-suffer.md new file mode 100644 index 000000000..a2c3d55e8 --- /dev/null +++ b/.changeset/wicked-mangos-suffer.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-import-x": patch +--- + +Make `no-cycle` rule faster diff --git a/src/rules/no-cycle.ts b/src/rules/no-cycle.ts index 58a0a20e1..8090fe45b 100644 --- a/src/rules/no-cycle.ts +++ b/src/rules/no-cycle.ts @@ -12,11 +12,11 @@ import { resolve, } from '../utils' -type Options = ModuleOptions & { +type Options = { allowUnsafeDynamicCyclicDependency?: boolean ignoreExternal?: boolean maxDepth?: number -} +} & ModuleOptions type MessageId = 'cycle' @@ -83,9 +83,10 @@ export = createRule<[Options?], MessageId>({ ? options.maxDepth : Number.POSITIVE_INFINITY - const ignoreModule = (name: string) => - options.ignoreExternal && - isExternalModule(name, resolve(name, context)!, context) + const ignoreModule = options.ignoreExternal + ? (name: string) => + isExternalModule(name, resolve(name, context)!, context) + : () => false return { ...moduleVisitor(function checkSourceValue(sourceNode, importer) { diff --git a/src/utils/export-map.ts b/src/utils/export-map.ts index 58f42b6ba..2153458ef 100644 --- a/src/utils/export-map.ts +++ b/src/utils/export-map.ts @@ -21,8 +21,8 @@ import type { } from '../types' import { getValue } from './get-value' -import { hashObject } from './hash' import { hasValidExtension, ignore } from './ignore' +import { lazy, defineLazyProperty } from './lazy-value' import { parse } from './parse' import { relative, resolve } from './resolve' import { isMaybeUnambiguousModule, isUnambiguousModule } from './unambiguous' @@ -56,26 +56,43 @@ export type ModuleImport = { declarations: Set } +const declTypes = new Set([ + 'VariableDeclaration', + 'ClassDeclaration', + 'TSDeclareFunction', + 'TSEnumDeclaration', + 'TSTypeAliasDeclaration', + 'TSInterfaceDeclaration', + 'TSAbstractClassDeclaration', + 'TSModuleDeclaration', +]) + export class ExportMap { static for(context: ChildContext) { - const { path: filepath } = context - - const cacheKey = context.cacheKey || hashObject(context).digest('hex') + const filepath = context.path + const cacheKey = context.cacheKey let exportMap = exportCache.get(cacheKey) - // return cached ignore - if (exportMap === null) { - return null - } + const stats = lazy(() => fs.statSync(filepath)) - const stats = fs.statSync(context.path) - if ( - exportMap != null && // date equality check - exportMap.mtime.valueOf() - stats.mtime.valueOf() === 0 - ) { - return exportMap + if (exportCache.has(cacheKey)) { + const exportMap = exportCache.get(cacheKey) + + // return cached ignore + if (exportMap === null) { + return null + } + + // check if the file has been modified since cached exportmap generation + if ( + exportMap != null && + exportMap.mtime - stats().mtime.valueOf() === 0 + ) { + return exportMap + } + + // future: check content equality? } - // future: check content equality? // check valid extensions first if (!hasValidExtension(filepath, context)) { @@ -84,7 +101,7 @@ export class ExportMap { } // check for and cache ignore - if (ignore(filepath, context)) { + if (ignore(filepath, context, true)) { log('ignored path due to ignore settings:', filepath) exportCache.set(cacheKey, null) return null @@ -103,13 +120,13 @@ export class ExportMap { exportMap = ExportMap.parse(filepath, content, context) // ambiguous modules return null - if (exportMap == null) { + if (exportMap === null) { log('ignored path due to ambiguous parse:', filepath) exportCache.set(cacheKey, null) return null } - exportMap.mtime = stats.mtime + exportMap.mtime = stats().mtime.valueOf() exportCache.set(cacheKey, exportMap) @@ -127,7 +144,7 @@ export class ExportMap { static parse(filepath: string, content: string, context: ChildContext) { const m = new ExportMap(filepath) - const isEsModuleInteropTrue = isEsModuleInterop() + const isEsModuleInteropTrue = lazy(isEsModuleInterop) let ast: TSESTree.Program let visitorKeys: TSESLint.SourceCode.VisitorKeys | null @@ -181,8 +198,9 @@ export class ExportMap { }, }) - const unambiguouslyESM = isUnambiguousModule(ast) - if (!unambiguouslyESM && !hasDynamicImports) { + const unambiguouslyESM = lazy(() => isUnambiguousModule(ast)) + + if (!hasDynamicImports && !unambiguouslyESM()) { return null } @@ -195,25 +213,6 @@ export class ExportMap { docStyleParsers[style] = availableDocStyleParsers[style] } - // attempt to collect module doc - if (ast.comments) { - ast.comments.some(c => { - if (c.type !== 'Block') { - return false - } - try { - const doc = doctrine.parse(c.value, { unwrap: true }) - if (doc.tags.some(t => t.title === 'module')) { - m.doc = doc - return true - } - } catch { - /* ignore */ - } - return false - }) - } - const namespaces = new Map() function remotePath(value: string) { @@ -394,18 +393,18 @@ export class ExportMap { return getter } - const source = makeSourceCode(content, ast) + const source = new SourceCode({ text: content, ast: ast as AST.Program }) function isEsModuleInterop() { const parserOptions = context.parserOptions || {} let tsconfigRootDir = parserOptions.tsconfigRootDir const project = parserOptions.project - const cacheKey = hashObject({ - tsconfigRootDir, - project, - }).digest('hex') - let tsConfig = tsconfigCache.get(cacheKey) - if (tsConfig === undefined) { + const cacheKey = stableHash({ tsconfigRootDir, project }) + let tsConfig: TsConfigJsonResolved | null + + if (tsconfigCache.has(cacheKey)) { + tsConfig = tsconfigCache.get(cacheKey)! + } else { tsconfigRootDir = tsconfigRootDir || process.cwd() let tsconfigResult if (project) { @@ -427,9 +426,7 @@ export class ExportMap { tsconfigCache.set(cacheKey, tsConfig) } - return tsConfig && tsConfig.compilerOptions - ? tsConfig.compilerOptions.esModuleInterop - : false + return tsConfig?.compilerOptions?.esModuleInterop ?? false } for (const n of ast.body) { @@ -516,7 +513,7 @@ export class ExportMap { } const exports = ['TSExportAssignment'] - if (isEsModuleInteropTrue) { + if (isEsModuleInteropTrue()) { exports.push('TSNamespaceExportDeclaration') } @@ -537,17 +534,6 @@ export class ExportMap { n.expression.id.name))) || null - const declTypes = new Set([ - 'VariableDeclaration', - 'ClassDeclaration', - 'TSDeclareFunction', - 'TSEnumDeclaration', - 'TSTypeAliasDeclaration', - 'TSInterfaceDeclaration', - 'TSAbstractClassDeclaration', - 'TSModuleDeclaration', - ]) - const getRoot = ( node: TSESTree.TSQualifiedName, ): TSESTree.Identifier => { @@ -580,7 +566,7 @@ export class ExportMap { } if ( - isEsModuleInteropTrue && // esModuleInterop is on in tsconfig + isEsModuleInteropTrue() && // esModuleInterop is on in tsconfig !m.namespace.has('default') // and default isn't added already ) { m.namespace.set('default', {}) // add default export @@ -652,17 +638,42 @@ export class ExportMap { } } + // attempt to collect module doc + defineLazyProperty(m, 'doc', () => { + if (ast.comments) { + for (let i = 0, len = ast.comments.length; i < len; i++) { + const c = ast.comments[i] + if (c.type !== 'Block') { + continue + } + try { + const doc = doctrine.parse(c.value, { unwrap: true }) + if (doc.tags.some(t => t.title === 'module')) { + return doc + } + } catch { + /* ignore */ + } + } + } + }) + if ( - isEsModuleInteropTrue && // esModuleInterop is on in tsconfig + isEsModuleInteropTrue() && // esModuleInterop is on in tsconfig m.namespace.size > 0 && // anything is exported !m.namespace.has('default') // and default isn't added already ) { m.namespace.set('default', {}) // add default export } - if (unambiguouslyESM) { - m.parseGoal = 'Module' - } + const prevParseGoal = m.parseGoal + defineLazyProperty(m, 'parseGoal', () => { + if (prevParseGoal !== 'Module' && unambiguouslyESM()) { + return 'Module' + } + return prevParseGoal + }) + return m } @@ -693,9 +704,9 @@ export class ExportMap { declare visitorKeys: TSESLint.SourceCode.VisitorKeys | null - private declare mtime: Date + private declare mtime: number - declare doc: Annotation + declare doc: Annotation | undefined constructor(public path: string) {} @@ -914,41 +925,43 @@ function captureDoc( ...nodes: Array ) { const metadata: { - doc?: Annotation + doc?: Annotation | undefined } = {} - // 'some' short-circuits on first 'true' - nodes.some(n => { - if (!n) { - return false - } - - try { - let leadingComments: TSESTree.Comment[] | undefined - - // n.leadingComments is legacy `attachComments` behavior - if ('leadingComments' in n && Array.isArray(n.leadingComments)) { - leadingComments = n.leadingComments as TSESTree.Comment[] - } else if (n.range) { - leadingComments = ( - source as unknown as TSESLint.SourceCode - ).getCommentsBefore(n) + defineLazyProperty(metadata, 'doc', () => { + for (let i = 0, len = nodes.length; i < len; i++) { + const n = nodes[i] + if (!n) { + continue } - if (!leadingComments || leadingComments.length === 0) { - return false - } + try { + let leadingComments: TSESTree.Comment[] | undefined - for (const parser of Object.values(docStyleParsers)) { - const doc = parser(leadingComments) - if (doc) { - metadata.doc = doc + // n.leadingComments is legacy `attachComments` behavior + if ('leadingComments' in n && Array.isArray(n.leadingComments)) { + leadingComments = n.leadingComments as TSESTree.Comment[] + } else if (n.range) { + leadingComments = ( + source as unknown as TSESLint.SourceCode + ).getCommentsBefore(n) } - } - return true - } catch { - return false + if (!leadingComments || leadingComments.length === 0) { + continue + } + + for (const parser of Object.values(docStyleParsers)) { + const doc = parser(leadingComments) + if (doc) { + return doc + } + } + + return + } catch { + continue + } } }) @@ -964,22 +977,20 @@ const availableDocStyleParsers = { * parse JSDoc from leading comments */ function captureJsDoc(comments: TSESTree.Comment[]) { - let doc: Annotation | undefined - // capture XSDoc - for (const comment of comments) { + + for (let i = comments.length - 1; i >= 0; i--) { + const comment = comments[i] // skip non-block comments if (comment.type !== 'Block') { continue } try { - doc = doctrine.parse(comment.value, { unwrap: true }) + return doctrine.parse(comment.value, { unwrap: true }) } catch { /* don't care, for now? maybe add to `errors?` */ } } - - return doc } /** @@ -1090,7 +1101,7 @@ function childContext( const { settings, parserOptions, parserPath, languageOptions } = context return { - cacheKey: makeContextCacheKey(context) + String(path), + cacheKey: makeContextCacheKey(context) + path, settings, parserOptions, parserPath, @@ -1121,16 +1132,3 @@ function makeContextCacheKey(context: RuleContext | ChildContext) { return hash } - -/** - * sometimes legacy support isn't _that_ hard... right? - */ -function makeSourceCode(text: string, ast: TSESTree.Program) { - if (SourceCode.length > 1) { - // ESLint 3 - return new SourceCode(text, ast as AST.Program) - } - - // ESLint 4+ - return new SourceCode({ text, ast: ast as AST.Program }) -} diff --git a/src/utils/ignore.ts b/src/utils/ignore.ts index dea41c9b8..e96575ee8 100644 --- a/src/utils/ignore.ts +++ b/src/utils/ignore.ts @@ -45,9 +45,15 @@ export function getFileExtensions(settings: PluginSettings) { return exts } -export function ignore(filepath: string, context: ChildContext | RuleContext) { +// In ExportMap.for, ignore() is called after hasValidExtension() check. +// Add an argument to skip the check +export function ignore( + filepath: string, + context: ChildContext | RuleContext, + skipExtensionCheck = false, +) { // check extension whitelist first (cheap) - if (!hasValidExtension(filepath, context)) { + if (!skipExtensionCheck && !hasValidExtension(filepath, context)) { return true } @@ -57,7 +63,8 @@ export function ignore(filepath: string, context: ChildContext | RuleContext) { return false } - for (const ignoreString of ignoreStrings) { + for (let i = 0, len = ignoreStrings.length; i < len; i++) { + const ignoreString = ignoreStrings[i] const regex = new RegExp(ignoreString) if (regex.test(filepath)) { log(`ignoring ${filepath}, matched pattern /${ignoreString}/`) diff --git a/src/utils/import-type.ts b/src/utils/import-type.ts index 37554cfb1..0c791a1be 100644 --- a/src/utils/import-type.ts +++ b/src/utils/import-type.ts @@ -43,11 +43,6 @@ export function isExternalModule( modulePath: string, context: RuleContext, ) { - if (arguments.length < 3) { - throw new TypeError( - 'isExternalModule: name, path, and context are all required', - ) - } return ( (isModule(name) || isScoped(name)) && typeTest(name, context, modulePath) === 'external' diff --git a/src/utils/lazy-value.ts b/src/utils/lazy-value.ts index 3686f09f4..a7f0a2639 100644 --- a/src/utils/lazy-value.ts +++ b/src/utils/lazy-value.ts @@ -16,3 +16,35 @@ export const lazy = (cb: () => T): LazyValue => { } export type LazyValue = () => Readonly + +export function defineLazyProperty< + ObjectType, + PropertyNameType extends string, + PropertyValueType, +>( + object: ObjectType, + propertyName: PropertyNameType, + valueGetter: () => PropertyValueType, +) { + const define = (value: PropertyValueType) => + Object.defineProperty(object, propertyName, { + value, + enumerable: true, + writable: true, + }) + + Object.defineProperty(object, propertyName, { + configurable: true, + enumerable: true, + get() { + const result = valueGetter() + define(result) + return result + }, + set(value) { + define(value) + }, + }) + + return object +} diff --git a/src/utils/module-cache.ts b/src/utils/module-cache.ts index d26653d3a..a7ddfbc5e 100644 --- a/src/utils/module-cache.ts +++ b/src/utils/module-cache.ts @@ -45,10 +45,8 @@ export class ModuleCache { // parse infinity if ( - (['∞', 'Infinity'] as const).includes( - // @ts-expect-error - TS can't narrow properly from `includes` - cacheSettings.lifetime, - ) + typeof cacheSettings.lifetime === 'string' && + (['∞', 'Infinity'] as const).includes(cacheSettings.lifetime) ) { cacheSettings.lifetime = Number.POSITIVE_INFINITY } diff --git a/src/utils/resolve.ts b/src/utils/resolve.ts index 096486b90..2132be837 100644 --- a/src/utils/resolve.ts +++ b/src/utils/resolve.ts @@ -2,6 +2,8 @@ import fs from 'node:fs' import { createRequire } from 'node:module' import path from 'node:path' +import stableHash from 'stable-hash' + import type { Arrayable, ImportResolver, @@ -10,7 +12,6 @@ import type { RuleContext, } from '../types' -import { hashObject } from './hash' import { ModuleCache } from './module-cache' import { pkgDir } from './pkg-dir' @@ -139,7 +140,7 @@ function fullResolve( const sourceDir = path.dirname(sourceFile) if (prevSettings !== settings) { - memoizedHash = hashObject(settings).digest('hex') + memoizedHash = stableHash(settings) prevSettings = settings }