diff --git a/packages/blocks-react/src/type-surface.test.ts b/packages/blocks-react/src/type-surface.test.ts index 766caaa740..a25d94663f 100644 --- a/packages/blocks-react/src/type-surface.test.ts +++ b/packages/blocks-react/src/type-surface.test.ts @@ -30,8 +30,11 @@ * `turbo.json` makes `test` depend on `build` and names `dist/**` among its * inputs, so an edit anywhere in the declaration's own inputs — this package's * sources, the engine's, the bundler config — rebuilds and re-runs. Global - * setup adds one build before collection so a direct `vitest run` on a tree - * with no `dist` works at all. + * setup does NOT build: it refuses to start when the declarations are missing, + * naming the command to run, because a build launched from inside collection + * rebuilds this package's whole dependency tree while sibling suites are + * importing from it. So a direct `vitest run` on a tree with no `dist` stops + * with one legible error rather than racing. * * Neither covers `vitest --watch`, and deliberately so. Vitest selects a suite * from its MODULE GRAPH, which cannot see a `.d.ts` read off disk; making it @@ -176,7 +179,16 @@ function resolveDeclaration( ]) { if (existsSync(candidate)) return candidate; } - return undefined; + // A relative specifier inside an emitted declaration names another emitted + // declaration. Finding none means the artifact is INCOMPLETE — a build stopped + // after writing the entries and before their chunks — and returning + // `undefined` would walk a smaller graph, derive fewer required types, and + // read as a clean pass. Under-reporting the obligation is the one answer this + // suite must never give, so an unresolvable reference stops the run. + throw new Error( + `${specifier} is referenced by ${from} and no declaration for it exists. ` + + `These declarations are incomplete; rebuild before asserting against them.` + ); } /** diff --git a/packages/blocks-react/turbo.json b/packages/blocks-react/turbo.json index 62f53ce1b5..571ccfa5a8 100644 --- a/packages/blocks-react/turbo.json +++ b/packages/blocks-react/turbo.json @@ -11,18 +11,21 @@ // suite either finds no `dist` at all or reads declarations produced // before the source change. // - // This makes turbo do that build, in order, and cache it. It is not the - // only guarantee: `vitest.global-setup.ts` builds too, so the entry - // points turbo never sees are covered as well. + // This makes turbo do that build, in order, and cache it. It is the ONLY + // thing that builds them: `vitest.global-setup.ts` refuses to start when + // the declarations are missing rather than producing them, because a + // build launched from inside collection rebuilds this package's whole + // dependency tree while sibling suites are importing from it. "dependsOn": ["$TURBO_EXTENDS$", "build"], // `$TURBO_EXTENDS$` keeps the root inputs in sync rather than restating // them; `dist/**` is appended because the artifact these suites assert // against is a real input to them, and a cached pass that ignored it // would survive a change to what the bundler emits. - // `vitest.global-setup.ts` is named because it IS the build precondition - // these suites rely on: a change to it changes what they assert against, - // and the root inputs cover `vitest.config.ts` alone — so without it a - // cached pass survives an edit to the setup and the run never happens. + // `vitest.global-setup.ts` is named because it decides whether these + // suites run at all: a change to what it accepts changes whether a given + // tree is allowed to assert anything, and the root inputs cover + // `vitest.config.ts` alone — so without it a cached pass survives an edit + // to the setup and the run never happens. "inputs": [ "$TURBO_EXTENDS$", "dist/**", diff --git a/packages/blocks-react/vitest.global-setup.ts b/packages/blocks-react/vitest.global-setup.ts index 64cb4b6d0f..deab27ba86 100644 --- a/packages/blocks-react/vitest.global-setup.ts +++ b/packages/blocks-react/vitest.global-setup.ts @@ -1,71 +1,81 @@ -import { execFileSync } from "node:child_process"; -import { dirname, join } from "node:path"; +import { existsSync, readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; /** - * Build this package before any suite is COLLECTED. + * Refuse to start when this package's declarations are missing, rather than + * building them. * * Several suites here assert against the BUILT declarations rather than the * source, because the questions they ask — what an entry exports, what a - * consumer can resolve — have answers only in the emitted artifact. A `dist` - * that is absent fails them at import time and a `dist` that merely predates - * the source certifies a surface nobody has. + * consumer can resolve — have answers only in the emitted artifact. So a `dist` + * that is absent has to stop the run. * - * **Global setup, because collection is concurrent.** Sibling suites import - * this package while any one suite's own hook would still be building, so on a - * tree with no `dist` they fail to resolve before that build finishes. Only a - * stage that completes before collection starts covers them. + * **Stopping is all it does, and that is the whole design.** Freshness belongs + * to the build system: `turbo.json` makes `test` depend on this package's own + * `build` and names `dist/**` among its inputs, so an edit to anything the + * declarations are emitted from rebuilds and re-runs. `pnpm test`, `turbo run + * test` and CI all arrive through that edge with the artifact already current. + * What is checked here is the path that bypasses it — a direct `vitest`, or + * `pnpm --filter @nextlyhq/blocks-react test`, which runs the package script + * without turbo's graph. * - * It runs ONCE per process, which is the whole of what it guarantees: the - * declarations are current when the run begins. Keeping them current as inputs - * change is Turbo's job — `test` depends on `build` and names `dist/**` among - * its inputs — and a `--watch` session is explicitly outside both, because - * Vitest selects suites from a module graph that cannot see a `.d.ts` read off - * disk. + * Building here as well looked like belt-and-braces and was not. `turbo run + * build --filter=@nextlyhq/blocks-react` carries the `^build` edge, so it + * rebuilds this package's whole dependency tree — the engine, the adapters, + * `nextly` — and each of those bundles with `clean: true`, deleting its `dist` + * before emitting. Run from global setup, that happens DURING collection, while + * sibling packages' suites are already importing from those same directories. + * A concurrent run then fails in whichever package was mid-import, with a + * missing chunk in code nobody changed. One package's convenience cannot be + * bought by deleting another package's build output underneath it. * - * Unconditional, with no attempt to detect a build that already happened. The - * signals available say a Turbo task is running, never that it was one - * carrying this package's build edge, and a precondition derived from a nearby - * signal is wrong silently. Where the build did already run this is a cache - * hit costing milliseconds. + * **Absent is checked; STALE is not, and cannot usefully be.** A `dist` older + * than the source it describes would pass here and let the suites read an + * artifact that agrees with itself while the source has moved. Modification + * times cannot close that: Turbo restores a cached `dist` with the timestamp of + * the restore, so any later touch of a source file — a checkout, a branch + * switch, a formatter — makes the source newer than an artifact whose content is + * current. Measured on this package: a cache-restored `dist/index.d.ts` at + * `…57.173` against an unchanged `src/index.ts` at `…57.691`. A timestamp guard + * would refuse to run on a correct tree, which is a worse failure than the rare + * one it prevents, and deciding freshness exactly means re-implementing Turbo's + * hashing. Turbo stays the authority, through the `test` -> `build` edge. * - * Through Turbo rather than `tsup` directly: the declaration build resolves - * `@nextlyhq/blocks-engine`, so invoking the bundler on a tree where that - * dependency has not been built fails before any declarations exist. `turbo - * run build` carries the `^build` edge, so the dependency is built first. + * The entries are read from `package.json` rather than listed here, so this + * checks exactly what the suites resolve and cannot drift from it as entries + * are added. */ export default function setup(): void { - const repoRoot = join(dirname(fileURLToPath(import.meta.url)), "..", ".."); + const packageRoot = dirname(fileURLToPath(import.meta.url)); + const manifest = JSON.parse( + readFileSync(join(packageRoot, "package.json"), "utf8") + ) as { name?: string; exports?: Record }; + const name = manifest.name ?? "@nextlyhq/blocks-react"; + // Scoped through `turbo run` directly rather than the root `build` script, + // which already carries `--filter=./packages/*`. Turbo's filters are + // ADDITIVE, so `pnpm build --filter ` selects 23 package builds rather + // than this one's subtree — advice that would clean unrelated packages' `dist` + // is the behaviour this file exists to stop recommending. + const recovery = `pnpm exec turbo run build --filter=${name}`; - try { - execFileSync( - "pnpm", - ["exec", "turbo", "run", "build", "--filter=@nextlyhq/blocks-react"], - { - cwd: repoRoot, - stdio: "pipe", - // Windows resolves package-manager shims via the shell: Corepack - // installs `pnpm.cmd`, which cannot be launched directly. Matches how - // the CLI spawns a package manager in `cli/commands/add.ts`. - shell: process.platform === "win32", - } - ); - } catch (error) { - // Compiler diagnostics are the only thing that makes a failure here - // actionable, and a discarded stream reduces every cause to the same - // opaque non-zero exit. - const output = `${streamText(error, "stdout")}${streamText(error, "stderr")}`; + const declarations = Object.entries(manifest.exports ?? {}) + .map(([subpath, entry]) => ({ subpath, types: entry?.types })) + .filter( + (entry): entry is { subpath: string; types: string } => + typeof entry.types === "string" + ) + .map(entry => ({ ...entry, file: resolve(packageRoot, entry.types) })); + + const missing = declarations + .filter(entry => !existsSync(entry.file)) + .map(entry => `${entry.subpath} (${entry.types})`); + if (missing.length > 0) { throw new Error( - `building @nextlyhq/blocks-react failed:\n${output.trim()}` + `${name} has no built declarations for ${missing.join(", ")}.\n` + + `Several suites here assert against the emitted artifact, so there is ` + + `nothing for them to read.\n` + + `Run \`${recovery}\`, or use \`pnpm test\` / \`turbo run test\`, which build first.` ); } } - -/** `stdout`/`stderr` off a failed `execFileSync`, without assuming its shape. */ -function streamText(error: unknown, key: "stdout" | "stderr"): string { - if (typeof error !== "object" || error === null || !(key in error)) return ""; - const value: unknown = Reflect.get(error, key); - if (typeof value === "string") return value; - if (value instanceof Uint8Array) return Buffer.from(value).toString("utf8"); - return ""; -}