Skip to content

Commit ba34510

Browse files
dmealingclaude
andcommitted
fix(codegen-ts): the routes template invites a copy that does not compile
FR-040's answer to "MetaObjects does not ship a codegen package per framework" is that you own the generator and retarget its emit. Most tiers make that true by RELOCATING the composition into the ejected file — `entity.ts` and `queries.ts` carry the composer's body verbatim, so every primitive they call is public by construction. The routes tier does not: its composition is richer (M:N junction traversal, TPH per-subtype route sets) and stays in the engine as `renderRoutesFile`, so the template's `customize:` note tells an adopter retargeting to another HTTP framework to copy that body out of the package source. Follow that instruction and you get a file that does not compile. Of the sixteen symbols the body imports from inside the package, fourteen resolve from `@metaobjectsdev/codegen-ts` and two do not — `routesHandlerName` and `TPH_POLYMORPHIC_VERBS`, both `TS2305: has no exported member`. Verified against the package's own exports map, not inferred: a probe importing all sixteen from the public entry reported exactly those two. It matters more than the two lines suggest. HTTP framework is the tier an adopter is MOST likely to have an opinion about — anyone outside Fastify and Hono has to retarget it, and everyone else can leave every other tier alone. So the one escape that had to work is the one that did not, and it failed at the point where the adopter has already committed to the approach. Both are exported now, and the gate is the durable half. It DERIVES the required set from `renderRoutesFile`'s own import statements rather than restating them, because a hand-kept list reproduces the defect one level up: the body gains an import, nobody re-reads the list, and the escape silently breaks again. It also asserts its own premise — that the reference template still contains the sentence inviting the copy — so if that instruction is withdrawn the test fails and asks to be re-read rather than quietly guarding nothing. Proven by breaking it: with `routesHandlerName` un-exported the gate goes red naming it, and green again when restored. Found while auditing how tightly the generated surface couples to the libraries the maintainer's own adopter estates happen to use. All seven are TypeScript on Drizzle with Postgres or D1, so the estate loop cannot see this class of problem — an escape hatch nobody in the estate set needs to take. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01At3v6M6uqECZ2Sb5eUv6YY
1 parent 0f152bf commit ba34510

2 files changed

Lines changed: 110 additions & 2 deletions

File tree

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,12 @@ export { warnRetiredCodegenAttrs } from "./retired-codegen-attrs.js";
8383

8484
export { formatTs } from "./format.js";
8585

86-
export { pluralize, columnNameFromField, tableNameFromEntity, viewNameFromProjection } from "./naming.js";
86+
export { pluralize, columnNameFromField, tableNameFromEntity, viewNameFromProjection, routesHandlerName } from "./naming.js";
87+
// routesHandlerName is public for the same reason the routes-expose members below are: the
88+
// routes reference template tells an adopter retargeting to another HTTP framework to copy
89+
// `renderRoutesFile`'s body out of the package source, and that body names this. See the
90+
// owned-composition-imports gate in test/ — it derives the required set from that body rather
91+
// than trusting this list to stay complete.
8792

8893
export { packageToPath, entityOutputPath, crossEntitySpecifier, barrelEntrySpecifier, relativeModuleSpecifier, entityModuleSpecifier, siblingSpecifier, barrelModuleSpecifier } from "./import-path.js";
8994
export type { OutputLayout, ResolvedTarget } from "./import-path.js";
@@ -194,7 +199,7 @@ export { renderEntityConstants, resourcePath } from "./templates/entity-constant
194199
export { renderQueriesFile } from "./templates/queries-file.js";
195200
// #348 — which CRUD verbs a generated routes file mounts. Public because an OWNED
196201
// routes generator (ADR-0034) composes the same render call and needs the same option.
197-
export { CRUD_VERBS, resolveExpose, intersectExpose, exposeLine } from "./routes-expose.js";
202+
export { CRUD_VERBS, TPH_POLYMORPHIC_VERBS, resolveExpose, intersectExpose, exposeLine } from "./routes-expose.js";
198203
export type { CrudVerb, ExposeOption } from "./routes-expose.js";
199204
export { renderRoutesFile } from "./templates/routes-file.js";
200205
export { renderRoutesFileHono } from "./templates/routes-file-hono.js";
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { readFileSync } from "node:fs";
3+
import { join } from "node:path";
4+
5+
/**
6+
* A reference template that says "copy this body out of the package source" is making a promise
7+
* the exports map has to keep.
8+
*
9+
* ADR-0034 gives an adopter an ownable copy of each generator, and most tiers relocate the whole
10+
* composition into that copy — `entity.ts` and `queries.ts` carry the composer's body verbatim, so
11+
* every primitive they call is public by construction. The routes tier does NOT: its composition
12+
* (M:N junction traversal, TPH per-subtype route sets) stays in the engine as `renderRoutesFile`,
13+
* and the template's `customize:` note tells an adopter retargeting to another HTTP framework to
14+
* copy that body out of the package source.
15+
*
16+
* That instruction was UNTRUE. Measured 2026-09-07 against the published surface: of the sixteen
17+
* symbols `renderRoutesFile`'s body imports from inside the package, fourteen resolved from
18+
* `@metaobjectsdev/codegen-ts` and two did not — `routesHandlerName` and `TPH_POLYMORPHIC_VERBS`,
19+
* both `TS2305: has no exported member`. So the documented escape ended in a file that does not
20+
* compile, on the ONE tier every adopter outside Fastify and Hono has to retarget. The whole
21+
* ownership story is what the project offers instead of a codegen package per framework (FR-040),
22+
* and it was load-bearing exactly where it was broken.
23+
*
24+
* This gate DERIVES the required set from the body rather than restating it, because a hand-kept
25+
* list is the same defect one level up: `renderRoutesFile` gains an import, nobody re-reads this
26+
* file, and the escape silently breaks again.
27+
*
28+
* It is deliberately scoped to the compositions a template tells you to copy. A body nobody is
29+
* invited to copy owes nothing.
30+
*/
31+
32+
const PKG = join(import.meta.dir, "..");
33+
34+
/** Compositions a reference template instructs an adopter to copy out of the package source. */
35+
const COPYABLE_COMPOSITIONS = [
36+
{
37+
body: "src/templates/routes-file.ts",
38+
invitedBy: "src/reference/routes.ts",
39+
// The sentence in that template that makes the promise. Matched on the fragment that fits
40+
// one line, because the header is comment-wrapped and the full sentence spans two. If it is
41+
// reworded, this test's premise changed and the pairing has to be re-read rather than the
42+
// string patched.
43+
promise: "copy `renderRoutesFile`'s body out",
44+
},
45+
] as const;
46+
47+
/** Named imports a module takes from paths INSIDE the package (relative specifiers). */
48+
function internalNamedImports(source: string): string[] {
49+
const names = new Set<string>();
50+
// `import { a, type B, c as d } from "./x.js"` / "../y.js" — multi-line bodies included.
51+
const re = /import\s*(?:type\s+)?\{([^}]*)\}\s*from\s*"(\.[^"]*)"/g;
52+
for (const m of source.matchAll(re)) {
53+
for (const raw of m[1]!.split(",")) {
54+
const name = raw
55+
.replace(/^\s*type\s+/, "")
56+
.split(/\s+as\s+/)[0]!
57+
.trim();
58+
if (name) names.add(name);
59+
}
60+
}
61+
return [...names].sort();
62+
}
63+
64+
/** Everything the package's public entry re-exports, by name. */
65+
function publicSurface(): Set<string> {
66+
const index = readFileSync(join(PKG, "src/index.ts"), "utf8");
67+
const names = new Set<string>();
68+
for (const m of index.matchAll(
69+
/export\s*(?:type\s+)?\{([^}]*)\}\s*from\s*"[^"]*"/g,
70+
)) {
71+
for (const raw of m[1]!.split(",")) {
72+
const part = raw.replace(/^\s*type\s+/, "").trim();
73+
// `a as b` re-exports under b — the name an adopter can import.
74+
const name = (part.includes(" as ") ? part.split(/\s+as\s+/)[1] : part)?.trim();
75+
if (name) names.add(name);
76+
}
77+
}
78+
for (const m of index.matchAll(
79+
/export\s+(?:declare\s+)?(?:const|function|class|type|interface)\s+([A-Za-z_$][\w$]*)/g,
80+
)) {
81+
names.add(m[1]!);
82+
}
83+
return names;
84+
}
85+
86+
describe("a composition an adopter is told to copy imports only public API", () => {
87+
const surface = publicSurface();
88+
89+
for (const { body, invitedBy, promise } of COPYABLE_COMPOSITIONS) {
90+
test(`${body} — every internal import is reachable from the package entry`, () => {
91+
const template = readFileSync(join(PKG, invitedBy), "utf8");
92+
// The premise: the reference template really does invite the copy. If this fails, the
93+
// instruction moved or was withdrawn — decide which before touching the assertion below.
94+
expect(template).toContain(promise);
95+
96+
const required = internalNamedImports(readFileSync(join(PKG, body), "utf8"));
97+
expect(required.length).toBeGreaterThan(0);
98+
99+
const missing = required.filter((n) => !surface.has(n));
100+
expect(missing).toEqual([]);
101+
});
102+
}
103+
});

0 commit comments

Comments
 (0)