-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix(core): improve buildExplicitTypeScriptDependnecies performance #33963
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
Changes from 2 commits
a6daf02
6ef7b46
27cee79
6658d01
87d1392
46fc4d0
02b5d9e
d4dc2d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,13 +22,15 @@ import { | |
| getRootTsConfigFileName, | ||
| resolveModuleByImport, | ||
| } from '../../utils/typescript'; | ||
| import { existsSync } from 'node:fs'; | ||
|
|
||
| /** | ||
| * The key is a combination of the package name and the workspace relative directory | ||
| * containing the file importing it e.g. `lodash__packages/my-lib`, the value is the | ||
| * resolved external node name from the project graph. | ||
| */ | ||
| type NpmResolutionCache = Map<string, string | null>; | ||
| type PackageJsonResolutionCache = Map<string, PackageJson | null>; | ||
|
|
||
| type PathPattern = { | ||
| pattern: string; | ||
|
|
@@ -44,6 +46,7 @@ type ParsedPatterns = { | |
| * Use a shared cache to avoid repeated npm package resolution work within the TargetProjectLocator. | ||
| */ | ||
| const defaultNpmResolutionCache: NpmResolutionCache = new Map(); | ||
| const defaultPackageJsonResolutionCache: PackageJsonResolutionCache = new Map(); | ||
|
|
||
| const experimentalNodeModules = new Set(['node:sqlite']); | ||
|
|
||
|
|
@@ -71,7 +74,8 @@ export class TargetProjectLocator { | |
| string, | ||
| ProjectGraphExternalNode | ||
| > = {}, | ||
| private readonly npmResolutionCache: NpmResolutionCache = defaultNpmResolutionCache | ||
| private readonly npmResolutionCache: NpmResolutionCache = defaultNpmResolutionCache, | ||
| private readonly packageJsonResolutionCache: PackageJsonResolutionCache = defaultPackageJsonResolutionCache | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did not review too carefully.. but just to sanity check me... This is OK for the daemon because everytime we calculate dependencies, we create a brand new project locator with no cache right? So this cache will not remain between different graph calculations?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, exactly. The cache is reused only during the single CLI command/daemon OP lifetime. |
||
| ) { | ||
| /** | ||
| * Only the npm external nodes should be included. | ||
|
|
@@ -211,7 +215,8 @@ export class TargetProjectLocator { | |
| // package.json refers to an external package, we do not match against the version found in there, we instead try and resolve the relevant package how node would | ||
| const externalPackageJson = this.readPackageJson( | ||
| packageName, | ||
| fullDirPath | ||
| fullDirPath, | ||
| workspaceRoot | ||
| ); | ||
| // The external package.json path might be not be resolvable, e.g. if a reference has been added to a project package.json, but the install command has not been run yet. | ||
| if (!externalPackageJson) { | ||
|
|
@@ -533,18 +538,26 @@ export class TargetProjectLocator { | |
| */ | ||
| private readPackageJson( | ||
| packageName: string, | ||
| relativeToDir: string | ||
| relativeToDir: string, | ||
| workspaceRoot: string | ||
| ): PackageJson | null { | ||
| // The package.json is directly resolvable | ||
| const packageJsonPath = resolveRelativeToDir( | ||
| join(packageName, 'package.json'), | ||
| relativeToDir | ||
| ); | ||
| if (packageJsonPath) { | ||
| if (this.packageJsonResolutionCache.has(packageJsonPath)) { | ||
| return this.packageJsonResolutionCache.get(packageJsonPath); | ||
| } | ||
| const parsedPackageJson = readJsonFile(packageJsonPath); | ||
|
|
||
| if (parsedPackageJson.name && parsedPackageJson.version) { | ||
| this.packageJsonResolutionCache.set(packageJsonPath, parsedPackageJson); | ||
| return parsedPackageJson; | ||
| } else { | ||
| this.packageJsonResolutionCache.set(packageJsonPath, null); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -556,14 +569,29 @@ export class TargetProjectLocator { | |
|
|
||
| while (dir !== dirname(dir)) { | ||
| const packageJsonPath = join(dir, 'package.json'); | ||
| try { | ||
| const parsedPackageJson = readJsonFile(packageJsonPath); | ||
| // Ensure the package.json contains the "name" and "version" fields | ||
| if (parsedPackageJson.name && parsedPackageJson.version) { | ||
| return parsedPackageJson; | ||
| if (this.packageJsonResolutionCache.has(packageJsonPath)) { | ||
| return this.packageJsonResolutionCache.get(packageJsonPath); | ||
| } | ||
| if (existsSync(packageJsonPath)) { | ||
meeroslav marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| try { | ||
| const parsedPackageJson = readJsonFile(packageJsonPath); | ||
| // Ensure the package.json contains the "name" and "version" fields | ||
| if (parsedPackageJson.name && parsedPackageJson.version) { | ||
| this.packageJsonResolutionCache.set( | ||
| packageJsonPath, | ||
| parsedPackageJson | ||
| ); | ||
| return parsedPackageJson; | ||
| } else { | ||
| this.packageJsonResolutionCache.set(packageJsonPath, null); | ||
| return null; | ||
| } | ||
| } catch { | ||
| // Package.json is invalid, keep traversing | ||
meeroslav marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| } catch { | ||
| // Package.json doesn't exist, keep traversing | ||
| } | ||
| if (dir === workspaceRoot) { | ||
| return null; | ||
meeroslav marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| dir = dirname(dir); | ||
meeroslav marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.