Skip to content

Commit 4e183ec

Browse files
dmealingclaude
andcommitted
fix(codegen-ts): dbImport is a generator's need, not the model's
#194 made dialect and dbImport optional config, then demanded both from any model declaring a source.rdb. dialect belongs there -- every sourced object is lowered to SQL, so a wrong default is silently wrong everywhere. dbImport does not: only a generator emitting `import { db } from ...` can read it, and a project whose generated queries take `db` as an explicit parameter has no singleton to name. Those projects were refused, and told their model "generates database code" that in fact imported nothing. The cost, measured. The public reference app's automated dependency PR (0.20.11 -> 0.21.6) had been red since 15 August -- twelve days -- on exactly this error. The remedy is one config line, which no bot can write, so the app sat four minor lines behind with a failing PR nobody reads while every human-facing signal said it was healthy. Done by hand, that upgrade cost the one line and two generated deltas. dbImport is now demanded at the point of USE. Reading it is what proves a generator emits the import, so reading is what asks for it; the runner's existing `[<generator>]` wrapper names which one. Both read surfaces are covered (renderContext and ctx.config) because a generator may use either. Declaration is TRACKED, never inferred by comparing against DEFAULT_DB_IMPORT -- `meta init` scaffolds a relative path and a project may legitimately write the default's own string, which a value comparison would read as absent. Verified against the reference app with its dbImport line removed: published 0.24.2 fails, this build emits all 29 files byte-identically. Tests cover both directions -- a sourced entity with no db-importing generator emits clean, routesFile() still demands it and names itself, and an explicit "./db" is honoured rather than read as absent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NynrRND6ZUwGvq3ZUTCfxG
1 parent 9f6f6aa commit 4e183ec

3 files changed

Lines changed: 129 additions & 18 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,36 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
## [Unreleased]
99

10+
### Fixed — `dbImport` was demanded from the model, and blocked an upgrade nobody saw fail
11+
12+
`#194` made `dialect` and `dbImport` optional config, then required both from any model
13+
declaring a `source.rdb`. `dialect` belongs there: every sourced object is lowered to SQL, so a
14+
wrong default is silently wrong everywhere. **`dbImport` does not.** Only a generator emitting
15+
`import { db } from …` can read it, and a project whose generated queries take `db` as an
16+
explicit parameter — the shape the scaffolded `queriesFile()` emits — has no singleton to name.
17+
Such a project was refused, and told its model "generates database code for: …" that in fact
18+
imported nothing.
19+
20+
**What that cost, measured on the public reference app.** Its automated dependency PR
21+
(`0.20.11` → `0.21.6`) had been failing since **15 August**, twelve days, on exactly this:
22+
23+
```
24+
meta: verify --codegen: regeneration failed: codegen config is missing dbImport —
25+
required because this model generates database code for: CouncilTurn, Council.
26+
##[error]Process completed with exit code 1
27+
```
28+
29+
The remedy is one config line, which no bot can write — so the app sat four minor lines behind
30+
with a red PR nobody reads, while every human-facing signal said it was healthy. When the
31+
upgrade was finally done by hand it took that one line and two generated deltas.
32+
33+
`dbImport` is now demanded at the point of **use**: reading it is what proves a generator emits
34+
the import, so reading is what asks for it, and the runner's existing `[<generator>]` wrapper
35+
names which one. A generator that never touches it never demands it. Declaration is *tracked*,
36+
never inferred by comparing against the default — `meta init` scaffolds a relative path and a
37+
project may legitimately write the default's own string. Verified against the reference app:
38+
published `0.24.2` fails, this build emits all 29 files byte-identically.
39+
1040
### Fixed — `meta verify --codegen` convicted files MetaObjects never wrote
1141

1242
Found by re-running the documented TypeScript quickstart cold against published `0.24.2`, as a

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

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -242,32 +242,42 @@ export async function runGen(opts: RunGenOpts): Promise<RunGenResult> {
242242
}
243243

244244
// #194 — dbImport / dialect are optional config, BUT a model that declares a
245-
// source.rdb genuinely needs them. Only a sourced object emits database
245+
// source.rdb genuinely needs `dialect`. Only a sourced object emits database
246246
// artifacts (schema / query / route code), and every generator below gates
247247
// that emission on hasAnyRdbSource — the #248 R2 source-based predicate — so a
248248
// sourceless object (a value object, or a sourceless projection) emits zero DB
249249
// code. Guard BEFORE normalizeConfig fills its inert defaults, so a DB project
250-
// that forgot either gets a clear error naming the objects, never
251-
// silently-defaulted output (e.g. a Postgres project quietly emitting sqlite).
252-
// A model of only value objects and/or sourceless projections reaches
253-
// normalizeConfig and gets the harmless placeholders.
250+
// that forgot it gets a clear error naming the objects, never
251+
// silently-defaulted output (a Postgres project quietly emitting sqlite).
252+
//
253+
// `dbImport` is NOT part of this check, and the difference is where the answer
254+
// is needed. A dialect is a property of the MODEL: every sourced object is
255+
// lowered to SQL, and a wrong default is silently wrong everywhere. A dbImport
256+
// is a property of a GENERATOR: only one that emits `import { db } from …` can
257+
// read it, and a project whose generated queries take `db` as a parameter has
258+
// no singleton to name. Demanding it from the model convicted exactly those
259+
// projects — the error even told them their model "generates database code"
260+
// that in fact imported nothing. It is demanded below at the point of USE, by
261+
// the generator that reads it, which the runner names in the thrown message.
254262
const dbEmittingObjects = safeEntities.filter(
255263
(e) => !e.isAbstract && hasAnyRdbSource(e),
256264
);
257-
if (dbEmittingObjects.length > 0) {
258-
const missing: string[] = [];
259-
if (opts.config.dialect === undefined) missing.push("dialect");
260-
if (opts.config.dbImport === undefined) missing.push("dbImport");
261-
if (missing.length > 0) {
262-
const names = dbEmittingObjects.map((e) => e.name).join(", ");
263-
throw new Error(
264-
`codegen config is missing ${missing.join(" and ")} — required because this model ` +
265-
`generates database code for: ${names}. Set ${missing.join(" and ")} in ` +
266-
`metaobjects.config.ts. (A model of only value objects and/or sourceless projections may omit them.)`,
267-
);
268-
}
265+
if (dbEmittingObjects.length > 0 && opts.config.dialect === undefined) {
266+
const names = dbEmittingObjects.map((e) => e.name).join(", ");
267+
throw new Error(
268+
`codegen config is missing dialect — required because this model ` +
269+
`generates database code for: ${names}. Set dialect in ` +
270+
`metaobjects.config.ts. (A model of only value objects and/or sourceless projections may omit it.)`,
271+
);
269272
}
270273

274+
/** Did the AUTHOR declare a dbImport reachable by this target? Tracked, never
275+
* inferred by comparing against DEFAULT_DB_IMPORT — `meta init` scaffolds a
276+
* relative path and a project may legitimately write the default's own string,
277+
* which a value comparison would then read as absent. */
278+
const dbImportUndeclaredFor = (targetName: string): boolean =>
279+
(opts.config.targets?.[targetName]?.dbImport ?? opts.config.dbImport) === undefined;
280+
271281
// 2. Resolve targets + entity-module target.
272282
const config = normalizeConfig(opts.config);
273283

@@ -429,6 +439,25 @@ export async function runGen(opts: RunGenOpts): Promise<RunGenResult> {
429439
warn: (msg) => warnings.push(`[${generator.name}] ${msg}`),
430440
};
431441

442+
// The point of USE for an undeclared dbImport (see the guard above). Reading
443+
// it is what proves this generator emits a db-singleton import, so reading is
444+
// what asks for it; a generator that never touches it never demands it. The
445+
// throw travels through the `[${generator.name}]` wrapper below, so the message
446+
// names the generator that wants the path. Both surfaces are covered because a
447+
// generator may read either.
448+
if (dbImportUndeclaredFor(selfTarget.name)) {
449+
const demand = (): never => {
450+
throw new Error(
451+
`codegen config is missing dbImport — this generator emits ` +
452+
`\`import { db } from …\` and needs the module to import it from. Set dbImport in ` +
453+
`metaobjects.config.ts (or on this generator's target). A project whose generated ` +
454+
`queries take \`db\` as a parameter never needs it.`,
455+
);
456+
};
457+
Object.defineProperty(renderContext, "dbImport", { get: demand, configurable: true });
458+
Object.defineProperty(ctx.config, "dbImport", { get: demand, configurable: true });
459+
}
460+
432461
let files: EmittedFile[];
433462
try {
434463
files = await generator.generate(ctx);

‎server/typescript/packages/codegen-ts/test/vo-only-config-optional.test.ts‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,22 @@
22
// zero DB code, so requiring them was a dead-but-mandatory tsc obligation). A model
33
// that DOES emit DB code must still set them, and omitting them errors clearly rather
44
// than silently defaulting (which would e.g. emit sqlite for a Postgres project).
5+
//
6+
// The two halves later split on WHERE the answer is needed. `dialect` stays eager:
7+
// a wrong default silently emits the wrong SQL for every sourced object, so the model
8+
// alone settles it. `dbImport` is only ever read by a generator that emits
9+
// `import { db } from …`, and a project whose generated queries take `db` as a
10+
// parameter never has one — so demanding it from the MODEL convicted projects that
11+
// could not use it. It is now demanded at the point of USE, by the generator that
12+
// needs it, naming that generator.
513
import { describe, test, expect, beforeEach, afterEach } from "bun:test";
614
import { mkdtempSync, rmSync } from "node:fs";
715
import { tmpdir } from "node:os";
816
import { join } from "node:path";
917
import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata";
1018
import { runGen } from "../src/runner.js";
1119
import { entityFile } from "../src/generators/entity-file.js";
20+
import { routesFile } from "../src/generators/routes-file.js";
1221
import type { MetaobjectsGenConfig } from "../src/metaobjects-config.js";
1322

1423
let tmp: string;
@@ -82,10 +91,53 @@ describe("#194 item 3 — dbImport/dialect optional for value-object-only and so
8291
err = e as Error;
8392
}
8493
expect(err).toBeDefined();
85-
expect(err!.message).toContain("dialect and dbImport");
94+
expect(err!.message).toContain("dialect");
8695
expect(err!.message).toContain("User"); // names the DB-emitting object
8796
});
8897

98+
test("a DB entity with dialect but no dbImport generates when nothing emits a db import", async () => {
99+
// The shape the eager guard convicted: a real sourced entity, a generator that
100+
// takes `db` as a parameter rather than importing a singleton. Nothing can read
101+
// dbImport, so nothing may demand it.
102+
const root = await load(ENTITY);
103+
const cfg = { ...configNoDb(), dialect: "sqlite" as const } as MetaobjectsGenConfig;
104+
const result = await runGen({ config: cfg, metadata: root, projectRoot: tmp });
105+
expect(result.files.length).toBeGreaterThan(0);
106+
});
107+
108+
test("a generator that DOES emit `import { db }` still demands dbImport, and names itself", async () => {
109+
const root = await load(ENTITY);
110+
const cfg = {
111+
...configNoDb(),
112+
dialect: "sqlite" as const,
113+
generators: [routesFile()],
114+
} as MetaobjectsGenConfig;
115+
let err: Error | undefined;
116+
try {
117+
await runGen({ config: cfg, metadata: root, projectRoot: tmp });
118+
} catch (e) {
119+
err = e as Error;
120+
}
121+
expect(err).toBeDefined();
122+
expect(err!.message).toContain("dbImport");
123+
expect(err!.message).toContain("routes"); // the runner prefixes [<generator>]
124+
});
125+
126+
test("an explicit dbImport equal to the default is honoured, not read as absent", async () => {
127+
// Declaration is tracked, never inferred by comparing against DEFAULT_DB_IMPORT:
128+
// `meta init` scaffolds a relative path, and a project is free to write the same
129+
// string the default happens to use.
130+
const root = await load(ENTITY);
131+
const cfg = {
132+
...configNoDb(),
133+
dialect: "sqlite" as const,
134+
dbImport: "./db",
135+
generators: [routesFile()],
136+
} as MetaobjectsGenConfig;
137+
const result = await runGen({ config: cfg, metadata: root, projectRoot: tmp });
138+
expect(result.files.length).toBeGreaterThan(0);
139+
});
140+
89141
test("a DB entity WITH dbImport/dialect set generates normally (no regression)", async () => {
90142
const root = await load(ENTITY);
91143
const cfg = { ...configNoDb(), dbImport: "./db", dialect: "postgres" as const } as MetaobjectsGenConfig;

0 commit comments

Comments
 (0)