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

perf: make ExportMap util and no-cycle rule faster #109

Merged
merged 18 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/rules/no-cycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import {
resolve,
} from '../utils'

type Options = ModuleOptions & {
type Options = {
allowUnsafeDynamicCyclicDependency?: boolean
ignoreExternal?: boolean
maxDepth?: number
}
} & ModuleOptions

type MessageId = 'cycle'

Expand Down Expand Up @@ -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) {
Expand Down
145 changes: 76 additions & 69 deletions src/utils/export-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
} 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'
Expand Down Expand Up @@ -58,24 +58,30 @@

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.valueOf() - stats().mtime.valueOf() === 0
) {
return exportMap
}

// future: check content equality?
}
// future: check content equality?

// check valid extensions first
if (!hasValidExtension(filepath, context)) {
Expand All @@ -84,7 +90,7 @@
}

// 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
Expand All @@ -103,13 +109,13 @@
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

exportCache.set(cacheKey, exportMap)

Expand All @@ -127,12 +133,12 @@

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
try {
;({ ast, visitorKeys } = parse(filepath, content, context))
; ({ ast, visitorKeys } = parse(filepath, content, context))

Check failure on line 141 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on macos-latest

Delete `·`

Check failure on line 141 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest

Delete `·`

Check failure on line 141 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest

Delete `·`
} catch (error) {
m.errors.push(error as ParseError)
return m // can't continue
Expand Down Expand Up @@ -181,9 +187,10 @@
},
})

const unambiguouslyESM = isUnambiguousModule(ast)
if (!unambiguouslyESM && !hasDynamicImports) {
return null
const unambiguouslyESM = lazy(() => isUnambiguousModule(ast))

if (!hasDynamicImports && !unambiguouslyESM()) {
return null;

Check failure on line 193 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on macos-latest

Delete `;`

Check failure on line 193 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest

Delete `;`

Check failure on line 193 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest

Delete `;`
}

const docStyles = (context.settings &&
Expand All @@ -195,25 +202,6 @@
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</* identifier */ string, /* source */ string>()

function remotePath(value: string) {
Expand Down Expand Up @@ -400,12 +388,12 @@
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) {
Expand All @@ -427,9 +415,7 @@
tsconfigCache.set(cacheKey, tsConfig)
}

return tsConfig && tsConfig.compilerOptions
? tsConfig.compilerOptions.esModuleInterop
: false
return tsConfig?.compilerOptions?.esModuleInterop ?? false
}

for (const n of ast.body) {
Expand Down Expand Up @@ -516,7 +502,7 @@
}

const exports = ['TSExportAssignment']
if (isEsModuleInteropTrue) {
if (isEsModuleInteropTrue()) {
exports.push('TSNamespaceExportDeclaration')
}

Expand All @@ -525,17 +511,17 @@
const exportedName =
n.type === 'TSNamespaceExportDeclaration'
? (
n.id ||
// @ts-expect-error - legacy parser type
n.name
).name
n.id ||

Check failure on line 514 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on macos-latest

Insert `··`

Check failure on line 514 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest

Insert `··`

Check failure on line 514 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest

Insert `··`
// @ts-expect-error - legacy parser type

Check failure on line 515 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on macos-latest

Insert `··`

Check failure on line 515 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest

Insert `··`

Check failure on line 515 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest

Insert `··`
n.name

Check failure on line 516 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on macos-latest

Replace `··············` with `················`

Check failure on line 516 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest

Replace `··············` with `················`

Check failure on line 516 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest

Replace `··············` with `················`
).name

Check failure on line 517 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on macos-latest

Insert `··`

Check failure on line 517 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest

Insert `··`

Check failure on line 517 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest

Insert `··`
: ('expression' in n &&
n.expression &&
(('name' in n.expression && n.expression.name) ||
('id' in n.expression &&
n.expression.id &&
n.expression.id.name))) ||
null
n.expression &&

Check failure on line 519 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on macos-latest

Insert `··`

Check failure on line 519 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest

Insert `··`

Check failure on line 519 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest

Insert `··`
(('name' in n.expression && n.expression.name) ||

Check failure on line 520 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on macos-latest

Insert `··`

Check failure on line 520 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest

Insert `··`

Check failure on line 520 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest

Insert `··`
('id' in n.expression &&

Check failure on line 521 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on macos-latest

Insert `··`

Check failure on line 521 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest

Insert `··`

Check failure on line 521 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest

Insert `··`
n.expression.id &&

Check failure on line 522 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on macos-latest

Replace `··················` with `····················`

Check failure on line 522 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest

Replace `··················` with `····················`

Check failure on line 522 in src/utils/export-map.ts

View workflow job for this annotation

GitHub Actions / Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest

Replace `··················` with `····················`
n.expression.id.name))) ||
null

const declTypes = new Set([
'VariableDeclaration',
Expand Down Expand Up @@ -565,7 +551,7 @@
('name' in node.id
? node.id.name === exportedName
: 'left' in node.id &&
getRoot(node.id).name === exportedName)) ||
getRoot(node.id).name === exportedName)) ||
('declarations' in node &&
node.declarations.find(
d => 'name' in d.id && d.id.name === exportedName,
Expand All @@ -580,7 +566,7 @@
}

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
Expand Down Expand Up @@ -652,15 +638,36 @@
}
}

// 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
break

Check warning

Code scanning / CodeQL

Unreachable statement Warning

This statement is unreachable.
}
} 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) {
if (unambiguouslyESM()) {
m.parseGoal = 'Module'
}
return m
Expand Down Expand Up @@ -695,9 +702,9 @@

private declare mtime: Date

declare doc: Annotation
declare doc: Annotation | undefined

constructor(public path: string) {}
constructor(public path: string) { }

get hasDefault() {
return this.get('default') != null
Expand Down Expand Up @@ -1090,7 +1097,7 @@
const { settings, parserOptions, parserPath, languageOptions } = context

return {
cacheKey: makeContextCacheKey(context) + String(path),
cacheKey: makeContextCacheKey(context) + path,
settings,
parserOptions,
parserPath,
Expand Down
13 changes: 10 additions & 3 deletions src/utils/ignore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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}/`)
Expand Down
5 changes: 0 additions & 5 deletions src/utils/import-type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
27 changes: 27 additions & 0 deletions src/utils/lazy-value.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,30 @@ export const lazy = <T>(cb: () => T): LazyValue<T> => {
}

export type LazyValue<T> = () => Readonly<T>

export function defineLazyProperty<
ObjectType extends Record<string, any>,
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;
}
6 changes: 2 additions & 4 deletions src/utils/module-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading
Loading