diff --git a/.changeset/always-install-before-build.md b/.changeset/always-install-before-build.md new file mode 100644 index 0000000..ba39aa8 --- /dev/null +++ b/.changeset/always-install-before-build.md @@ -0,0 +1,7 @@ +--- +"playground-cli": patch +--- + +`dot deploy` and `dot build` now run `pnpm install` (or the project's package manager equivalent) before every build, not just when `node_modules/` is missing. A stale `node_modules/` left over from a branch switch or a lockfile bump used to slip past the missing-folder guard and produce opaque Vite/Rollup errors like `"X is not exported by ..."`; the only fix was to re-run `pnpm install` by hand. The install step is idempotent (~1s when nothing has changed), so the happy path is essentially unaffected. + +Also surfaces more of the failing build's output in the CLI error message (40 lines instead of 10), so when a build does fail the actual error line — not just the trailing stack trace — makes it into the rendered output. And the same error no longer renders twice in the deploy TUI: the per-section row marks which step failed with `✕`, and the bottom `deploy failed` row carries the message once. diff --git a/src/commands/deploy/DeployScreen.tsx b/src/commands/deploy/DeployScreen.tsx index 367bf33..684529f 100644 --- a/src/commands/deploy/DeployScreen.tsx +++ b/src/commands/deploy/DeployScreen.tsx @@ -997,7 +997,6 @@ function RunningStage({ @@ -1030,7 +1029,6 @@ function ContractsSectionView({ state }: { state: ContractsSectionState }) { {state.contracts.length > 0 && ( @@ -1072,7 +1070,6 @@ function FrontendSectionView({ state }: { state: FrontendSectionState }) { {running && state.latestLog && {truncate(state.latestLog, 120)}} diff --git a/src/utils/build/detect.test.ts b/src/utils/build/detect.test.ts index a68c0c9..5e08969 100644 --- a/src/utils/build/detect.test.ts +++ b/src/utils/build/detect.test.ts @@ -28,7 +28,6 @@ function input(overrides: Partial = {}): DetectInput { packageJson: null, lockfiles: new Set(), configFiles: new Set(), - hasNodeModules: true, cargoToml: null, ...overrides, }; @@ -147,19 +146,23 @@ describe("detectBuildConfig", () => { }); describe("detectInstallConfig", () => { - it("returns null when node_modules is already present", () => { + it("returns an install command even when node_modules already exists (idempotent reconcile)", () => { + // A stale node_modules/ — e.g. after a branch switch — bypasses any + // missing-folder guard and lets the build run against package versions + // that don't match the lockfile. We install unconditionally so that + // case can't reach the build step. expect( detectInstallConfig( input({ packageJson: { dependencies: { vite: "^5.0.0" } }, - hasNodeModules: true, + lockfiles: new Set(["pnpm-lock.yaml"]), }), ), - ).toBeNull(); + ).toEqual({ cmd: "pnpm", args: ["install"], description: "pnpm install" }); }); it("returns null when package.json is missing", () => { - expect(detectInstallConfig(input({ hasNodeModules: false }))).toBeNull(); + expect(detectInstallConfig(input())).toBeNull(); }); it("returns null when the project declares no dependencies", () => { @@ -169,7 +172,6 @@ describe("detectInstallConfig", () => { detectInstallConfig( input({ packageJson: { scripts: { build: "echo hi" } }, - hasNodeModules: false, }), ), ).toBeNull(); @@ -180,7 +182,6 @@ describe("detectInstallConfig", () => { detectInstallConfig( input({ packageJson: { devDependencies: { vite: "^7.0.0" } }, - hasNodeModules: false, }), ), ).toEqual({ cmd: "npm", args: ["install"], description: "npm install" }); @@ -192,7 +193,6 @@ describe("detectInstallConfig", () => { input({ packageJson: { dependencies: { react: "^19.0.0" } }, lockfiles: new Set(["pnpm-lock.yaml"]), - hasNodeModules: false, }), ), ).toEqual({ cmd: "pnpm", args: ["install"], description: "pnpm install" }); @@ -202,7 +202,6 @@ describe("detectInstallConfig", () => { input({ packageJson: { dependencies: { react: "^19.0.0" } }, lockfiles: new Set(["bun.lockb"]), - hasNodeModules: false, }), ), ).toEqual({ cmd: "bun", args: ["install"], description: "bun install" }); @@ -212,7 +211,6 @@ describe("detectInstallConfig", () => { input({ packageJson: { dependencies: { react: "^19.0.0" } }, lockfiles: new Set(["yarn.lock"]), - hasNodeModules: false, }), ), ).toEqual({ cmd: "yarn", args: ["install"], description: "yarn install" }); diff --git a/src/utils/build/detect.ts b/src/utils/build/detect.ts index c39a519..e4c9456 100644 --- a/src/utils/build/detect.ts +++ b/src/utils/build/detect.ts @@ -61,8 +61,6 @@ export interface DetectInput { lockfiles: Set; /** Set of additional config-file basenames (e.g. vite.config.ts). */ configFiles: Set; - /** Whether a node_modules/ directory exists at the project root. */ - hasNodeModules: boolean; /** Raw Cargo.toml contents (used to gate the cdm contract flow). Null when absent. */ cargoToml: string | null; } @@ -172,17 +170,19 @@ export function detectContractsType(input: DetectInput): ContractsType | null { /** * Decide whether we need to run an install step before building. Returns the - * install command when the project has dependencies declared but no - * node_modules/ directory, otherwise null. + * install command whenever the project declares any dependencies, otherwise + * null. * - * Rationale: without this check, `dot build` for an uninstalled project falls - * through to `npx vite build` (or similar), which ephemerally downloads the - * framework binary but can't resolve the project's own `vite.config.ts` - * imports — yielding a confusing ERR_MODULE_NOT_FOUND deep in the config - * loader. Auto-installing first eliminates the footgun. + * We install unconditionally (not just when node_modules/ is missing) because + * a stale node_modules/ — e.g. after a branch switch or a teammate bumping the + * lockfile — bypasses the missing-folder guard and lets the build run against + * package versions that don't match the lockfile. The resulting "X is not + * exported by Y" error from Vite/Rollup is opaque and the user has no signal + * that the fix is a re-install. pnpm/yarn/npm are all idempotent when in sync + * (~1s no-op), so the cost is negligible. Skipping install for a deps-free + * package.json is still safe: there's nothing to install. */ export function detectInstallConfig(input: DetectInput): InstallConfig | null { - if (input.hasNodeModules) return null; const pkg = input.packageJson; if (!pkg) return null; const depCount = diff --git a/src/utils/build/runner.ts b/src/utils/build/runner.ts index ce1aed4..8d68447 100644 --- a/src/utils/build/runner.ts +++ b/src/utils/build/runner.ts @@ -76,7 +76,6 @@ export function loadDetectInput(projectDir: string): DetectInput { packageJson, lockfiles, configFiles, - hasNodeModules: existsSync(join(root, "node_modules")), cargoToml, }; } diff --git a/src/utils/deploy/run.test.ts b/src/utils/deploy/run.test.ts index 577988d..31a367e 100644 --- a/src/utils/deploy/run.test.ts +++ b/src/utils/deploy/run.test.ts @@ -62,7 +62,7 @@ const { packageJson: { scripts: { build: "vite build" } }, lockfiles: new Set(), configFiles: new Set(), - hasNodeModules: true, + cargoToml: null, })), detectContractsTypeMock: vi.fn<() => "foundry" | "hardhat" | "cdm" | null>(() => null), runContractsPhaseMock: vi.fn< diff --git a/src/utils/process.test.ts b/src/utils/process.test.ts index cb27d3e..53fedc3 100644 --- a/src/utils/process.test.ts +++ b/src/utils/process.test.ts @@ -87,9 +87,9 @@ describe("runStreamed", () => { ).rejects.toThrow(/\(no output\)/); }); - it("caps the captured tail and reports only the LAST 10 lines on failure (not the first)", async () => { + it("caps the captured tail and reports only the LAST 40 lines on failure (not the first)", async () => { // Generate 60 numbered lines, then exit 1. Tail buffer holds the last - // 50; the error message shows the last 10 of those — so 51..60. + // 40, which is what the error message reports — so 21..60. try { await runStreamed({ cmd: "/bin/sh", @@ -99,15 +99,50 @@ describe("runStreamed", () => { expect.fail("expected rejection"); } catch (err) { const msg = (err as Error).message; - // Last 10 lines must appear. - for (let i = 51; i <= 60; i++) { + // Last 40 lines must appear. + for (let i = 21; i <= 60; i++) { expect(msg).toContain(`line-${i}`); } // Anything earlier must NOT: confirms we're taking the tail, not - // the head, and that the slice is capped at 10. + // the head, and that the buffer is capped at 40. expect(msg).not.toContain("line-1\n"); - expect(msg).not.toContain("line-50\n"); - expect(msg).not.toContain("line-10\n"); + expect(msg).not.toContain("line-20\n"); + } + }); + + it("preserves a Vite/Rollup-style error message that precedes a long stack trace", async () => { + // Realistic shape: vite prints the build banner, a single descriptive + // error line, a code snippet, and finally a ~12-frame stack trace. + // With the older 10-line snippet the actual error was pushed off the + // window by the trailing trace; the wider window keeps it visible. + const script = [ + "echo 'vite v7.3.2 building client environment for production...'", + "echo 'transforming...'", + "echo '✓ 1544 modules transformed.'", + "echo '✗ Build failed in 843ms'", + "echo 'error during build:'", + `echo ' src/utils/contracts.ts (40:9): \"createContractRuntimeFromClient\" is not exported by node_modules/@parity/product-sdk-contracts/dist/index.js'`, + "echo 'file: /tmp/contracts.ts:40:9'", + "echo '38: import type { PolkadotSigner } from \"polkadot-api\";'", + "echo '39: import { keccak256 } from \"@parity/product-sdk-utils\";'", + "echo '40: import { createContractRuntimeFromClient } from \"@parity/product-sdk-contracts\";'", + "echo ' ^'", + "echo '41: import { paseo_asset_hub } from \"@parity/product-sdk-descriptors\";'", + "for i in $(seq 1 12); do echo ' at frame-'$i' (file:///.../rollup/dist/es/shared/node-entry.js:1234:5)'; done", + "exit 1", + ].join("; "); + + try { + await runStreamed({ + cmd: "/bin/sh", + args: ["-c", script], + description: "vite-like-failure", + }); + expect.fail("expected rejection"); + } catch (err) { + const msg = (err as Error).message; + expect(msg).toContain('"createContractRuntimeFromClient" is not exported'); + expect(msg).toContain("at frame-12"); } }); }); diff --git a/src/utils/process.ts b/src/utils/process.ts index a7c0f86..4884dad 100644 --- a/src/utils/process.ts +++ b/src/utils/process.ts @@ -34,8 +34,12 @@ export interface RunStreamedOptions { /** * Spawn a child process, stream every non-empty stdout/stderr line through - * `onData`, and resolve/reject based on exit code. Includes the last ~10 + * `onData`, and resolve/reject based on exit code. Includes the last ~40 * lines of output in the rejection message so failures are diagnosable. + * + * The window is 40 (not 10) because Vite/Rollup and many bundlers print the + * meaningful error first and then a 10–20 line stack trace; a smaller window + * keeps the trace and drops the message that explains what actually broke. */ export async function runStreamed(opts: RunStreamedOptions): Promise { const description = opts.description ?? `${opts.cmd} ${opts.args.join(" ")}`; @@ -49,7 +53,7 @@ export async function runStreamed(opts: RunStreamedOptions): Promise { }); const tail: string[] = []; - const MAX_TAIL = 50; + const MAX_TAIL = 40; const forward = (chunk: Buffer) => { for (const line of chunk.toString().split("\n")) { @@ -71,7 +75,7 @@ export async function runStreamed(opts: RunStreamedOptions): Promise { if (code === 0) { resolvePromise(); } else { - const snippet = tail.slice(-10).join("\n") || "(no output)"; + const snippet = tail.join("\n") || "(no output)"; rejectPromise( new Error( `${failurePrefix} (${description}) with exit code ${code}.\n${snippet}`,