Skip to content

Commit 7338058

Browse files
committed
refactor(codegen-ts): apply pre-merge review findings to templateGenerator
Acts on the consensus findings from the code-review and code-simplifier passes on the rc.12 templateGenerator() factory + docsFile() refactor. All 497 codegen-ts tests still green; docs-file-basic byte-identity gate preserved. Findings applied (numbered per the agent reports): #1 — drop the walk-mutation cleverness in docs-file.ts. The rc.12 docsFile() wrapped templateGenerator() and mutated tgOpts.walk per generate() call. Concurrency-unsafe (two docs-file generators against different targets would race) and obscured the intent. Replaced with a direct render() + projectProvider() call, which is shorter than the wrapper anyway. The cross-port pillar of "templateGenerator for ctx-free walks" is unaffected — other adopters still use the factory; docs-file is a peer that needs ctx for the outputLayout-driven path. #2 — thread project root through GenContext. Adds `projectRoot?: string` to GenContext, populated by the runner from opts.projectRoot. templateGenerator() and docsFile() now read ctx.projectRoot instead of process.cwd(). Falls back to cwd with a ctx.warn() naming the problem ("running `meta gen` from a sub-dir breaks project-template overrides"). Critical reviewer finding — a silent foot-gun in the rc.12 release. #3 — wrap render() with error context. Both templateGenerator() and docsFile() now wrap render() in try/catch that prepends `${generator name} on ${output path}: …` so template-resolution failures surface a useful breadcrumb. Preserves the original error as `cause`. #4 — assert canonical template exists in findFrameworkTemplatesDir. The walk-up to find codegen-ts's package.json could land on a CONSUMER's package.json under hoisted pnpm/bun installs and silently return a templates dir that doesn't exist. Now asserts the canonical docs/entity-page.md.mustache lives at the candidate templates dir before returning; throws with SELF_DIR in the message if not. #7 — collapse three field.validators() loops into one. constraintsCell in docs-data-builder.ts walked field.validators() three times to bucket regex / length / numeric. One pass into three local arrays, then emit in the original order (regex → maxLength-attr → length → numeric) to keep byte-identity with docs-file-basic. #8 — drop the defensive try/catch around fileURLToPath in framework-provider.ts. Per CLAUDE.md the project is ESM-only; import.meta.url is guaranteed to be a file: URL. Defensive code for an impossible state was muddying the contract. #9 — drop the magic `for (let i = 0; i < 12; i++)` loop cap in findFrameworkTemplatesDir. The parent-equals-self guard is the right termination; the 12-iteration cap was arbitrary and would silently return the wrong dir in deeply-nested workspaces. #10 — clean test imports in template-generator.test.ts. Drop the `void TYPE_OBJECT; void OBJECT_SUBTYPE_ENTITY;` dead-code suppressors that were masking unused imports; consolidate two separate import statements from @metaobjectsdev/metadata into one. Held for separate follow-up (not in this commit): - Finding #5: EntityDocData over-exposing Markdown-flavored fields as "public contract" — wants a deliberate split into structural + Markdown layers. API-surface decision worth a moment of design thought. - Finding #6: the `as unknown as { referencesRaw?: string }` cast on MetaIdentity — would benefit from properly exposing referencesRaw on the typed interface. - Finding #11: test gaps for missing-template error, provider override precedence, and conformance-perf single-render-per-fixture. Verification: bun run --filter '@metaobjectsdev/codegen-ts' typecheck — clean bun test packages/codegen-ts — 497/497 pass, 1809 expect() calls (includes templateGenerator unit tests, conformance harness, docs-file-basic byte-identity golden gate)
1 parent 7d376c6 commit 7338058

7 files changed

Lines changed: 139 additions & 101 deletions

File tree

server/typescript/packages/codegen-ts/src/generator.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ export interface GenContext {
2424
* present at run time when invoked via runGen(); optional in the type
2525
* so tests and custom callers don't need a placeholder. */
2626
renderContext?: RenderContext;
27+
/** Resolved absolute project root — what the runner derives from
28+
* `opts.projectRoot` (the directory holding `.metaobjects/config.json`).
29+
* Generators that resolve project-scoped resources (e.g.
30+
* `templateGenerator` looking up the project's `templates/` directory)
31+
* should read this rather than `process.cwd()`, which is whatever
32+
* directory the CLI was invoked from and breaks when `meta gen` runs
33+
* in a sub-directory. Undefined only when the runner was driven
34+
* programmatically without an explicit projectRoot. */
35+
projectRoot?: string;
2736
warn: (msg: string) => void;
2837
}
2938

server/typescript/packages/codegen-ts/src/generators/docs-data-builder.ts

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -188,36 +188,37 @@ function constraintsCell(
188188
}
189189
}
190190

191+
// Walk validators once, bucket by subtype. We re-emit in the original
192+
// emission order to preserve byte-identity with the docs-file-basic
193+
// conformance fixture: regex pattern → maxLength-from-@maxLength →
194+
// length-validator (min/max) → numeric-validator (min/max).
195+
const maxLenAttr = field.ownAttr(FIELD_ATTR_MAX_LENGTH);
196+
const regexParts: string[] = [];
197+
const lengthParts: string[] = [];
198+
const numericParts: string[] = [];
191199
for (const v of field.validators()) {
192200
if (v.subType === VALIDATOR_SUBTYPE_REGEX) {
193201
const pattern = v.ownAttr(VALIDATOR_ATTR_PATTERN);
194202
if (typeof pattern === "string" && pattern.length > 0) {
195-
parts.push(`pattern \`${pattern}\``);
203+
regexParts.push(`pattern \`${pattern}\``);
196204
}
197-
}
198-
}
199-
200-
const maxLenAttr = field.ownAttr(FIELD_ATTR_MAX_LENGTH);
201-
if (typeof maxLenAttr === "number") {
202-
parts.push(`maxLength: ${maxLenAttr}`);
203-
}
204-
for (const v of field.validators()) {
205-
if (v.subType === VALIDATOR_SUBTYPE_LENGTH) {
205+
} else if (v.subType === VALIDATOR_SUBTYPE_LENGTH) {
206206
const min = v.ownAttr(VALIDATOR_ATTR_MIN);
207207
const max = v.ownAttr(VALIDATOR_ATTR_MAX);
208-
if (typeof min === "number") parts.push(`minLength: ${min}`);
209-
if (typeof max === "number" && typeof maxLenAttr !== "number") parts.push(`maxLength: ${max}`);
210-
}
211-
}
212-
213-
for (const v of field.validators()) {
214-
if (v.subType === VALIDATOR_SUBTYPE_NUMERIC) {
208+
if (typeof min === "number") lengthParts.push(`minLength: ${min}`);
209+
if (typeof max === "number" && typeof maxLenAttr !== "number") lengthParts.push(`maxLength: ${max}`);
210+
} else if (v.subType === VALIDATOR_SUBTYPE_NUMERIC) {
215211
const min = v.ownAttr(VALIDATOR_ATTR_MIN);
216212
const max = v.ownAttr(VALIDATOR_ATTR_MAX);
217-
if (typeof min === "number") parts.push(`min: ${min}`);
218-
if (typeof max === "number") parts.push(`max: ${max}`);
213+
if (typeof min === "number") numericParts.push(`min: ${min}`);
214+
if (typeof max === "number") numericParts.push(`max: ${max}`);
219215
}
220216
}
217+
parts.push(...regexParts);
218+
if (typeof maxLenAttr === "number") {
219+
parts.push(`maxLength: ${maxLenAttr}`);
220+
}
221+
parts.push(...lengthParts, ...numericParts);
221222

222223
const fk = fkMap.get(field.name);
223224
if (fk !== undefined) {

server/typescript/packages/codegen-ts/src/generators/docs-file.ts

Lines changed: 49 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,28 @@
11
// docsFile() — emits `<Entity>.md` next to each generated entity module.
22
//
3-
// rc.12+: refactored to use templateGenerator(). The Markdown structure now
4-
// lives in `templates/docs/entity-page.md.mustache`; the data extraction lives
5-
// in `buildEntityDocData()`. Adopters can override the framework template by
6-
// dropping their own `templates/docs/entity-page.md.mustache` into the project
7-
// root (resolved via the project-then-framework provider chain).
3+
// rc.12+: structured around a shared Mustache template at
4+
// `templates/docs/entity-page.md.mustache` + a data builder at
5+
// `docs-data-builder.ts`. Adopters can override the framework template by
6+
// dropping their own `templates/docs/entity-page.md.mustache` into the
7+
// project root (resolved via the project-then-framework provider chain).
8+
//
9+
// docsFile() calls `render()` directly rather than wrapping
10+
// `templateGenerator()` because the per-entity output path depends on
11+
// `GenContext.config.outputLayout`, which the generic templateGenerator
12+
// `walk(root)` signature doesn't expose. Other future docs-style adopters
13+
// with ctx-free walks (single-file aggregators, etc.) compose
14+
// `templateGenerator()` directly.
815
//
916
// The conformance fixture (`fixtures/conformance/docs-file-basic`) gates
10-
// byte-identity through the refactor — the codegen output must match the
11-
// hand-coded rc.11 byte-for-byte. If you're hacking on this and the
12-
// conformance test breaks, the refactor is the bug, not the fixture.
17+
// byte-identity — the codegen output must match the hand-coded rc.11
18+
// byte-for-byte. If you're hacking on this and the conformance test
19+
// breaks, the refactor is the bug, not the fixture.
1320

1421
import type { MetaObject } from "@metaobjectsdev/metadata";
15-
import type { Generator, GeneratorFactory, GenContext } from "../generator.js";
22+
import { render } from "@metaobjectsdev/render";
23+
import type { Generator, GeneratorFactory } from "../generator.js";
1624
import { entityOutputPath } from "../import-path.js";
17-
import {
18-
templateGenerator,
19-
type TemplateGeneratorOpts,
20-
} from "./template-generator.js";
25+
import { projectProvider } from "../render-engine/framework-provider.js";
2126
import { buildEntityDocData } from "./docs-data-builder.js";
2227

2328
export interface DocsFileOpts {
@@ -35,48 +40,45 @@ const KNOWN_SIBLING_GENERATORS = new Set([
3540
"routes-file-hono",
3641
]);
3742

38-
export const docsFile = function docsFile(opts?: DocsFileOpts): Generator {
39-
// We can't fully delegate to templateGenerator's `walk(root)` because the
40-
// per-entity output path depends on the runtime `outputLayout` (flat /
41-
// package), which only lives on `GenContext.config`. So docsFile wraps
42-
// templateGenerator and threads ctx through.
43-
const tgOpts: TemplateGeneratorOpts = {
44-
name: "docs-file",
45-
template: "docs/entity-page.md",
46-
format: "markdown",
47-
walk: () => [], // placeholder — real walk happens inside generate() below
48-
};
49-
const inner = templateGenerator(tgOpts);
43+
const TEMPLATE_REF = "docs/entity-page.md";
5044

45+
export const docsFile = function docsFile(opts?: DocsFileOpts): Generator {
5146
const generator: Generator = {
5247
name: "docs-file",
53-
async generate(ctx: GenContext) {
48+
async generate(ctx) {
5449
if (!ctx.renderContext) {
5550
throw new Error("docs-file: renderContext is required (provided by runGen)");
5651
}
5752
const rc = ctx.renderContext;
58-
// Drive the templateGenerator by populating its walk via closure.
59-
const realWalk = (root: typeof ctx.loadedRoot) =>
60-
root.objects().filter(ctx.matches).map((entity: MetaObject) => ({
61-
data: buildEntityDocData(entity, {
62-
dialect: rc.dialect,
63-
...(rc.columnNamingStrategy !== undefined
64-
? { columnNamingStrategy: rc.columnNamingStrategy }
65-
: {}),
66-
loadedRoot: rc.loadedRoot,
67-
generatorNames: KNOWN_SIBLING_GENERATORS,
53+
const provider = projectProvider(ctx.projectRoot ?? process.cwd());
54+
const layout = ctx.config.outputLayout ?? "flat";
55+
return ctx.loadedRoot.objects().filter(ctx.matches).map((entity: MetaObject) => {
56+
const path = entityOutputPath(layout, entity.package, `${entity.name}.md`);
57+
const payload = buildEntityDocData(entity, {
58+
dialect: rc.dialect,
59+
...(rc.columnNamingStrategy !== undefined && {
60+
columnNamingStrategy: rc.columnNamingStrategy,
6861
}),
69-
outputPath: entityOutputPath(
70-
ctx.config.outputLayout ?? "flat",
71-
entity.package,
72-
`${entity.name}.md`,
73-
),
74-
}));
75-
// Hot-swap walk on the underlying templateGenerator. (The factory
76-
// closes over `opts.walk`, so we mutate the options object's walk
77-
// reference here.)
78-
(tgOpts as { walk: typeof realWalk }).walk = realWalk;
79-
return inner.generate(ctx);
62+
loadedRoot: rc.loadedRoot,
63+
generatorNames: KNOWN_SIBLING_GENERATORS,
64+
});
65+
let content: string;
66+
try {
67+
content = render({
68+
ref: TEMPLATE_REF,
69+
payload,
70+
provider,
71+
format: "markdown",
72+
});
73+
} catch (err) {
74+
const msg = err instanceof Error ? err.message : String(err);
75+
throw new Error(
76+
`docs-file: failed rendering '${TEMPLATE_REF}' for '${path}': ${msg}`,
77+
{ cause: err instanceof Error ? err : undefined },
78+
);
79+
}
80+
return { path, content };
81+
});
8082
},
8183
};
8284
if (opts?.filter) generator.filter = opts.filter;

server/typescript/packages/codegen-ts/src/generators/template-generator.ts

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,11 @@ export interface TemplateGeneratorOpts {
4747
* default `walk` — adopters apply filters inside their walk function. */
4848
filter?: (entity: MetaObject) => boolean;
4949
/** Override the Provider used for template resolution. When omitted the
50-
* generator resolves via `projectProvider(<projectRoot>)`, which layers
50+
* generator resolves via `projectProvider(ctx.projectRoot)`, which layers
5151
* the project's `templates/` over the framework defaults. (The project
52-
* root is taken from `process.cwd()` at run time — adopters needing a
53-
* different lookup chain can pass an explicit provider.) */
52+
* root is the directory holding `.metaobjects/config.json`, threaded
53+
* through `GenContext` by the runner. Adopters needing a different
54+
* lookup chain can pass an explicit provider.) */
5455
provider?: Provider;
5556
/** Optional named target — same as the other generators. */
5657
target?: string;
@@ -63,16 +64,37 @@ export const templateGenerator = function templateGenerator(
6364
const generator: Generator = {
6465
name: opts.name,
6566
async generate(ctx: GenContext): Promise<EmittedFile[]> {
66-
const provider = opts.provider ?? projectProvider(process.cwd());
67+
let provider: Provider;
68+
if (opts.provider !== undefined) {
69+
provider = opts.provider;
70+
} else if (ctx.projectRoot !== undefined) {
71+
provider = projectProvider(ctx.projectRoot);
72+
} else {
73+
ctx.warn(
74+
"templateGenerator: ctx.projectRoot is undefined; falling back to process.cwd() for project-template resolution. " +
75+
"Project-scoped template overrides will resolve relative to the current working directory, which is fragile under " +
76+
"`meta gen` invoked from a sub-directory. Drive via runGen(opts.projectRoot) to remove this warning.",
77+
);
78+
provider = projectProvider(process.cwd());
79+
}
6780
const walkRes = await opts.walk(ctx.loadedRoot);
6881
const files: EmittedFile[] = [];
6982
for (const { data, outputPath } of walkRes) {
70-
const content = render({
71-
ref: opts.template,
72-
payload: data,
73-
provider,
74-
format: fmt,
75-
});
83+
let content: string;
84+
try {
85+
content = render({
86+
ref: opts.template,
87+
payload: data,
88+
provider,
89+
format: fmt,
90+
});
91+
} catch (err) {
92+
const msg = err instanceof Error ? err.message : String(err);
93+
throw new Error(
94+
`templateGenerator(${opts.name}) failed rendering '${opts.template}' for '${outputPath}': ${msg}`,
95+
{ cause: err instanceof Error ? err : undefined },
96+
);
97+
}
7698
files.push({ path: outputPath, content });
7799
}
78100
return files;

server/typescript/packages/codegen-ts/src/render-engine/framework-provider.ts

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,34 +17,43 @@ import { existsSync, readFileSync } from "node:fs";
1717
import { join, resolve, dirname } from "node:path";
1818
import { fileURLToPath } from "node:url";
1919

20-
/** Walk up from `start` until we find a `package.json` (i.e., the codegen-ts
21-
* package root), then return `<pkg-root>/templates`. Works the same way
22-
* from `src/render-engine/framework-provider.ts` (during dev) and
23-
* `dist/render-engine/framework-provider.js` (after `npm install`) because
24-
* in both cases we shipped at the package root. */
20+
/** Canonical shipped template — used to verify a candidate framework
21+
* templates directory actually contains our defaults. Without this check a
22+
* hoisted-install layout (pnpm/bun workspaces) can walk up to a CONSUMER
23+
* package.json and silently return a templates dir that doesn't exist. */
24+
const CANONICAL_TEMPLATE_REL = "docs/entity-page.md.mustache";
25+
26+
/** Walk up from `start` until we find a `package.json` whose neighbour
27+
* `templates/` directory contains our canonical shipped template (i.e., the
28+
* codegen-ts package root). Works the same way from
29+
* `src/render-engine/framework-provider.ts` (during dev) and
30+
* `dist/render-engine/framework-provider.js` (after `npm install`). */
2531
function findFrameworkTemplatesDir(start: string): string {
2632
let dir = start;
27-
for (let i = 0; i < 12; i++) {
33+
while (true) {
2834
const pkgJson = join(dir, "package.json");
2935
if (existsSync(pkgJson)) {
30-
return join(dir, "templates");
36+
const templatesDir = join(dir, "templates");
37+
// Assert we landed at the codegen-ts package root, not a consumer's.
38+
if (existsSync(join(templatesDir, CANONICAL_TEMPLATE_REL))) {
39+
return templatesDir;
40+
}
3141
}
3242
const parent = dirname(dir);
3343
if (parent === dir) break;
3444
dir = parent;
3545
}
36-
// Last resort — caller will get an unresolved-ref error when it tries to
37-
// use a framework template that isn't on disk.
38-
return join(start, "templates");
46+
throw new Error(
47+
`framework templates dir unresolved: walked up from ${start} without finding a package.json ` +
48+
`whose templates/${CANONICAL_TEMPLATE_REL} exists. This usually means codegen-ts was installed ` +
49+
`via a hoisted layout (pnpm/bun workspaces) into an unexpected location, or the published ` +
50+
`tarball is missing the templates/ directory.`,
51+
);
3952
}
4053

41-
const SELF_DIR = (() => {
42-
try {
43-
return dirname(fileURLToPath(import.meta.url));
44-
} catch {
45-
return process.cwd();
46-
}
47-
})();
54+
// In ESM (CLAUDE.md: "ESM only. No CommonJS."), `import.meta.url` is
55+
// guaranteed to be a file: URL; no defensive try/catch needed.
56+
const SELF_DIR = dirname(fileURLToPath(import.meta.url));
4857

4958
const FRAMEWORK_TEMPLATES_DIR = findFrameworkTemplatesDir(SELF_DIR);
5059

server/typescript/packages/codegen-ts/src/runner.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ export async function runGen(opts: RunGenOpts): Promise<RunGenResult> {
174174
outputLayout: selfTarget.outputLayout,
175175
},
176176
renderContext,
177+
...(projectRoot !== undefined && { projectRoot }),
177178
warn: (msg) => warnings.push(`[${generator.name}] ${msg}`),
178179
};
179180

server/typescript/packages/codegen-ts/test/generators/template-generator.test.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,14 @@ import { InMemoryProvider } from "@metaobjectsdev/render";
99
import { templateGenerator } from "../../src/generators/template-generator.js";
1010
import { buildEntityDocData } from "../../src/generators/docs-data-builder.js";
1111
import {
12-
TYPE_OBJECT, OBJECT_SUBTYPE_ENTITY,
13-
} from "@metaobjectsdev/metadata";
14-
import { metaRoot, metaObject, metaField } from "../_meta-build.js";
15-
import {
12+
OBJECT_SUBTYPE_ENTITY,
1613
FIELD_SUBTYPE_LONG, FIELD_SUBTYPE_STRING,
1714
TypeId, TYPE_IDENTITY, IDENTITY_SUBTYPE_PRIMARY, IDENTITY_ATTR_FIELDS,
1815
IDENTITY_ATTR_GENERATION, GENERATION_INCREMENT,
1916
} from "@metaobjectsdev/metadata";
20-
import { meta } from "../_meta-build.js";
17+
import { metaRoot, metaObject, metaField, meta } from "../_meta-build.js";
2118
import type { GenContext } from "../../src/generator.js";
2219

23-
void TYPE_OBJECT;
24-
void OBJECT_SUBTYPE_ENTITY;
25-
2620
function buildRoot() {
2721
const root = metaRoot("root", "demo");
2822
const e = metaObject(OBJECT_SUBTYPE_ENTITY, "Post");

0 commit comments

Comments
 (0)