Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions cli/commands/generate/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { exists, join, readTextFile } from "veryfront/fs";
import { generateIntegration } from "./integration-generator.ts";
import { isScaffoldType, scaffoldProjectFile } from "../../scaffold/engine.ts";

const MDX_EXTENSION_PACKAGE = "@veryfront/ext-content-mdx";

const PROJECT_MARKERS = [
"veryfront.config.ts",
"veryfront.config.js",
Expand Down Expand Up @@ -130,4 +132,51 @@ export async function generateCommand(
}

for (const file of result.files) cliLogger.info(`Created ${file.path}`);
await warnIfMdxExtensionMissing(projectDir, result.files.map((file) => file.path));
}

/**
* Tell the user to install the MDX extension when we have just written an
* `.mdx` file into a project that does not declare it.
*
* The pages router scaffolds `.mdx` for `page` and `layout`. Since
* `@veryfront/ext-content-mdx` became an optional peer of the npm package, a
* project that never installed it renders those routes as an error — while
* this command has just reported "Created" and exited 0. The compile path
* already throws a typed error naming the package, but by then the developer
* is debugging a route they were told was fine.
*
* Best-effort: a project without a readable package.json (a Deno project, say)
* gets no warning rather than a false one.
*/
async function warnIfMdxExtensionMissing(
projectDir: string,
paths: string[],
): Promise<void> {
if (!paths.some((path) => path.endsWith(".mdx"))) return;
try {
const raw = await readTextFile(join(projectDir, "package.json"));
const manifest = JSON.parse(raw) as Record<string, Record<string, string> | undefined>;
const declared = [
manifest.dependencies,
manifest.devDependencies,
manifest.peerDependencies,
manifest.optionalDependencies,
].some((group) => group?.[MDX_EXTENSION_PACKAGE] !== undefined);
if (declared) return;
// Lockfile-aware: hard-coding `npm install` in a pnpm/yarn/bun project
// writes a competing package-lock.json and leaves the real lockfile stale.
const { detectProjectInstallTarget, formatInstallCommand } = await import(
"#veryfront/extensions/install-command.ts"
);
const install = formatInstallCommand(
MDX_EXTENSION_PACKAGE,
detectProjectInstallTarget(projectDir),
);
cliLogger.warn(
`This project does not depend on ${MDX_EXTENSION_PACKAGE}, so the generated .mdx file will not render. Install it with: ${install}`,
);
} catch {
// No readable package.json: say nothing rather than warn wrongly.
}
}
109 changes: 109 additions & 0 deletions cli/shared/ensure-content-processor.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { assertEquals, assertRejects, assertStringIncludes } from "#veryfront/testing/assert.ts";
import { describe, it } from "#veryfront/testing/bdd.ts";
import { resolve, tryResolve, unregister } from "../../src/extensions/contracts.ts";
import type { ContentProcessor } from "veryfront/extensions/content";
import { ensureBuiltinContentProcessor } from "./ensure-content-processor.ts";

/** The error Node raises for an uninstalled optional peer dependency. */
function missingPackageError(): Error {
return Object.assign(
new Error(
"Cannot find package '@veryfront/ext-content-mdx' imported from " +
"/app/node_modules/veryfront/esm/cli/shared/ensure-content-processor.js",
),
{ code: "ERR_MODULE_NOT_FOUND" },
);
}

class StubContentProcessor {
compileMdx() {
return Promise.reject(new Error("unused"));
}
compileMarkdown() {
return Promise.reject(new Error("unused"));
}
getRemarkPlugins() {
return [];
}
getRehypePlugins() {
return [];
}
}

describe("cli/shared/ensure-content-processor", () => {
it("registers the MDX processor when the extension is installed", async () => {
try {
await ensureBuiltinContentProcessor(() =>
Promise.resolve({ MdxContentProcessor: StubContentProcessor })
);

assertEquals(
tryResolve<ContentProcessor>("ContentProcessor") instanceof StubContentProcessor,
true,
);
} finally {
unregister("ContentProcessor");
}
});

// @veryfront/ext-content-mdx is an optional peer, so a plain
// `npm install veryfront` does not install it. Server startup calls this
// unconditionally (cli/shared/server-startup.ts), so throwing here would
// break `npx veryfront dev` for every project — including the ones with no
// .mdx file at all.
it("does not fail startup when the MDX extension is not installed", async () => {
await ensureBuiltinContentProcessor(() => Promise.reject(missingPackageError()));

assertEquals(tryResolve<ContentProcessor>("ContentProcessor"), undefined);
});

// With no processor registered, the compile path is what reports the
// problem, and it names the package to install.
it("leaves the actionable install message to the content compile path", async () => {
await ensureBuiltinContentProcessor(() => Promise.reject(missingPackageError()));

let message = "";
try {
resolve<ContentProcessor>("ContentProcessor");
} catch (error) {
message = error instanceof Error ? error.message : String(error);
}

assertStringIncludes(message, "@veryfront/ext-content-mdx");
});

// What importFirstPartyExtensionModule actually throws: the raw resolution
// error is wrapped, its message gains an install hint, and the original pair
// (package + workspace source) lands on `cause` as an AggregateError. The
// classifier's message patterns are anchored, so the wrapper itself never
// matches — only the cause chain does.
it("tolerates the wrapped install-hint error the real loader throws", async () => {
const packageError = new Error(
"Cannot find package '@veryfront/ext-content-mdx' imported from " +
"/app/node_modules/veryfront/esm/src/extensions/first-party-import.js",
);
const sourceError = new Error(
"Cannot find module " +
"'/app/node_modules/veryfront/esm/extensions/ext-content-mdx/src/index.ts'",
);
const wrapped = new Error(
`${packageError.message} First-party extension "ext-content-mdx" is not ` +
"installed; install @veryfront/ext-content-mdx alongside veryfront to enable it.",
{ cause: new AggregateError([packageError, sourceError], packageError.message) },
);

await ensureBuiltinContentProcessor(() => Promise.reject(wrapped));

assertEquals(tryResolve<ContentProcessor>("ContentProcessor"), undefined);
});

// A broken transitive dependency inside an *installed* extension must not be
// mistaken for "not installed" and silently swallowed.
it("rethrows real load failures from an installed extension", async () => {
await assertRejects(
() => ensureBuiltinContentProcessor(() => Promise.reject(new Error("boom"))),
Error,
"boom",
);
});
});
56 changes: 50 additions & 6 deletions cli/shared/ensure-content-processor.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,42 @@
import { tryResolve } from "veryfront/extensions";
import { register } from "../../src/extensions/contracts.ts";
import type { ContentProcessor } from "veryfront/extensions/content";
import { importFirstPartyExtensionModule } from "veryfront/extensions/first-party-import";
import {
firstPartyExtensionSourceSpecifiers,
importFirstPartyExtensionModule,
isMissingFirstPartyExtensionModule,
} from "veryfront/extensions/first-party-import";

type ContentMdxExtensionModule = {
MdxContentProcessor: new () => ContentProcessor;
};

const CONTENT_MDX_DIRECTORY = "ext-content-mdx";
const CONTENT_MDX_PACKAGE = `@veryfront/${CONTENT_MDX_DIRECTORY}`;

/**
* Specifiers a "not installed" failure is allowed to name: the npm package and
* the workspace source entries the loader tries first. Derived rather than
* written out so they stay in step with the loader, and so this module keeps
* naming no extension source path of its own.
*
* A load failure naming anything else — a broken transitive dependency inside
* an installed ext-content-mdx, say — is a real error and must not be
* swallowed.
*/
const CONTENT_MDX_SPECIFIERS = [
CONTENT_MDX_PACKAGE,
...firstPartyExtensionSourceSpecifiers(CONTENT_MDX_DIRECTORY).map((specifier) =>
specifier.replace(/^(?:\.\.\/)+/, "")
),
];

let contentMdxModulePromise: Promise<ContentMdxExtensionModule> | undefined;

function loadContentMdxModule(): Promise<ContentMdxExtensionModule> {
contentMdxModulePromise ??= importFirstPartyExtensionModule<ContentMdxExtensionModule>(
"ext-content-mdx",
"@veryfront/ext-content-mdx",
CONTENT_MDX_DIRECTORY,
CONTENT_MDX_PACKAGE,
).catch((error) => {
contentMdxModulePromise = undefined;
throw error;
Expand Down Expand Up @@ -40,9 +64,29 @@ export function prefetchBuiltinContentProcessor(): void {
* `setupAll` to `teardownAll` to `reset()` clears the contract registry, so this
* must run *after* the server-start (or `getConfig`) call returns. We skip
* registration when a user-provided extension already supplied the contract.
*
* The npm distribution declares @veryfront/ext-content-mdx as an *optional
* peer* (see scripts/build/npm-package-metadata.ts), so a plain
* `npm install veryfront` does not install it. Every server start calls this,
* including projects with no .mdx or .md file at all, so a missing package must
* not be fatal here. Leaving the contract unregistered defers the report to the
* compile path, which throws the typed MISSING_EXTENSION_ERROR naming
* @veryfront/ext-content-mdx only when content is actually rendered.
*
* `load` is a test seam and defaults to the real module loader.
*/
export async function ensureBuiltinContentProcessor(): Promise<void> {
export async function ensureBuiltinContentProcessor(
load: () => Promise<ContentMdxExtensionModule> = loadContentMdxModule,
): Promise<void> {
if (tryResolve<ContentProcessor>("ContentProcessor")) return;
const { MdxContentProcessor } = await loadContentMdxModule();
register<ContentProcessor>("ContentProcessor", new MdxContentProcessor());

let module: ContentMdxExtensionModule;
try {
module = await load();
} catch (error) {
if (isMissingFirstPartyExtensionModule(error, CONTENT_MDX_SPECIFIERS)) return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add the MDX peer when the generator creates MDX files

In an npm project configured for the pages router, veryfront generate page ... and veryfront generate layout ... still create .mdx files in cli/scaffold/engine.ts, but the generate command neither adds @veryfront/ext-content-mdx to package.json nor tells the user to install it. After this missing-package path returns successfully, veryfront dev reports the generated route as unusable even though the command reported that it was created successfully. Update the MDX-generating flow to install or declare the newly optional peer.

Useful? React with 👍 / 👎.

throw error;
}

register<ContentProcessor>("ContentProcessor", new module.MdxContentProcessor());
}
50 changes: 50 additions & 0 deletions cli/shared/project-creation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { STARTER_TEMPLATE_NAMES } from "../../templates/types.ts";
import {
createProject,
type CreateProjectRequest,
materializeScaffold,
type ProjectCreationEvent,
} from "./project-creation.ts";

Expand Down Expand Up @@ -502,3 +503,52 @@ describe("createProject", () => {
}
});
});

describe("cli/project-creation MDX extension declaration", () => {
// Raised in review on #3783. `firstPartyExtensions` came only from the
// template config, but the `mdx` feature adds app/docs/*.mdx on top of ANY
// template — so `--template ai-agent --features mdx` scaffolded MDX routes
// with no extension declared, and every one of them failed at runtime.
it("declares ext-content-mdx when the mdx feature is selected on a non-mdx template", async () => {
const scaffold = await materializeScaffold({
template: "ai-agent",
features: ["mdx"],
projectName: "mdx-feature-probe",
});

// The mdx feature scaffolds no files: it sets `mdx.enabled` and tips the
// user to author `.mdx` themselves. Selecting it still has to declare the
// extension, or following that tip fails at runtime.
assertEquals(
scaffold.files.filter((file) => file.path.endsWith(".mdx")).length,
0,
"the mdx feature is config-and-tips only; update this if it starts shipping files",
);

const packageJson = JSON.parse(
scaffold.files.find((file) => file.path === "package.json")?.content ?? "{}",
);
const declared = Object.keys(packageJson.dependencies ?? {});
assertEquals(
declared.includes("@veryfront/ext-content-mdx"),
true,
`expected @veryfront/ext-content-mdx to be declared, got ${declared.join(", ")}`,
);
});

it("does not declare ext-content-mdx for a template with no mdx files", async () => {
const scaffold = await materializeScaffold({
template: "ai-agent",
projectName: "no-mdx-probe",
});

const mdxFiles = scaffold.files.filter((file) => file.path.endsWith(".mdx"));
assertEquals(mdxFiles.length, 0, "ai-agent alone should ship no .mdx");

const packageJson = JSON.parse(
scaffold.files.find((file) => file.path === "package.json")?.content ?? "{}",
);
const declared = Object.keys(packageJson.dependencies ?? {});
assertEquals(declared.includes("@veryfront/ext-content-mdx"), false);
});
});
38 changes: 37 additions & 1 deletion cli/shared/project-creation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,8 @@ function dedupeEnvVars(envVars: EnvVarConfig[]): EnvVarConfig[] {
});
}

const MDX_EXTENSION_PACKAGE = "@veryfront/ext-content-mdx";

async function loadTemplateFiles(
template: InitTemplate,
): Promise<
Expand Down Expand Up @@ -253,6 +255,36 @@ async function loadTemplateFiles(
};
}

/**
* Declare `@veryfront/ext-content-mdx` whenever the assembled project actually
* contains an `.mdx` file.
*
* The extension is an optional peer of the npm package — it drags
* `@types/mdx`, which breaks `tsc --noEmit` for every library consumer — so a
* project that renders MDX has to install it or those routes fail at runtime.
*
* Two ways a project ends up needing it, and the template config sees neither
* on its own:
*
* - A scaffolded `.mdx` file. The `minimal` starter ships `app/about/page.mdx`.
* - The `mdx` feature. It scaffolds no files — it sets `mdx.enabled` in the
* config and tips the user to "Create .mdx files in app/ directory" — so a
* user who follows that advice on any template would hit a runtime failure
* with nothing in `package.json` to explain it.
*/
function withMdxExtension(
firstPartyExtensions: string[] | undefined,
files: TemplateFile[],
features: FeatureName[],
): string[] | undefined {
const needsMdx = features.includes("mdx") ||
files.some((file) => file.path.endsWith(".mdx"));
if (!needsMdx) return firstPartyExtensions;
const existing = firstPartyExtensions ?? [];
if (existing.includes(MDX_EXTENSION_PACKAGE)) return existing;
return [...existing, MDX_EXTENSION_PACKAGE];
}

async function assembleFeatureFiles(
features: FeatureName[],
templateFiles: TemplateFile[],
Expand Down Expand Up @@ -491,7 +523,11 @@ async function assembleScaffold(request: {
tips: [...featureAssembly.tips, ...integrationAssembly.tips],
packageJsonOptions: {
dependencies: template.dependencies,
firstPartyExtensions: template.firstPartyExtensions,
firstPartyExtensions: withMdxExtension(
template.firstPartyExtensions,
integrationAssembly.files,
request.features,
),
integrations: integrationAssembly.loadedIntegrations.map((integration) => ({
name: integration.config.name,
npmDependencies: integration.config.npmDependencies,
Expand Down
12 changes: 6 additions & 6 deletions docs/api-reference/veryfront/scaffold.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,20 @@ for (const file of files) {

| Name | Description | Source |
| --------------------------- | --------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------- |
| `SCAFFOLD_TEMPLATE_ALIASES` | Slugs other product surfaces use for a template this CLI names differently. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L588) |
| `SCAFFOLD_TEMPLATE_ALIASES` | Slugs other product surfaces use for a template this CLI names differently. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L624) |

### Functions

| Name | Description | Source |
| ------------------------- | ----------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------- |
| `listScaffoldTemplates` | Every template slug a caller may ask for, canonical names and aliases. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L603) |
| `materializeScaffold` | Produce the complete contents of a new project without touching a disk. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L643) |
| `resolveScaffoldTemplate` | Canonical starter template for a slug, or `null` when nothing matches. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L595) |
| `listScaffoldTemplates` | Every template slug a caller may ask for, canonical names and aliases. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L639) |
| `materializeScaffold` | Produce the complete contents of a new project without touching a disk. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L679) |
| `resolveScaffoldTemplate` | Canonical starter template for a slug, or `null` when nothing matches. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L631) |

### Types

| Name | Description | Source |
| ---------------------------- | --------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------- |
| `MaterializedScaffold` | A new project: every file it starts with, plus anything worth telling the author. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L625) |
| `MaterializeScaffoldRequest` | What to build: which starter, under what name, for which runtime. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L608) |
| `MaterializedScaffold` | A new project: every file it starts with, plus anything worth telling the author. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L661) |
| `MaterializeScaffoldRequest` | What to build: which starter, under what name, for which runtime. | [source](https://github.com/veryfront/veryfront-code/blob/main/cli/shared/project-creation.ts#L644) |
| `TemplateFile` | | [source](https://github.com/veryfront/veryfront-code/blob/main/templates/types.ts#L17) |
Loading