Skip to content

Commit be347aa

Browse files
dmealingclaude
andcommitted
fix(codegen-ts): extractor/render-helper import the payload VO from its real module (not the non-existent ./payloads.js)
The standalone extractor() + renderHelper() generators emitted `import type { <PayloadVO(s)> } from "./payloads.js"`, but NO TS generator emits payloads.ts — so a real `meta gen` run produced *.extractor.ts / *.render.ts that failed to compile (TS2307). The payload VO interface and its enum union-aliases actually live in the VO's OWN generated entity module (entityFile emits <VO>.ts exporting `interface <VO>` + `type <Owner><Field>`), which is exactly where api-docs already documents the type. Fix: - render-helper: import the payload interface from `./<payloadRef>.js`. - extractor: group the reachable payload types BY the entity module that exports them (VO interface + its own enum aliases → `./<VO>.js`; each nested VO → its own module), replacing the single dangling `./payloads.js` import. - Reconcile the mirror→strict mapper to the entity-module interface's optional shape: optional fields are `f?: T` (= T | undefined, never T | null) there, so an absent optional now maps to `?? undefined` (was `?? null`, which matched payload-codegen's `T | null` shape and was TS2322 against the real module). This was a latent skew the dead import had masked. Proof: new golden/extractor-render-payload-imports.test.ts runs the REAL generators (entityFile + outputParser + extractor + renderHelper) into a temp dir and tsc-compiles the emitted files against the REAL VO modules — the payload import resolves (no TS2307) and the types are found. The existing extractor compile gate now compiles against the real per-VO modules instead of a hand-written payloads.ts. promptRender's self-contained inline-interface flow (which strips the import) is unchanged. Full codegen-ts suite green (696 pass). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 73c76f9 commit be347aa

4 files changed

Lines changed: 377 additions & 53 deletions

File tree

server/typescript/packages/codegen-ts/src/templates/extractor.ts

Lines changed: 65 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ function isObjectField(field: MetaData): boolean {
6464

6565
/**
6666
* The union-alias type name for a `field.enum` with effective `@values`, or undefined when the
67-
* field is not a value-constrained enum. Reuses `enumUnionAliasName` — the SAME naming the payload
68-
* emitter (`payload-codegen.ts`) types the field as — so the cast target resolves to the exact
69-
* alias exported from `payloads.ts`. `ownerName` is the owning value-object's interface name.
67+
* field is not a value-constrained enum. Reuses `enumUnionAliasName` — the SAME naming the entity
68+
* inferred-types emitter types the field as — so the cast target resolves to the exact alias
69+
* exported from the owning VO's entity module. `ownerName` is the owning value-object's interface name.
7070
*/
7171
function enumAlias(field: MetaData, ownerName: string): string | undefined {
7272
if (field.subType !== FIELD_SUBTYPE_ENUM) return undefined;
@@ -76,11 +76,12 @@ function enumAlias(field: MetaData, ownerName: string): string | undefined {
7676
}
7777

7878
/**
79-
* True iff the field is required IN THE STRICT PAYLOAD TYPE. This MUST match
80-
* payload-codegen.ts's `isFieldRequired` predicate EXACTLY (boolean `true` only) — the payload
81-
* interface decides `T` vs `T | null` by that predicate, and the mapper's optionality assumption
82-
* (`m.f!` vs `m.f ?? null`) has to agree with the type it is constructing. A `@required:"true"`
83-
* string field is therefore `T | null` in the payload AND optional here (no skew).
79+
* True iff the field is required IN THE STRICT PAYLOAD TYPE. The strict payload IS the VO's own
80+
* generated entity-module interface (`renderValueObjectInterface`), which types a required field
81+
* `f: T` and an optional one `f?: T` (i.e. `T | undefined` — NOT `T | null`). So the mapper's
82+
* optionality assumption (`m.f!` vs `m.f ?? undefined`) has to agree with THAT interface, and an
83+
* absent optional maps to `undefined`, never `null`. This predicate matches the interface's
84+
* required test (boolean `true` only) so the two never skew.
8485
*/
8586
function isFieldRequired(field: MetaData): boolean {
8687
return field.ownAttr(FIELD_ATTR_REQUIRED) === true;
@@ -93,8 +94,10 @@ function mapperName(vo: MetaData): string {
9394

9495
/**
9596
* The mapper-body initializer expression for one field, reading mirror member `m.<name>` and
96-
* mapping it onto the strict payload's exact optionality (required → `m.f!`; optional → `m.f ?? null`).
97-
* Nested single/array objects recurse into their toStrict<Type> mapper, guarding null when optional.
97+
* mapping it onto the strict payload's exact optionality (required → `m.f!`; optional → `m.f ?? undefined`).
98+
* The strict payload is the VO's generated entity-module interface, whose optional fields are
99+
* `f?: T` (= `T | undefined`, never `T | null`), so an absent optional maps to `undefined`.
100+
* Nested single/array objects recurse into their toStrict<Type> mapper, guarding when optional.
98101
*/
99102
function strictArg(field: MetaData, root: MetaData, ownerName: string): string {
100103
const name = field.name;
@@ -104,25 +107,25 @@ function strictArg(field: MetaData, root: MetaData, ownerName: string): string {
104107
const target = refVo(field, root);
105108
if (target === undefined) {
106109
// Unresolved @objectRef — the payload type would be `unknown`; pass through as-is.
107-
return required ? `m.${name}!` : `m.${name} ?? null`;
110+
return required ? `m.${name}!` : `m.${name} ?? undefined`;
108111
}
109112
const fn = mapperName(target);
110113
if (isArray(field)) {
111114
// Required array-of-objects: each element mapped; element nulls dropped at the type level
112115
// via the non-null assertion (extract never yields null elements for a present array).
113116
if (required) return `m.${name}!.map((e) => ${fn}(e!))`;
114-
return `m.${name} ? m.${name}!.map((e) => ${fn}(e!)) : null`;
117+
return `m.${name} ? m.${name}!.map((e) => ${fn}(e!)) : undefined`;
115118
}
116119
// Single nested object.
117120
if (required) return `${fn}(m.${name}!)`;
118-
return `m.${name} ? ${fn}(m.${name}) : null`;
121+
return `m.${name} ? ${fn}(m.${name}) : undefined`;
119122
}
120123

121124
// Scalar ARRAY (e.g. `field.string` with isArray): the mirror types it `(T | null)[] | null`
122-
// but the strict payload types it `T[]` (required) / `T[] | null` (optional). A bare `m.f!`
125+
// but the strict payload types it `T[]` (required) / `T[]?` (optional). A bare `m.f!`
123126
// would leave the element type `T | null`, a `tsc --strict` TS2322 error. Filter out null
124127
// elements so the element type narrows to non-null (consistent with the lost-element DROP policy
125-
// already used for required arrays-of-objects above).
128+
// already used for required arrays-of-objects above). An absent optional array maps to `undefined`.
126129
//
127130
// ENUM arrays: the mirror element is a plain `string`, but the strict payload types it as the
128131
// closed `<Alias>[]` union. The null-filter alone narrows to `string[]`, not `<Alias>[]` — a
@@ -136,23 +139,23 @@ function strictArg(field: MetaData, root: MetaData, ownerName: string): string {
136139
return alias !== undefined ? `(${filtered}) as ${alias}[]` : filtered;
137140
}
138141
const filtered = `m.${name}.filter((x): x is NonNullable<typeof x> => x != null)`;
139-
const guarded = `m.${name} == null ? null : ${filtered}`;
142+
const guarded = `m.${name} == null ? undefined : ${filtered}`;
140143
return alias !== undefined
141-
? `m.${name} == null ? null : (${filtered}) as ${alias}[]`
144+
? `m.${name} == null ? undefined : (${filtered}) as ${alias}[]`
142145
: guarded;
143146
}
144147

145148
// Scalar / enum (single): the strict payload's optionality decides the shape.
146-
// Required → non-null assertion; optional → `?? null` (matches the payload's `f?: T | null`).
149+
// Required → non-null assertion; optional → `?? undefined` (matches the entity-module `f?: T`).
147150
//
148151
// ENUM scalar: the mirror member is a plain `string`, but the strict payload types it as the
149152
// closed `<Alias>` union — assigning `string` into `<Alias>` is a `tsc --strict` TS2322 error.
150153
// So the value is CAST to `<Alias>`. Sound for the same reason as enum arrays above: the engine
151154
// already validated membership (or extract throws on a lost required field).
152155
if (alias !== undefined) {
153-
return required ? `m.${name}! as ${alias}` : `(m.${name} ?? null) as ${alias} | null`;
156+
return required ? `m.${name}! as ${alias}` : `(m.${name} ?? undefined) as ${alias} | undefined`;
154157
}
155-
return required ? `m.${name}!` : `m.${name} ?? null`;
158+
return required ? `m.${name}!` : `m.${name} ?? undefined`;
156159
}
157160

158161
/**
@@ -202,32 +205,52 @@ function emitMapper(
202205
}
203206

204207
/**
205-
* Collect the strict payload-interface names reachable from `vo` (for the type-only import),
206-
* PLUS every enum union-alias reachable from those VOs. Both are exported from `payloads.ts`
207-
* (the alias is hoisted above the interface there), so the extractor's `as <Alias>` casts need
208-
* the alias names imported alongside the interface names. Deduped, in discovery order.
208+
* One payload-type import group: the strict types (VO interface + its own enum
209+
* union-aliases) imported from a single VO entity module.
209210
*/
210-
function reachablePayloadTypes(vo: MetaData, root: MetaData): string[] {
211-
const order: string[] = [];
212-
const seen = new Set<string>();
211+
interface PayloadImportGroup {
212+
/** The VO whose entity module exports these types (`./<module>.js`). */
213+
module: string;
214+
/** The strict type names exported by that module, in discovery order. */
215+
types: string[];
216+
}
217+
218+
/**
219+
* Group the strict payload types reachable from `vo` BY THE ENTITY MODULE THAT EXPORTS THEM.
220+
*
221+
* Each value-object gets its own generated entity module (`entityFile()` emits `<VO>.ts` exporting
222+
* `export interface <VO>`), and `renderEnumTypeAliases` hoists every `field.enum` union-alias INTO
223+
* the OWNING VO's module (`export type <Owner><Field> = ...` co-located with the interface). So a
224+
* VO's interface AND the aliases for its own enum fields are imported from `./<VO>.js` — NOT from a
225+
* single `payloads.ts` (which no generator emits). Deduped, in discovery order, one group per VO.
226+
*/
227+
function reachablePayloadGroups(vo: MetaData, root: MetaData): PayloadImportGroup[] {
228+
const groups: PayloadImportGroup[] = [];
229+
const seenVo = new Set<string>();
230+
const seenAlias = new Set<string>();
213231
const visit = (cur: MetaData) => {
214-
if (seen.has(cur.name)) return;
215-
seen.add(cur.name);
216-
order.push(cur.name);
232+
if (seenVo.has(cur.name)) return;
233+
seenVo.add(cur.name);
234+
// The VO interface + its OWN enum aliases share the VO's entity module.
235+
const types: string[] = [cur.name];
217236
for (const f of fields(cur)) {
218237
const alias = enumAlias(f, cur.name);
219-
if (alias !== undefined && !seen.has(alias)) {
220-
seen.add(alias);
221-
order.push(alias);
238+
if (alias !== undefined && !seenAlias.has(alias)) {
239+
seenAlias.add(alias);
240+
types.push(alias);
222241
}
242+
}
243+
groups.push({ module: cur.name, types });
244+
// Recurse into nested object refs (their interfaces live in their own modules).
245+
for (const f of fields(cur)) {
223246
if (isObjectField(f)) {
224247
const target = refVo(f, root);
225248
if (target !== undefined) visit(target);
226249
}
227250
}
228251
};
229252
visit(vo);
230-
return order;
253+
return groups;
231254
}
232255

233256
/** Collect the mirror-interface names reachable from `vo` (root mirror + nested VO mirrors). */
@@ -286,10 +309,16 @@ export function renderExtractor(root: MetaData, templateName: string): string {
286309
const extractName = `extract${templateName}`;
287310
const rootMapper = mapperName(vo);
288311

289-
const payloadTypes = reachablePayloadTypes(vo, root);
312+
const payloadGroups = reachablePayloadGroups(vo, root);
290313
const mirrorTypes = reachableMirrorTypes(vo, root, rootMirror);
291314
const mappers = emitMappers(vo, root, rootMirror);
292315

316+
// One type-only import per VO entity module (the VO interface + its own enum
317+
// union-aliases co-located there). NOT a single non-existent `./payloads.js`.
318+
const payloadImports = payloadGroups
319+
.map((g) => `import type { ${g.types.join(", ")} } from "./${g.module}.js";`)
320+
.join("\n");
321+
293322
const lostMsg =
294323
`${extractName}: lost required field(s): `;
295324

@@ -301,7 +330,7 @@ export function renderExtractor(root: MetaData, templateName: string): string {
301330
`// mapping the all-nullable mirror onto the strict payload. No registry / binding / factory.\n` +
302331
`\n` +
303332
`import {\n ${extractLenientWithName},\n type ${mirrorTypes.join(",\n type ")},\n} from "./${templateName}.output.js";\n` +
304-
`import type { ${payloadTypes.join(", ")} } from "./payloads.js";\n` +
333+
`${payloadImports}\n` +
305334
`import type { MetaRoot } from "@metaobjectsdev/metadata";\n` +
306335
`import type { ExtractionResult } from "@metaobjectsdev/render";\n` +
307336
`\n` +

server/typescript/packages/codegen-ts/src/templates/render-helper.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ export function renderRenderHelper(
192192

193193
return `import { render } from "@metaobjectsdev/render";
194194
import type { Provider, EmailDocument } from "@metaobjectsdev/render";
195-
import type { ${payloadRef} } from "./payloads.js";
195+
import type { ${payloadRef} } from "./${payloadRef}.js";
196196
197197
/**
198198
* Render the ${templateName} email (subject + html body${typeof textBodyRef === "string" ? " + text body" : ""}) from a
@@ -230,7 +230,7 @@ export function ${fnName}(payload: ${payloadRef}, provider: Provider): EmailDocu
230230

231231
return `import { render } from "@metaobjectsdev/render";
232232
import type { Provider } from "@metaobjectsdev/render";
233-
import type { ${payloadRef} } from "./payloads.js";
233+
import type { ${payloadRef} } from "./${payloadRef}.js";
234234
235235
/**
236236
* Render the ${templateName} document from a typed ${payloadRef} payload. Wraps the

server/typescript/packages/codegen-ts/test/extractor-codegen.test.ts

Lines changed: 54 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,41 @@ import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata";
1818
import { renderOutputParser } from "../src/templates/output-parser.js";
1919
import { renderExtractor } from "../src/templates/extractor.js";
2020
import { generatePayloadInterfaces } from "../src/payload-codegen.js";
21+
import { entityFile } from "../src/generators/index.js";
22+
import { makeRenderContext } from "../src/render-context.js";
23+
import { buildPkMap } from "../src/pk-resolver.js";
24+
import { buildRelationMap } from "../src/relation-resolver.js";
25+
import type { GenContext } from "../src/generator.js";
26+
27+
// Emit the REAL per-VO entity modules (`<VO>.ts` exporting `interface <VO>` +
28+
// its enum union-aliases) the same way `meta gen` does, so the generated
29+
// extractor's `from "./<VO>.js"` payload imports RESOLVE against the modules
30+
// that actually exist (no hand-written `payloads.ts` that no generator emits).
31+
async function writeEntityModules(
32+
dir: string,
33+
root: Awaited<ReturnType<typeof loadRoot>>,
34+
): Promise<void> {
35+
const renderContext = makeRenderContext({
36+
dialect: "sqlite",
37+
loadedRoot: root,
38+
outDir: "/tmp",
39+
dbImport: "~/db",
40+
pkMap: buildPkMap(root),
41+
relationMap: buildRelationMap(root),
42+
});
43+
const ctx: GenContext = {
44+
entities: root.objects(),
45+
loadedRoot: root,
46+
matches: () => true,
47+
projectRoot: "/tmp",
48+
config: { outDir: "/tmp", extStyle: "none", dbImport: "~/db", dialect: "sqlite" } as never,
49+
renderContext,
50+
warn: () => {},
51+
};
52+
for (const f of await entityFile().generate(ctx)) {
53+
writeFileSync(join(dir, f.path), f.content);
54+
}
55+
}
2156

2257
// ---------------------------------------------------------------------------
2358
// tsc --strict compile gate (mirrors fr004-verify-demo.test.ts's `compile()`).
@@ -166,12 +201,13 @@ describe("Extractor codegen — source shape", () => {
166201
// NOT a bare `m.tags!` / `m.flags!` (which would be a tsc --strict TS2322 error).
167202
expect(src).toContain("(m.tags ?? []).filter((x): x is NonNullable<typeof x> => x != null)");
168203
expect(src).toContain(
169-
"m.flags == null ? null : m.flags.filter((x): x is NonNullable<typeof x> => x != null)",
204+
"m.flags == null ? undefined : m.flags.filter((x): x is NonNullable<typeof x> => x != null)",
170205
);
171206
expect(src).not.toContain("m.tags!");
172207
expect(src).not.toContain("m.flags!");
173-
// optional single nested object → null-guarded recurse into its mapper
174-
expect(src).toContain("m.shipTo ? toStrictCustomer(m.shipTo) : null");
208+
// optional single nested object → undefined-guarded recurse into its mapper
209+
// (the strict entity-module interface types optionals `?: T` = `T | undefined`).
210+
expect(src).toContain("m.shipTo ? toStrictCustomer(m.shipTo) : undefined");
175211
});
176212

177213
test("payload typing: field.enum is a value-constrained union alias (not unknown), single + array", async () => {
@@ -233,7 +269,6 @@ describe("Extractor codegen — source shape", () => {
233269

234270
test("tsc --strict gate: emitted payload + parser + extractor type-check with ZERO diagnostics", async () => {
235271
const root = await loadRoot(MODEL);
236-
const payloadSrc = generatePayloadInterfaces(root, "Order");
237272
const parserSrc = renderOutputParser(root, "OrderOut");
238273
const extractorSrc = renderExtractor(root, "OrderOut");
239274

@@ -246,13 +281,16 @@ describe("Extractor codegen — source shape", () => {
246281

247282
const dir = mkdtempSync(join(import.meta.dir, "extractor-tsc-"));
248283
TEMP_DIRS.push(dir);
249-
writeFileSync(join(dir, "payloads.ts"), payloadSrc);
284+
// The REAL per-VO entity modules the extractor imports its payload types from.
285+
await writeEntityModules(dir, root);
250286
writeFileSync(join(dir, "OrderOut.output.ts"), parserSrc);
251287
writeFileSync(join(dir, "OrderOut.extractor.ts"), extractorSrc);
252288
writeFileSync(join(dir, "engine.d.ts"), ENGINE_STUBS);
253289

254290
const diagnostics = compile(dir, [
255-
"payloads.ts",
291+
"Order.ts",
292+
"Customer.ts",
293+
"Line.ts",
256294
"OrderOut.output.ts",
257295
"OrderOut.extractor.ts",
258296
"engine.d.ts",
@@ -266,13 +304,13 @@ describe("Extractor codegen — source shape", () => {
266304
describe("Extractor codegen — import-and-RUN proof (bun dynamic import)", () => {
267305
test("extractOrder() extracts dirty JSON into the strict payload; missing-required throws; extract re-exposed", async () => {
268306
const root = await loadRoot(MODEL);
269-
const payloadSrc = generatePayloadInterfaces(root, "Order");
270307
const parserSrc = renderOutputParser(root, "OrderOut");
271308
const extractorSrc = renderExtractor(root, "OrderOut");
272309

273310
const dir = mkdtempSync(join(import.meta.dir, "extractor-emit-"));
274311
TEMP_DIRS.push(dir);
275-
writeFileSync(join(dir, "payloads.ts"), payloadSrc);
312+
// The extractor's payload imports are `import type` (erased at runtime), so the
313+
// VO modules need not be loaded here; the run-proof exercises the emitted logic.
276314
writeFileSync(join(dir, "OrderOut.output.ts"), parserSrc);
277315
writeFileSync(join(dir, "OrderOut.extractor.ts"), extractorSrc);
278316

@@ -303,22 +341,23 @@ describe("Extractor codegen — import-and-RUN proof (bun dynamic import)", () =
303341
expect(order.priority).toBe("HIGH");
304342
// enum array: null-filtered, typed as OrderLabels[].
305343
expect(order.labels).toEqual(["A", "B"]);
306-
// optional single nested object `shipTo` ABSENT in this input → the `m.shipTo ? toStrictCustomer(...) : null`
307-
// branch produces null at runtime (not undefined, not a partial object).
308-
expect(order.shipTo).toBeNull();
344+
// optional single nested object `shipTo` ABSENT in this input → the `m.shipTo ? toStrictCustomer(...) : undefined`
345+
// branch produces undefined at runtime (matches the strict `shipTo?: Customer` entity-module
346+
// interface optionality — not a partial object).
347+
expect(order.shipTo).toBeUndefined();
309348

310-
// optional single nested object PRESENT → the null-guard recurses into toStrictCustomer and populates.
349+
// optional single nested object PRESENT → the guard recurses into toStrictCustomer and populates.
311350
const withShipTo =
312351
'{ "customer": { "name": "Ada" }, "lines": [ { "sku": "A", "qty": 2 } ], "tags": ["x"], "priority": "LOW", "labels": ["A"], "shipTo": { "name": "Grace" } }';
313352
const shipped = ex.extractOrderOut(root, withShipTo);
314-
expect(shipped.shipTo).not.toBeNull();
353+
expect(shipped.shipTo).not.toBeUndefined();
315354
expect(shipped.shipTo.name).toBe("Grace");
316-
// and when shipTo is genuinely absent on a separate clean input, it is null (re-confirm the branch)
355+
// and when shipTo is genuinely absent on a separate clean input, it is undefined (re-confirm the branch)
317356
const noShipTo = ex.extractOrderOut(
318357
root,
319358
'{ "customer": { "name": "Ada" }, "lines": [ { "sku": "A", "qty": 2 } ], "tags": ["x"], "priority": "LOW", "labels": ["A"] }',
320359
);
321-
expect(noShipTo.shipTo).toBeNull();
360+
expect(noShipTo.shipTo).toBeUndefined();
322361

323362
// missing required `customer` → throws
324363
expect(() => ex.extractOrderOut(root, '{ "lines": [] }')).toThrow();

0 commit comments

Comments
 (0)