fix(desktop): refuse the build when ANY declared non-optional dep is missing
Widen the salvaged guard from a hand-maintained four-package floor to the class it stands for: every `dependencies` + `devDependencies` entry in the desktop workspace manifest. Live probe on this box: a tree holding vite, katex, electron and electron-builder but missing `@rolldown/plugin-babel` still passed the floor-only guard, and `vite build` died loading `vite.config.ts` after `prebuild` had already run. The floor stays as an unconditional fallback for an unreadable manifest; optionalDependencies are skipped because npm legitimately omits them (get-windows). Five new vitest cases (12 total); the two class tests fail when the manifest union is removed. Refs #86443.
This commit is contained in:
@@ -31,13 +31,25 @@ import { isMain } from "./utils.mjs"
|
||||
// others, which is how an incomplete install reached `vite build` and died on
|
||||
// an unresolved `katex/dist/katex.min.css` with no hint that the install — not
|
||||
// the source — was at fault (#86443).
|
||||
//
|
||||
// These four are the documented floor — always checked, even when the app's
|
||||
// package.json cannot be read. The full class is wider: EVERY non-optional
|
||||
// package the workspace manifest declares is something the build may import
|
||||
// (`vite.config.ts` pulls `@rolldown/plugin-babel`, `@vitejs/plugin-react`,
|
||||
// `@tailwindcss/vite`; `bundle-electron-main.mjs` pulls `esbuild`; the renderer
|
||||
// imports the rest). A hand-maintained list drifts the moment a new import
|
||||
// lands, so `checkRootInstall` unions the floor with the manifest's declared
|
||||
// `dependencies` + `devDependencies` — a partial install is refused whichever
|
||||
// package it happened to drop. `optionalDependencies` are excluded by design:
|
||||
// npm legitimately skips them (platform-gated natives like `get-windows`).
|
||||
const BUILD_CRITICAL_PACKAGES = ["vite", "katex", "electron", "electron-builder"]
|
||||
export { BUILD_CRITICAL_PACKAGES }
|
||||
|
||||
// Resolve the way Node's own lookup does — walk `node_modules` upward — rather
|
||||
// than through `require.resolve`. A package whose `exports` map does not expose
|
||||
// `./package.json` is not resolvable by path even when correctly installed, and
|
||||
// that must not read as "missing".
|
||||
// that must not read as "missing". Scoped names (`@scope/name`) are a nested
|
||||
// directory under `node_modules`, which `join` handles.
|
||||
function packageIsInstalled(name, fromDir) {
|
||||
let dir = fromDir
|
||||
for (;;) {
|
||||
@@ -48,10 +60,27 @@ function packageIsInstalled(name, fromDir) {
|
||||
}
|
||||
}
|
||||
|
||||
// Every package the workspace manifest at `appDir` declares as required
|
||||
// (`dependencies` + `devDependencies`; never `optionalDependencies`). An
|
||||
// unreadable or malformed manifest yields [] — the floor still applies, and
|
||||
// the build's own manifest read fails loudly on its own.
|
||||
export function requiredPackages(appDir) {
|
||||
try {
|
||||
const manifest = JSON.parse(readFileSync(join(appDir, "package.json"), "utf8"))
|
||||
return [
|
||||
...Object.keys(manifest.dependencies ?? {}),
|
||||
...Object.keys(manifest.devDependencies ?? {}),
|
||||
]
|
||||
} catch {
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
// Pure check — returns { ok: true } or { ok: false, error: "..." }.
|
||||
// Kept side-effect-free so it can be unit tested without spawning a process.
|
||||
export function checkRootInstall(appDir, rootDir) {
|
||||
const missing = BUILD_CRITICAL_PACKAGES.filter(pkg => !packageIsInstalled(pkg, appDir))
|
||||
const wanted = [...new Set([...BUILD_CRITICAL_PACKAGES, ...requiredPackages(appDir)])]
|
||||
const missing = wanted.filter(pkg => !packageIsInstalled(pkg, appDir))
|
||||
if (missing.length > 0) {
|
||||
return {
|
||||
ok: false,
|
||||
|
||||
@@ -4,15 +4,17 @@ import os from 'node:os'
|
||||
import path from 'node:path'
|
||||
import { test } from 'vitest'
|
||||
|
||||
import { BUILD_CRITICAL_PACKAGES as BUILD_CRITICAL, checkRootInstall } from '../scripts/assert-root-install.mjs'
|
||||
import { BUILD_CRITICAL_PACKAGES as BUILD_CRITICAL, checkRootInstall, requiredPackages } from '../scripts/assert-root-install.mjs'
|
||||
|
||||
// Build a throwaway repo shaped like this one: an app workspace whose
|
||||
// dependencies are hoisted to the repo root, which is what the guard walks.
|
||||
function makeTree({ rootPackages = BUILD_CRITICAL, react = '19.2.7', reactDom = '19.2.7' } = {}) {
|
||||
// `manifest` is merged into the app's package.json so tests can declare
|
||||
// dependencies the guard is expected to read.
|
||||
function makeTree({ rootPackages = BUILD_CRITICAL, react = '19.2.7', reactDom = '19.2.7', manifest = {} } = {}) {
|
||||
const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-assert-root-'))
|
||||
const appDir = path.join(tempRoot, 'apps', 'desktop')
|
||||
fs.mkdirSync(appDir, { recursive: true })
|
||||
fs.writeFileSync(path.join(appDir, 'package.json'), JSON.stringify({ name: 'desktop' }), 'utf8')
|
||||
fs.writeFileSync(path.join(appDir, 'package.json'), JSON.stringify({ name: 'desktop', ...manifest }), 'utf8')
|
||||
|
||||
const writePackage = (name, version) => {
|
||||
const dir = path.join(tempRoot, 'node_modules', name)
|
||||
@@ -120,3 +122,76 @@ test('checkRootInstall accepts a package nested in the app workspace', () => {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
// The class, not the four instances: the floor list is what a partial install
|
||||
// has been *seen* to drop, but any declared non-optional package can be the one
|
||||
// missing next (`vite.config.ts` imports `@rolldown/plugin-babel`, which the
|
||||
// floor never named). The guard must read the manifest so the list cannot drift
|
||||
// behind a new import.
|
||||
test('checkRootInstall fails when a declared devDependency outside the floor is missing', () => {
|
||||
const { tempRoot, appDir } = makeTree({
|
||||
manifest: { devDependencies: { '@rolldown/plugin-babel': '1.0.0', esbuild: '1.0.0' } },
|
||||
rootPackages: [...BUILD_CRITICAL, 'esbuild']
|
||||
})
|
||||
try {
|
||||
const result = checkRootInstall(appDir, tempRoot)
|
||||
assert.equal(result.ok, false)
|
||||
assert.match(result.error, /@rolldown\/plugin-babel/)
|
||||
assert.doesNotMatch(result.error, /esbuild/)
|
||||
} finally {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('checkRootInstall fails when a declared runtime dependency is missing', () => {
|
||||
const { tempRoot, appDir } = makeTree({
|
||||
manifest: { dependencies: { '@vscode/codicons': '1.0.0' } }
|
||||
})
|
||||
try {
|
||||
const result = checkRootInstall(appDir, tempRoot)
|
||||
assert.equal(result.ok, false)
|
||||
assert.match(result.error, /@vscode\/codicons/)
|
||||
} finally {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
// npm skips optionalDependencies legitimately (platform-gated natives), so an
|
||||
// absent optional package is not a partial install.
|
||||
test('checkRootInstall ignores missing optionalDependencies', () => {
|
||||
const { tempRoot, appDir } = makeTree({
|
||||
manifest: { optionalDependencies: { 'get-windows': '9.3.0' } }
|
||||
})
|
||||
try {
|
||||
assert.deepEqual(checkRootInstall(appDir, tempRoot), { ok: true })
|
||||
} finally {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('checkRootInstall passes when every declared package is installed', () => {
|
||||
const { tempRoot, appDir } = makeTree({
|
||||
manifest: { dependencies: { '@scope/pkg': '1.0.0' }, devDependencies: { esbuild: '1.0.0' } },
|
||||
rootPackages: [...BUILD_CRITICAL, '@scope/pkg', 'esbuild']
|
||||
})
|
||||
try {
|
||||
assert.deepEqual(checkRootInstall(appDir, tempRoot), { ok: true })
|
||||
} finally {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
// The floor is unconditional: a manifest the guard cannot parse must not turn
|
||||
// the check off.
|
||||
test('checkRootInstall keeps the floor when the manifest is unreadable', () => {
|
||||
const { tempRoot, appDir } = makeTree({ rootPackages: ['vite'] })
|
||||
fs.writeFileSync(path.join(appDir, 'package.json'), '{not json', 'utf8')
|
||||
try {
|
||||
assert.deepEqual(requiredPackages(appDir), [])
|
||||
const result = checkRootInstall(appDir, tempRoot)
|
||||
assert.equal(result.ok, false)
|
||||
assert.match(result.error, /katex/)
|
||||
} finally {
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user