Skip to content

Commit ec0a851

Browse files
authored
fix: properly collect node_modules when they're ESM and we're node>=16 (#9380)
1 parent ed8ea12 commit ec0a851

File tree

21 files changed

+167
-253
lines changed

21 files changed

+167
-253
lines changed

.changeset/chatty-wolves-live.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"app-builder-lib": patch
3+
"builder-util": patch
4+
"dmg-builder": patch
5+
---
6+
7+
fix: properly collect node_modules when they're ESM and we're node>=16

.github/workflows/test.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,8 @@ jobs:
243243
env:
244244
TEST_FILES: ${{ matrix.testFiles }}
245245
FORCE_COLOR: 1
246+
TEST_SEQUENTIAL: true
247+
DEBUG: electron-builder
246248

247249
test-mac-26:
248250
runs-on: macos-26

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"generate-changeset": "pnpm changeset",
2121
"generate-schema": "typescript-json-schema packages/app-builder-lib/tsconfig-scheme.json Configuration --out packages/app-builder-lib/scheme.json --noExtraProps --useTypeOfKeyword --strictNullChecks --required && node ./scripts/fix-schema.js",
2222
"generate-all": "pnpm generate-schema && pnpm prettier",
23-
"ci:test": "vitest run --no-update --allowOnly",
23+
"ci:test": "vitest run",
2424
"ci:version": "pnpm i && pnpm changelog && changeset version && node scripts/update-package-version-export.js && pnpm compile && pnpm generate-all && git add .",
2525
"ci:publish": "pnpm i && pnpm compile && pnpm publish -r --tag next && changeset tag",
2626
"docs:prebuild": "docker build -t mkdocs-dockerfile -f mkdocs-dockerfile . ",
@@ -52,6 +52,7 @@
5252
"eslint-config-prettier": "^9.1.0",
5353
"eslint-plugin-prettier": "^5.2.1",
5454
"fs-extra": "10.1.0",
55+
"is-ci": "^4.1.0",
5556
"prettier": "3.3.3",
5657
"ts-node": "^10.9.2",
5758
"typedoc": "^0.26.7",

packages/app-builder-lib/src/asar/unpackDetector.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ export function detectUnpackedDirs(fileSet: ResolvedFileSet, autoUnpackDirs: Set
3939
continue
4040
}
4141

42-
if (log.isDebugEnabled) {
43-
log.debug({ file: stat.moduleFullFilePath, reason: "contains executable code" }, "not packed into asar archive")
44-
}
42+
log.debug({ file: stat.moduleFullFilePath, reason: "contains executable code" }, "not packed into asar archive")
4543
autoUnpackDirs.add(stat.moduleRootPath)
4644
}
4745
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ export function getCollectorByPackageManager(pm: PM, rootDir: string, tempDirMan
2525
case PM.BUN:
2626
return new BunNodeModulesCollector(rootDir, tempDirManager)
2727
case PM.NPM:
28-
default:
2928
return new NpmNodeModulesCollector(rootDir, tempDirManager)
3029
}
3130
}
@@ -72,7 +71,7 @@ async function findWorkspaceRoot(pm: PM, cwd: string): Promise<string | undefine
7271

7372
switch (pm) {
7473
case PM.PNPM:
75-
command = { command: "pnpm", args: ["root", "-w"] }
74+
command = { command: "pnpm", args: ["--workspace-root", "exec", "pwd"] }
7675
break
7776

7877
case PM.YARN_BERRY:

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

Lines changed: 73 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { exists, log, retry, TmpDir } from "builder-util"
2-
import { spawn as spawnProcess } from "child_process"
2+
import * as childProcess from "child_process"
33
import { CancellationToken } from "builder-util-runtime"
44
import * as fs from "fs-extra"
55
import { createWriteStream, readJson } from "fs-extra"
@@ -9,6 +9,7 @@ import { hoist, type HoisterResult, type HoisterTree } from "./hoist"
99
import { createModuleCache, type ModuleCache } from "./moduleCache"
1010
import { getPackageManagerCommand, PM } from "./packageManager"
1111
import type { Dependency, DependencyGraph, NodeModuleInfo, PackageJson } from "./types"
12+
import { fileURLToPath, pathToFileURL } from "node:url"
1213

1314
export abstract class NodeModulesCollector<ProdDepType extends Dependency<ProdDepType, OptionalDepType>, OptionalDepType> {
1415
private nodeModules: NodeModuleInfo[] = []
@@ -17,21 +18,33 @@ export abstract class NodeModulesCollector<ProdDepType extends Dependency<ProdDe
1718
protected pkgJsonCache: Map<string, string> = new Map()
1819
protected memoResolvedModules = new Map<string, Promise<string | null>>()
1920

21+
private importMetaResolve = new Lazy(async () => {
22+
try {
23+
// Node >= 16 builtin ESM-aware resolver
24+
const packageName = "import-meta-resolve"
25+
const imported = await import(packageName)
26+
return imported.resolve
27+
} catch {
28+
return null
29+
}
30+
})
31+
2032
// Unified cache for all file system and module operations
2133
protected cache: ModuleCache = createModuleCache()
2234

2335
protected isHoisted = new Lazy<boolean>(async () => {
24-
const command = getPackageManagerCommand(this.installOptions.manager)
36+
const { manager } = this.installOptions
37+
const command = getPackageManagerCommand(manager)
2538

2639
const config = (await this.asyncExec(command, ["config", "list"])).stdout
2740
if (config == null) {
28-
log.debug({ manager: this.installOptions.manager }, "unable to determine if node_modules are hoisted: no config output. falling back to hoisted mode")
41+
log.debug({ manager }, "unable to determine if node_modules are hoisted: no config output. falling back to hoisted mode")
2942
return false
3043
}
3144
const lines = Object.fromEntries(config.split("\n").map(line => line.split("=").map(s => s.trim())))
3245

3346
if (lines["node-linker"] === "hoisted") {
34-
log.debug({ manager: this.installOptions.manager }, "node_modules are hoisted")
47+
log.debug({ manager }, "node_modules are hoisted")
3548
return true
3649
}
3750

@@ -57,7 +70,7 @@ export abstract class NodeModulesCollector<ProdDepType extends Dependency<ProdDe
5770

5871
await this.collectAllDependencies(tree, packageName)
5972

60-
const realTree: ProdDepType = await this.getTreeFromWorkspaces(tree)
73+
const realTree: ProdDepType = await this.getTreeFromWorkspaces(tree, packageName)
6174
await this.extractProductionDependencyGraph(realTree, packageName)
6275

6376
if (cancellationToken.cancelled) {
@@ -184,27 +197,57 @@ export abstract class NodeModulesCollector<ProdDepType extends Dependency<ProdDe
184197
* Resolves a package to its filesystem location using Node.js module resolution.
185198
* Returns the directory containing the package, not the package.json path.
186199
*/
187-
protected resolvePackageDir = (packageName: string, fromDir: string): string | null => {
200+
protected async resolvePackageDir(packageName: string, fromDir: string): Promise<string | null> {
188201
const cacheKey = `${packageName}::${fromDir}`
189-
190-
// Check memoization cache
191202
if (this.cache.requireResolve.has(cacheKey)) {
192203
return this.cache.requireResolve.get(cacheKey)!
193204
}
194205

195-
try {
196-
// require.resolve finds the main entry point, so we look for package.json instead
197-
const packageJsonPath = require.resolve(`${packageName}/package.json`, {
198-
paths: [fromDir, this.rootDir],
199-
})
200-
const result = path.dirname(packageJsonPath)
201-
this.cache.requireResolve.set(cacheKey, result)
202-
return result
203-
} catch (error: any) {
204-
log.warn({ packageName, fromDir, error: error.message }, "could not resolve package")
206+
const resolveEntry = async () => {
207+
// 1) Try Node ESM resolver (handles exports reliably)
208+
if (await this.importMetaResolve.value) {
209+
try {
210+
const url = (await this.importMetaResolve.value)(packageName, pathToFileURL(fromDir).href)
211+
return url.startsWith("file://") ? fileURLToPath(url) : null
212+
} catch {
213+
// ignore
214+
}
215+
}
216+
217+
// 2) Fallback: require.resolve (CJS, old packages)
218+
try {
219+
return require.resolve(packageName, { paths: [fromDir, this.rootDir] })
220+
} catch {
221+
// ignore
222+
}
223+
224+
return null
225+
}
226+
227+
const entry = await resolveEntry()
228+
if (!entry) {
205229
this.cache.requireResolve.set(cacheKey, null)
206230
return null
207231
}
232+
233+
// 3) Walk upward until you find a package.json
234+
let dir = path.dirname(entry)
235+
while (true) {
236+
const pkgFile = path.join(dir, "package.json")
237+
if (await exists(pkgFile)) {
238+
this.cache.requireResolve.set(cacheKey, dir)
239+
return dir
240+
}
241+
242+
const parent = path.dirname(dir)
243+
if (parent === dir) {
244+
break // root reached
245+
}
246+
dir = parent
247+
}
248+
249+
this.cache.requireResolve.set(cacheKey, null)
250+
return null
208251
}
209252

210253
protected requireMemoized(pkgPath: string): PackageJson {
@@ -244,20 +287,20 @@ export abstract class NodeModulesCollector<ProdDepType extends Dependency<ProdDe
244287
return { name, version }
245288
}
246289

247-
protected async getTreeFromWorkspaces(tree: ProdDepType): Promise<ProdDepType> {
248-
if (tree.workspaces && tree.dependencies) {
249-
const packageJson = await this.appPkgJson.value
250-
const dependencyName = packageJson.name
290+
protected async getTreeFromWorkspaces(tree: ProdDepType, packageName: string): Promise<ProdDepType> {
291+
if (!(tree.workspaces && tree.dependencies)) {
292+
return tree
293+
}
251294

252-
for (const [key, value] of Object.entries(tree.dependencies)) {
253-
if (key === dependencyName) {
254-
log.debug({ key, path: value.path }, "returning workspace tree for root dependency")
255-
return value
256-
}
295+
if (tree.dependencies?.[packageName]) {
296+
const { name, path, dependencies } = tree.dependencies[packageName]
297+
log.debug({ name, path, dependencies: JSON.stringify(dependencies) }, "pruning root app/self package from workspace tree")
298+
for (const [name, pkg] of Object.entries(dependencies ?? {})) {
299+
tree.dependencies[name] = pkg
257300
}
301+
delete tree.dependencies[packageName]
258302
}
259-
260-
return tree
303+
return Promise.resolve(tree)
261304
}
262305

263306
private transformToHoisterTree(obj: DependencyGraph, key: string, nodes: Map<string, HoisterTree> = new Map()): HoisterTree {
@@ -352,7 +395,7 @@ export abstract class NodeModulesCollector<ProdDepType extends Dependency<ProdDe
352395
await new Promise<void>((resolve, reject) => {
353396
const outStream = createWriteStream(tempOutputFile)
354397

355-
const child = spawnProcess(command, args, {
398+
const child = childProcess.spawn(command, args, {
356399
cwd,
357400
shell: false, // required to prevent console logs polution from shell profile loading when `true`
358401
})

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,11 @@ export class NpmNodeModulesCollector extends NodeModulesCollector<NpmDependency,
1616

1717
protected async getDependenciesTree(pm: PM): Promise<NpmDependency> {
1818
try {
19-
// force NPM collection as Yarn Berry extends this class and PnP is not supported directly
2019
return await super.getDependenciesTree(pm)
2120
} catch (error: any) {
22-
log.info({ pm, parser: PM.NPM, error: error.message }, "unable to process dependency tree, falling back to using manual node_modules traversal")
21+
log.info({ pm: this.installOptions.manager, parser: PM.NPM, error: error.message }, "unable to process dependency tree, falling back to using manual node_modules traversal")
2322
}
24-
// node_modules linker fallback. (Slower due to system ops, so we only use it as a fallback)
23+
// node_modules linker fallback. (Slower due to system ops, so we only use it as a fallback) [e.g. corepack env will not allow npm CLI to extract tree]
2524
return this.buildNodeModulesTreeManually(this.rootDir)
2625
}
2726

@@ -110,13 +109,21 @@ export class NpmNodeModulesCollector extends NodeModulesCollector<NpmDependency,
110109
for (const [depName, depVersion] of Object.entries(allProdDepNames)) {
111110
try {
112111
// Resolve the dependency using Node.js module resolution from this package's directory
113-
const depPath = this.resolvePackageDir(depName, packageDir)
112+
const depPath = await this.resolvePackageDir(depName, packageDir)
114113

115114
if (!depPath) {
116115
log.warn({ package: pkg.name, dependency: depName, version: depVersion }, "dependency not found, skipping")
117116
continue
118117
}
119118

119+
const resolvedDepPath = await this.resolvePath(depPath)
120+
121+
// Skip if this dependency resolves to the base directory or any parent we're already processing
122+
if (resolvedDepPath === resolvedPackageDir || resolvedDepPath === (await this.resolvePath(baseDir))) {
123+
log.debug({ package: pkg.name, dependency: depName, resolvedPath: resolvedDepPath }, "skipping self-referential dependency")
124+
continue
125+
}
126+
120127
log.debug({ package: pkg.name, dependency: depName, resolvedPath: depPath }, "processing production dependency")
121128

122129
// Recursively build the dependency tree for this dependency

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ import * as path from "path"
44
import * as which from "which"
55

66
export enum PM {
7-
NPM = "npm",
8-
YARN = "yarn",
97
PNPM = "pnpm",
8+
YARN = "yarn",
109
YARN_BERRY = "yarn-berry",
1110
BUN = "bun",
11+
NPM = "npm",
1212
}
1313

1414
// Cache for resolved paths

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { log } from "builder-util"
1+
import { isEmptyOrSpaces, log } from "builder-util"
22
import * as path from "path"
33
import { NodeModulesCollector } from "./nodeModulesCollector"
44
import { PM } from "./packageManager"
@@ -19,8 +19,14 @@ export class PnpmNodeModulesCollector extends NodeModulesCollector<PnpmDependenc
1919
return
2020
}
2121

22-
const getProductionDependencies = async (tree: PnpmDependency): Promise<{ prodDeps: Record<string, string>; optionalDependencies: Record<string, string> } | null> => {
23-
const p = path.normalize(this.resolvePackageDir(tree.name, tree.path) ?? (await this.resolvePath(tree.path)))
22+
const getProductionDependencies = async (depTree: PnpmDependency): Promise<{ prodDeps: Record<string, string>; optionalDependencies: Record<string, string> } | null> => {
23+
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}`)
27+
}
28+
29+
const p = path.normalize((await this.resolvePackageDir(packageName, depTree.path)) ?? (await this.resolvePath(depTree.path)))
2430
const pkgJsonPath = path.join(p, "package.json")
2531

2632
let packageJson: PackageJson
@@ -33,7 +39,8 @@ export class PnpmNodeModulesCollector extends NodeModulesCollector<PnpmDependenc
3339
return { prodDeps: { ...packageJson.dependencies, ...packageJson.optionalDependencies }, optionalDependencies: { ...packageJson.optionalDependencies } }
3440
}
3541

36-
const json = tree.name === dependencyId ? null : await getProductionDependencies(tree)
42+
const packageName = tree.name || tree.from
43+
const json = packageName === dependencyId ? null : await getProductionDependencies(tree)
3744
const prodDependencies = json?.prodDeps ?? { ...(tree.dependencies || {}), ...(tree.optionalDependencies || {}) }
3845
if (prodDependencies == null) {
3946
this.productionGraph[dependencyId] = { dependencies: [] }

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export class YarnBerryNodeModulesCollector extends NpmNodeModulesCollector {
4747
const output = await this.asyncExec("yarn", ["config", "--json"], rootDir)
4848

4949
if (!output.stdout) {
50+
log.debug(null, "Yarn config returned no output, assuming default Yarn v1 behavior (hoisted, non-PnP)")
5051
return {
5152
yarnVersion,
5253
nodeLinker,

0 commit comments

Comments
 (0)