diff --git a/cli/commands/build/command-help.ts b/cli/commands/build/command-help.ts index a41c3ea06e..d6e6d3d4f4 100644 --- a/cli/commands/build/command-help.ts +++ b/cli/commands/build/command-help.ts @@ -44,6 +44,10 @@ export const buildHelp: CommandHelp = { description: "Select build preset (e.g. embedded)", }, ], + notes: [ + "--preset embedded emits a single bundle, so of the build flags it honours only -o/--output and build.outDir. Global flags such as --json, --verbose and --quiet are unaffected.", + "It rejects --dry-run, --split/--no-split, --compress/--no-compress, --prefetch, --ssg/--no-ssg, --include and --exclude rather than ignoring them.", + ], examples: [ "veryfront build", "veryfront build --output dist", diff --git a/cli/commands/build/command.test.ts b/cli/commands/build/command.test.ts index 5a53bd5576..66a554c2a8 100644 --- a/cli/commands/build/command.test.ts +++ b/cli/commands/build/command.test.ts @@ -264,3 +264,35 @@ describe("commands/build/command", () => { }); }); }); + +describe("cli/build resolveBuildOutputDir clearsOutputDir", () => { + // Raised in review on #3781. The guard exists because the production build + // removes its output directory first. The embedded preset only mkdir's and + // writes, so applying the guard there rejected `-o .` — a plausible call for + // a preset meant to be embedded in a host project — over a hazard that does + // not exist on that path. + it("rejects an output directory containing the project when the caller clears it", () => { + assertThrows( + () => resolveBuildOutputDir("/tmp/proj", "/tmp/proj", { build: {} }), + Error, + ); + }); + + it("allows the same directory when the caller only writes into it", () => { + assertEquals( + resolveBuildOutputDir("/tmp/proj", "/tmp/proj", { build: {} }, { + clearsOutputDir: false, + }), + "/tmp/proj", + ); + }); + + it("still honours build.outDir when the guard is opted out", () => { + assertEquals( + resolveBuildOutputDir("/tmp/proj", undefined, { build: { outDir: "custom" } }, { + clearsOutputDir: false, + }), + "/tmp/proj/custom", + ); + }); +}); diff --git a/cli/commands/build/command.ts b/cli/commands/build/command.ts index 0329c9c5db..76fdc0f7c1 100644 --- a/cli/commands/build/command.ts +++ b/cli/commands/build/command.ts @@ -84,9 +84,15 @@ export function resolveBuildOutputDir( projectDir: string, explicitOutputDir: string | undefined, config: Pick, + options: { clearsOutputDir?: boolean } = {}, ): string { const outputDir = explicitOutputDir ?? resolveConfiguredOutputDir(projectDir, config); - assertOutputDirExcludesProject(projectDir, outputDir, explicitOutputDir !== undefined); + // The guard below exists because the caller wipes the directory first. A + // caller that only writes into it is not dangerous, and rejecting it would + // block legitimate invocations — see the embedded preset. + if (options.clearsOutputDir !== false) { + assertOutputDirExcludesProject(projectDir, outputDir, explicitOutputDir !== undefined); + } return outputDir; } @@ -102,9 +108,12 @@ function resolveConfiguredOutputDir( /** * Refuse an output directory that is the project directory or an ancestor of it. * - * The build clears its output directory before writing, so an `outDir` of `.` - * or `..` would recursively delete the project's own source — or the workspace - * above it. That was unreachable while `build.outDir` was ignored; now that the + * The production build clears its output directory before writing + * (`build-setup.ts` removes it recursively), so an `outDir` of `.` or `..` + * would recursively delete the project's own source — or the workspace above + * it. This is why the check is opt-out via `clearsOutputDir`: a caller that + * only writes into the directory, like the embedded preset, carries no such + * risk and must not be blocked. That was unreachable while `build.outDir` was ignored; now that the * config value is honored, a stale compatibility-era config could reach it. * Failing loudly is the only safe answer: silently substituting `dist` would * reintroduce the ignored-configuration bug this change exists to fix. diff --git a/cli/commands/build/embedded-preset-flags.test.ts b/cli/commands/build/embedded-preset-flags.test.ts new file mode 100644 index 0000000000..7f7d3c0d96 --- /dev/null +++ b/cli/commands/build/embedded-preset-flags.test.ts @@ -0,0 +1,155 @@ +import "#veryfront/schemas/_test-setup.ts"; +import { assertEquals, assertRejects } from "#veryfront/testing/assert.ts"; +import { describe, it } from "#veryfront/testing/bdd.ts"; +import { join } from "#veryfront/compat/path/index.ts"; +import { withCwd } from "#veryfront/testing/cwd.ts"; +import { parseCliArgs } from "#cli/shared/args"; +import { assertEmbeddedPresetFlags, handleBuildCommand } from "./handler.ts"; + +async function exists(path: string): Promise { + try { + await Deno.stat(path); + return true; + } catch (error) { + if (error instanceof Deno.errors.NotFound) return false; + throw error; + } +} + +async function makeProject(prefix: string): Promise { + const projectDir = await Deno.makeTempDir({ prefix }); + await Deno.mkdir(join(projectDir, "app"), { recursive: true }); + await Deno.writeTextFile(join(projectDir, "app/page.mdx"), "# Home\n"); + return projectDir; +} + +describe("commands/build/handler embedded preset flags", () => { + it("does not write a bundle for a --dry-run embedded build", async () => { + const projectDir = await makeProject("vf-embedded-dry-run-"); + try { + await withCwd(projectDir, async () => { + await assertRejects( + () => + handleBuildCommand( + parseCliArgs(["build", "--preset", "embedded", "--dry-run"]), + ), + Error, + "--dry-run", + ); + }); + + assertEquals( + await exists(join(projectDir, "dist/embedded/manifest.json")), + false, + "a dry run must not write dist/embedded/manifest.json", + ); + } finally { + await Deno.remove(projectDir, { recursive: true }); + } + }); + + it("writes an embedded bundle to build.outDir from veryfront.config.js", async () => { + const projectDir = await makeProject("vf-embedded-outdir-"); + try { + await Deno.writeTextFile( + join(projectDir, "veryfront.config.js"), + 'export default { build: { outDir: "custom-out" } };\n', + ); + + await withCwd(projectDir, async () => { + await handleBuildCommand(parseCliArgs(["build", "--preset", "embedded"])); + }); + + assertEquals( + await exists(join(projectDir, "custom-out/embedded/manifest.json")), + true, + "build.outDir must decide where the embedded preset writes", + ); + assertEquals( + await exists(join(projectDir, "dist")), + false, + "the embedded preset must not fall back to dist/ when build.outDir is set", + ); + } finally { + await Deno.remove(projectDir, { recursive: true }); + } + }); + + describe("flag validation", () => { + const rejected: Array<{ argv: string[]; flag: string }> = [ + { argv: ["build", "--preset", "embedded", "--dry-run"], flag: "--dry-run" }, + { argv: ["build", "--preset", "embedded", "--split"], flag: "--split" }, + { argv: ["build", "--preset", "embedded", "--no-split"], flag: "--no-split" }, + { argv: ["build", "--preset", "embedded", "--compress"], flag: "--compress" }, + { argv: ["build", "--preset", "embedded", "--no-compress"], flag: "--no-compress" }, + { argv: ["build", "--preset", "embedded", "--prefetch"], flag: "--prefetch" }, + { argv: ["build", "--preset", "embedded", "--prefetch=false"], flag: "--prefetch" }, + { argv: ["build", "--preset", "embedded", "--ssg"], flag: "--ssg" }, + { argv: ["build", "--preset", "embedded", "--no-ssg"], flag: "--no-ssg" }, + { argv: ["build", "--preset", "embedded", "--include", "/docs"], flag: "--include" }, + { argv: ["build", "--preset", "embedded", "--exclude", "/api"], flag: "--exclude" }, + ]; + + for (const { argv, flag } of rejected) { + it(`rejects ${argv.slice(3).join(" ")} and names ${flag}`, () => { + const args = parseCliArgs(argv); + let thrown: unknown; + try { + assertEmbeddedPresetFlags(args, "embedded"); + } catch (error) { + thrown = error; + } + + assertEquals(thrown instanceof Error, true, `${flag} must be rejected`); + const message = (thrown as Error).message; + assertEquals( + message.startsWith("Invalid "), + true, + `usage errors must start with "Invalid " so the router exits 2: ${message}`, + ); + assertEquals( + message.includes(flag), + true, + `the error must name ${flag}: ${message}`, + ); + }); + } + + const accepted: string[][] = [ + ["build", "--preset", "embedded"], + ["build", "--preset", "embedded", "-o", "out"], + ["build", "--preset", "embedded", "--output", "out"], + ["build", "--preset", "embedded", "--json"], + ["build", "--preset", "embedded", "--verbose"], + ["build", "--preset", "embedded", "--quiet"], + ]; + + for (const argv of accepted) { + it(`accepts ${argv.slice(1).join(" ")}`, () => { + assertEmbeddedPresetFlags(parseCliArgs(argv), "embedded"); + }); + } + + it("leaves the default preset alone", () => { + assertEmbeddedPresetFlags( + parseCliArgs(["build", "--dry-run", "--no-split", "--include", "/docs"]), + undefined, + ); + }); + + it("names every rejected flag the user typed", () => { + let thrown: unknown; + try { + assertEmbeddedPresetFlags( + parseCliArgs(["build", "--preset", "embedded", "--dry-run", "--no-ssg"]), + "embedded", + ); + } catch (error) { + thrown = error; + } + const message = (thrown as Error).message; + assertEquals(message.includes("--dry-run"), true, message); + assertEquals(message.includes("--no-ssg"), true, message); + }); + }); +}); diff --git a/cli/commands/build/handler.ts b/cli/commands/build/handler.ts index 897dfe5669..e029b18515 100644 --- a/cli/commands/build/handler.ts +++ b/cli/commands/build/handler.ts @@ -1,9 +1,8 @@ import { defineSchema, lazySchema } from "veryfront/schemas"; import type { InferSchema } from "veryfront/extensions/schema"; import { dim } from "#cli/ui"; -import { join } from "veryfront/platform/path"; import { cliLogger, isVerbose, logSuccess } from "#cli/utils"; -import { cwd } from "veryfront/platform"; +import { cwd, runtime } from "veryfront/platform"; import { CommonArgs, createArgParser, parseArgsOrThrow } from "#cli/shared/args"; import { ensureCliBundlerContracts } from "#cli/shared/default-contracts"; import { showHeader } from "#cli/utils"; @@ -55,12 +54,76 @@ export const parseBuildArgs = createArgParser(BuildArgsSchema, { dryRun: CommonArgs.dryRun, }, { rejectUnknown: true }); +/** + * Flags the embedded preset cannot honour, in the spelling the user types. + * + * The embedded preset emits one esbuild bundle: there is nothing to split, + * no compression pass, no prefetch manifest and no prerender step, so + * `--split`, `--compress`, `--prefetch`, `--ssg`, `--include` and `--exclude` + * describe stages it does not have. `--dry-run` is different in kind — the + * preset simply never implemented it, and a flag whose whole contract is + * "changes nothing" writing to disk is the worst outcome of the three. + * + * Rejecting is the honest answer for all of them. Accepting a flag and + * dropping it, which is what this path used to do, tells the user the build + * ran the way they asked when it did not. + */ +const UNSUPPORTED_EMBEDDED_FLAGS = [ + "dry-run", + "split", + "no-split", + "compress", + "no-compress", + "prefetch", + "ssg", + "no-ssg", + "include", + "exclude", +] as const; + +/** + * Refuse an embedded build that was given a flag the preset cannot honour. + * + * Reads `__explicit` from the *raw* args rather than the parsed options on + * purpose. The schema defaults `split`, `compress` and `prefetch` to `true` + * and `dryRun`, `noSplit`, `noCompress` and `noSsg` to `false`, so the parsed + * object cannot tell a typed flag from a default — keying off it would reject + * every embedded build, including a bare one. + * + * The message starts with `Invalid ` so the router maps it to exit code 2 as a + * usage error, matching `parseArgsOrThrow`. + * + * @param args raw parsed argv, carrying `__explicit` + * @param preset the lowercased `--preset` value, if any + * @internal + */ +export function assertEmbeddedPresetFlags( + args: ParsedArgs, + preset: string | undefined, +): void { + if (preset !== "embedded") return; + + const explicit = args.__explicit ?? {}; + const unsupported = UNSUPPORTED_EMBEDDED_FLAGS + .filter((flag) => explicit[flag] === true) + .map((flag) => `--${flag}`); + if (unsupported.length === 0) return; + + throw new Error( + `Invalid build arguments: the embedded preset does not support ${unsupported.join(", ")}`, + ); +} + export async function handleBuildCommand(args: ParsedArgs): Promise { showHeader(); const opts = parseArgsOrThrow(parseBuildArgs, "build", args); + const preset = opts.preset?.toLowerCase(); + // Before any bundler setup, so a rejected build touches nothing and returns + // immediately. + assertEmbeddedPresetFlags(args, preset); + await ensureCliBundlerContracts(); const projectDir = cwd(); - const preset = opts.preset?.toLowerCase(); if (preset === "embedded") { await ensureBuiltinContentProcessor(); @@ -87,8 +150,23 @@ export async function handleBuildCommand(args: ParsedArgs): Promise { async function handleEmbeddedBuild(projectDir: string, outputDir?: string): Promise { const { buildEmbeddedPreset } = await import("veryfront/build"); + const { getConfig } = await import("veryfront/config"); + const { resolveBuildOutputDir } = await import("./command.ts"); - const finalOutput = outputDir ?? join(projectDir, "dist"); + // The config was never loaded on this path, so `build.outDir` was ignored + // and the preset always wrote `dist`. Resolving through the same helper the + // default path uses also brings its guard against an output directory that + // contains the project. + const adapter = await runtime.get(); + const config = await getConfig(projectDir, adapter); + // `clearsOutputDir: false` because `buildEmbeddedPreset` only mkdir's and + // writes into the target; unlike the production build it never removes it. + // Without this, `--preset embedded -o .` — a plausible call for a preset + // whose whole purpose is embedding into a host project — hard-fails on a + // deletion hazard that does not exist on this path. + const finalOutput = resolveBuildOutputDir(projectDir, outputDir, config, { + clearsOutputDir: false, + }); cliLogger.info("Building embedded preset..."); if (isVerbose()) { @@ -100,6 +178,7 @@ async function handleEmbeddedBuild(projectDir: string, outputDir?: string): Prom projectDir, outDir: finalOutput, runtime: "deno", + config, }); logSuccess("Built embedded preset");