Skip to content

Commit 65eecac

Browse files
authored
fix: search for actual path in hoisted mode for pnpm since it can still return virtual paths via CLI (#9392)
1 parent 841a290 commit 65eecac

File tree

5 files changed

+1743
-67
lines changed

5 files changed

+1743
-67
lines changed

.changeset/tall-lions-cry.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"app-builder-lib": patch
3+
---
4+
5+
fix: returning actual path in hoisted mode for pnpm since it can still return virtual paths

packages/app-builder-lib/src/node-module-collector/nodeModulesCollector.ts

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,20 @@ export abstract class NodeModulesCollector<ProdDepType extends Dependency<ProdDe
172172
return this.cache.realPath.get(filePath)!
173173
}
174174

175+
protected requireMemoized(pkgPath: string): PackageJson {
176+
if (!this.cache.packageJson.has(pkgPath)) {
177+
this.cache.packageJson.set(pkgPath, require(pkgPath))
178+
}
179+
return this.cache.packageJson.get(pkgPath)!
180+
}
181+
182+
protected existsSyncMemoized(filePath: string): boolean {
183+
if (!this.cache.exists.has(filePath)) {
184+
this.cache.exists.set(filePath, fs.existsSync(filePath))
185+
}
186+
return this.cache.exists.get(filePath)!
187+
}
188+
175189
protected async resolvePath(filePath: string): Promise<string> {
176190
// Check if we've already resolved this path
177191
if (this.cache.realPath.has(filePath)) {
@@ -211,15 +225,17 @@ export abstract class NodeModulesCollector<ProdDepType extends Dependency<ProdDe
211225
try {
212226
const url = (await this.importMetaResolve.value)(packageName, pathToFileURL(fromDir).href)
213227
return url.startsWith("file://") ? fileURLToPath(url) : null
214-
} catch {
228+
} catch (error: any) {
229+
log.debug({ error: error.message }, "import-meta-resolve failed")
215230
// ignore
216231
}
217232
}
218233

219234
// 2) Fallback: require.resolve (CJS, old packages)
220235
try {
221236
return require.resolve(packageName, { paths: [fromDir, this.rootDir] })
222-
} catch {
237+
} catch (error: any) {
238+
log.debug({ error: error.message }, "require.resolve failed")
223239
// ignore
224240
}
225241

@@ -236,7 +252,7 @@ export abstract class NodeModulesCollector<ProdDepType extends Dependency<ProdDe
236252
let dir = path.dirname(entry)
237253
while (true) {
238254
const pkgFile = path.join(dir, "package.json")
239-
if (await exists(pkgFile)) {
255+
if (await this.existsMemoized(pkgFile)) {
240256
this.cache.requireResolve.set(cacheKey, dir)
241257
return dir
242258
}
@@ -252,20 +268,6 @@ export abstract class NodeModulesCollector<ProdDepType extends Dependency<ProdDe
252268
return null
253269
}
254270

255-
protected requireMemoized(pkgPath: string): PackageJson {
256-
if (!this.cache.packageJson.has(pkgPath)) {
257-
this.cache.packageJson.set(pkgPath, require(pkgPath))
258-
}
259-
return this.cache.packageJson.get(pkgPath)!
260-
}
261-
262-
protected existsSyncMemoized(filePath: string): boolean {
263-
if (!this.cache.exists.has(filePath)) {
264-
this.cache.exists.set(filePath, fs.existsSync(filePath))
265-
}
266-
return this.cache.exists.get(filePath)!
267-
}
268-
269271
protected cacheKey(pkg: ProdDepType): string {
270272
const rel = path.relative(this.rootDir, pkg.path)
271273
return `${pkg.name}::${pkg.version}::${rel ?? "."}`
@@ -296,7 +298,7 @@ export abstract class NodeModulesCollector<ProdDepType extends Dependency<ProdDe
296298

297299
if (tree.dependencies?.[packageName]) {
298300
const { name, path, dependencies } = tree.dependencies[packageName]
299-
log.debug({ name, path, dependencies: JSON.stringify(dependencies) }, "pruning root app/self package from workspace tree")
301+
log.debug({ name, path, dependencies: JSON.stringify(dependencies) }, "pruning root app/self reference from workspace tree")
300302
for (const [name, pkg] of Object.entries(dependencies ?? {})) {
301303
tree.dependencies[name] = pkg
302304
}

packages/app-builder-lib/src/node-module-collector/pnpmNodeModulesCollector.ts

Lines changed: 59 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -11,78 +11,95 @@ export class PnpmNodeModulesCollector extends NodeModulesCollector<PnpmDependenc
1111
}
1212

1313
protected getArgs(): string[] {
14-
return ["list", "--prod", "--json", "--depth", "Infinity"]
14+
return ["list", "--prod", "--json", "--depth", "Infinity", "--long"]
1515
}
1616

17-
protected async extractProductionDependencyGraph(tree: PnpmDependency, dependencyId: string) {
18-
if (this.productionGraph[dependencyId]) {
19-
return
20-
}
21-
22-
const getProductionDependencies = async (depTree: PnpmDependency): Promise<{ prodDeps: Record<string, string>; optionalDependencies: Record<string, string> } | null> => {
17+
private async resolveActualPath(depTree: PnpmDependency): Promise<string> {
18+
// If using hoisted mode, try to find the package at the hoisted location first
19+
if (await this.isHoisted.value) {
2320
const packageName = depTree.name || depTree.from
24-
if (isEmptyOrSpaces(packageName)) {
25-
log.error(depTree, `Cannot determine production dependencies for package with empty name`)
26-
throw new Error(`Cannot compute production dependencies for package with empty name: ${packageName}`)
21+
if (packageName) {
22+
const hoistedPath = path.join(this.rootDir, "node_modules", packageName)
23+
if (await this.existsMemoized(hoistedPath)) {
24+
return hoistedPath
25+
}
2726
}
27+
}
28+
// Fall back to the reported path (which might be the .pnpm store path)
29+
return depTree.path
30+
}
2831

29-
const p = path.normalize((await this.resolvePackageDir(packageName, depTree.path)) ?? (await this.resolvePath(depTree.path)))
30-
const pkgJsonPath = path.join(p, "package.json")
32+
private async getProductionDependencies(depTree: PnpmDependency): Promise<{ path: string; prodDeps: Record<string, string>; optionalDependencies: Record<string, string> }> {
33+
const packageName = depTree.name || depTree.from
34+
if (isEmptyOrSpaces(packageName)) {
35+
log.error(depTree, `Cannot determine production dependencies for package with empty name`)
36+
throw new Error(`Cannot compute production dependencies for package with empty name: ${packageName}`)
37+
}
3138

32-
let packageJson: PackageJson
33-
try {
34-
packageJson = this.requireMemoized(pkgJsonPath)
35-
} catch (error: any) {
36-
log.warn(null, `Failed to read package.json for ${p}: ${error.message}`)
37-
return null
38-
}
39-
return { prodDeps: { ...packageJson.dependencies, ...packageJson.optionalDependencies }, optionalDependencies: { ...packageJson.optionalDependencies } }
39+
const actualPath = await this.resolveActualPath(depTree)
40+
const resolvedLocalPath = await this.resolvePath(actualPath)
41+
const p = path.normalize(resolvedLocalPath)
42+
const pkgJsonPath = path.join(p, "package.json")
43+
44+
let packageJson: PackageJson
45+
try {
46+
packageJson = this.requireMemoized(pkgJsonPath)
47+
} catch (error: any) {
48+
log.warn(null, `Failed to read package.json for ${p}: ${error.message}`)
49+
return { path: p, prodDeps: {}, optionalDependencies: {} }
50+
}
51+
return { path: p, prodDeps: { ...packageJson.dependencies, ...packageJson.optionalDependencies }, optionalDependencies: { ...packageJson.optionalDependencies } }
52+
}
53+
54+
protected async extractProductionDependencyGraph(tree: PnpmDependency, dependencyId: string) {
55+
if (this.productionGraph[dependencyId]) {
56+
return
4057
}
4158

4259
const packageName = tree.name || tree.from
43-
const json = packageName === dependencyId ? null : await getProductionDependencies(tree)
60+
const json = packageName === dependencyId ? null : await this.getProductionDependencies(tree)
4461
const prodDependencies = json?.prodDeps ?? { ...(tree.dependencies || {}), ...(tree.optionalDependencies || {}) }
4562
if (prodDependencies == null) {
4663
this.productionGraph[dependencyId] = { dependencies: [] }
4764
return
4865
}
4966
const deps = { ...(tree.dependencies || {}), ...(tree.optionalDependencies || {}) }
5067
this.productionGraph[dependencyId] = { dependencies: [] }
51-
const depPromises = Object.entries(deps)
52-
.filter(([packageName, dependency]) => {
53-
// First check if it's in production dependencies
54-
if (!prodDependencies[packageName]) {
55-
return false
56-
}
68+
const depPromises = Object.entries(deps).map(async ([packageName, dependency]) => {
69+
// First check if it's in production dependencies
70+
if (!prodDependencies[packageName]) {
71+
return undefined
72+
}
5773

58-
// Then check if optional dependency path exists (using memoized version)
59-
if (json?.optionalDependencies?.[packageName] && !this.existsSyncMemoized(dependency.path)) {
60-
log.debug(null, `Optional dependency ${packageName}@${dependency.version} path doesn't exist: ${dependency.path}`)
61-
return false
74+
// Then check if optional dependency path exists (using actual resolved path)
75+
if (json?.optionalDependencies?.[packageName]) {
76+
const actualPath = await this.resolveActualPath(dependency)
77+
if (!(await this.existsMemoized(actualPath))) {
78+
log.debug(null, `Optional dependency ${packageName}@${dependency.version} path doesn't exist: ${actualPath}`)
79+
return undefined
6280
}
81+
}
82+
const childDependencyId = this.packageVersionString(dependency)
83+
await this.extractProductionDependencyGraph(dependency, childDependencyId)
84+
return childDependencyId
85+
})
6386

64-
return true
65-
})
66-
.map(async ([, dependency]) => {
67-
const childDependencyId = this.packageVersionString(dependency)
68-
await this.extractProductionDependencyGraph(dependency, childDependencyId)
69-
return childDependencyId
70-
})
71-
72-
const dependencies = await Promise.all(depPromises)
87+
const dependencies = (await Promise.all(depPromises)).filter((id): id is string => id !== undefined)
7388
this.productionGraph[dependencyId] = { dependencies }
7489
}
7590

7691
protected async collectAllDependencies(tree: PnpmDependency) {
7792
// Collect regular dependencies
7893
for (const [key, value] of Object.entries(tree.dependencies || {})) {
79-
this.allDependencies.set(`${key}@${value.version}`, value)
94+
const json = await this.getProductionDependencies(value)
95+
this.allDependencies.set(`${key}@${value.version}`, { ...value, path: json.path })
8096
await this.collectAllDependencies(value)
8197
}
8298

8399
// Collect optional dependencies if they exist
84100
for (const [key, value] of Object.entries(tree.optionalDependencies || {})) {
85-
this.allDependencies.set(`${key}@${value.version}`, value)
101+
const json = await this.getProductionDependencies(value)
102+
this.allDependencies.set(`${key}@${value.version}`, { ...value, path: json.path })
86103
await this.collectAllDependencies(value)
87104
}
88105
}

packages/app-builder-lib/src/node-module-collector/yarnNodeModulesCollector.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ export class YarnNodeModulesCollector extends NodeModulesCollector<YarnDependenc
123123

124124
const rootPkgJson = await this.appPkgJson.value
125125
return Promise.resolve({
126-
name: rootPkgJson?.name || ".",
127-
version: rootPkgJson?.version || "unknown",
126+
name: rootPkgJson.name,
127+
version: rootPkgJson.version,
128128
path: this.rootDir,
129129
dependencies,
130130
workspaces: rootPkgJson?.workspaces,

0 commit comments

Comments
 (0)