Skip to content

Commit d0e43af

Browse files
dmealingclaude
andcommitted
fix(migrate,runtime,tanstack): six bugs found by a downstream D1/uuid adopter
All six reported against 0.15.20 and confirmed on main; failing tests written first for each. Three turned out worse than reported. migrate-ts — data integrity (the dangerous ones): 1. uuid-identity PK's synthesized DEFAULT was diffed as an ordinary default, so EVERY uuid-PK table drifted on EVERY run with zero metadata changes. The emitter synthesizes the DEFAULT at DDL time (sqlite/d1 `lower(hex(randomblob(16)))`; postgres `gen_random_uuid()`) and expected-schema deliberately never records it (uuid generation is modeled as `identity`), but introspection reads it back as a real expr default — so the two always disagreed. On SQLite/D1 that false positive is DESTRUCTIVE, not just noisy: with no ALTER COLUMN, the recreate-and-copy path rebuilt the whole table (CREATE __new_x / INSERT SELECT / DROP / RENAME) for every already-migrated uuid-PK table, on every incremental migrate. NOT SQLITE-ONLY as reported: postgres emits a bogus ALTER every run too — the guard is dialect-agnostic. 2. D1 introspection didn't exclude Cloudflare/wrangler infrastructure tables. `_cf_METADATA` appears the moment any write touches a local D1, and D1's authorizer then denies even pragma_table_info against it (SQLITE_AUTH) — since readTableInfo runs per enumerated table, this ABORTED every second-and-later `meta migrate --dialect d1` (invisible until the first incremental run, because the first migration is the write that creates it). `d1_migrations` is queryable, so it merely read as an undeclared "extra" table — and the diff proposed DROP TABLE on wrangler's own bookkeeping. Filter in the query, so `_cf_METADATA` is never even fetched. 3. SQLite quoted every literal default regardless of column type, so `field.boolean @default false` emitted `DEFAULT 'false'` on a NUMERIC-affinity column. SQLite cannot coerce that, stores TEXT, and `col = 0` then silently misses the row (verified against real sqlite3). Numeric defaults hit the same path but were merely invisible — affinity coerces a numeric-looking '0', which is exactly why this sat unnoticed. Defaults are now rendered per the column's SQL type (boolean -> unquoted 1/0; numerics unquoted; text still quoted and escaped; non-numeric junk on a numeric column falls back to quoting). An existing golden had PINNED the buggy shape; updated with the reason, after proving the new output correct in real SQLite. Postgres is left as-is deliberately: it casts a quoted literal at DDL-parse time, so it is not producing wrong data, and churning its DDL output would be risk for no gain. runtime-ts — a server-side runtime bug: 4. The read-only (projection) mounts coerced the GET-by-id path param through `Number(id)` unconditionally, so any uuid/text PK 404'd for every id that actually exists — the row is right there in the same projection's list response, but its own detail route can never find it (with libsql it throws outright). AFFECTED BOTH MOUNTS, hono and drizzle-fastify, not just hono. Root cause is tidy: runtime-ts already has `parseId`, used by every WRITABLE mount; only the read-only ones bypassed it. Added a column-aware `coerceIdForColumn` (reads the PK's real `dataType` off the Drizzle column, so existing generated code is fixed with no regen and no option change) plus `rawIdLiteral` for the raw-SQL view path, which previously 400'd a perfectly valid uuid. codegen-ts-tanstack — wrong generated types: 5. The hooks template hardcoded `id: number` at every emission site, so a uuid PK generated hooks typing the key as a number while the row's real id is a string — a genuine tsc error at every consumer call site. `getPkInfo(entity, ctx)` already existed and is used correctly by the queries template; the hooks template just never called it. Threaded through all three render paths (read-only, writable, TPH) — more sites than reported, including the bare `number` delete-mutation variable types. 6. The grid-hook template's `sorting[0].id` failed `noUncheckedIndexedAccess` (a `.length > 0` guard doesn't narrow an index access). Fixed with real narrowing rather than a non-null assertion; goldens regenerated after confirming the diff is exactly that and semantically identical. Full suite 5056 pass / 0 fail; typecheck clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hd5NWZ1rKau63vJzrBSpLb
1 parent 92709fb commit d0e43af

18 files changed

Lines changed: 641 additions & 54 deletions

File tree

server/typescript/packages/codegen-ts-tanstack/src/templates/grid-hook-file.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,9 @@ export function ${hookName}() {
110110
for (const f of columnFilters) {
111111
filterObj[f.id] = f.value as unknown;
112112
}
113-
const sort = sorting.length > 0
114-
? \`\${sorting[0].id}:\${sorting[0].desc ? "desc" : "asc"}\`
113+
const sortEntry = sorting[0];
114+
const sort = sortEntry
115+
? \`\${sortEntry.id}:\${sortEntry.desc ? "desc" : "asc"}\`
115116
: undefined;
116117
return ${buildFilterQsSym}({
117118
...filterObj,

server/typescript/packages/codegen-ts-tanstack/src/templates/hooks-file.ts

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
entityModuleSpecifier,
99
isTphDiscriminatorBase,
1010
tphPlan,
11+
getPkInfo,
1112
} from "@metaobjectsdev/codegen-ts";
1213

1314
/**
@@ -150,6 +151,9 @@ function capitalize(s: string): string {
150151

151152
function renderReadOnlyHooksFile(entity: MetaObject, entityModule: string, ctx: RenderContext): string {
152153
const entityName = entity.name;
154+
// The id parameter must follow the entity's DECLARED primary-key type — a uuid/string
155+
// PK typed as `number` is simply wrong for the data (and a tsc error at every call site).
156+
const pkType = getPkInfo(entity, ctx).tsType;
153157
const entityNamePlural = pluralize(entityName);
154158
const lcEntity = entityName.charAt(0).toLowerCase() + entityName.slice(1);
155159
const keysVar = `${lcEntity}Keys`;
@@ -176,13 +180,13 @@ export const ${keysVar} = {
176180
lists: () => [...${keysVar}.all(), "list"] as const,
177181
list: (filter?: ${entityName}Filter) => [...${keysVar}.lists(), filter ?? {}] as const,
178182
details: () => [...${keysVar}.all(), "detail"] as const,
179-
detail: (id: number) => [...${keysVar}.details(), id] as const,${relationKeyLine}
183+
detail: (id: ${pkType}) => [...${keysVar}.details(), id] as const,${relationKeyLine}
180184
};
181185
`;
182186

183187
const queries: Code = code`
184188
export function use${entityName}(
185-
id: number,
189+
id: ${pkType},
186190
opts?: Omit<${useQueryOptionsSym}<${entityName}Row>, "queryKey" | "queryFn">,
187191
): ${useQueryResultSym}<${entityName}Row> {
188192
const fetcher = ${useEntityFetcherSym}();
@@ -222,6 +226,9 @@ export function use${entityNamePlural}(
222226

223227
function renderFullHooksFile(entity: MetaObject, entityModule: string, ctx: RenderContext): string {
224228
const entityName = entity.name;
229+
// The id parameter must follow the entity's DECLARED primary-key type — a uuid/string
230+
// PK typed as `number` is simply wrong for the data (and a tsc error at every call site).
231+
const pkType = getPkInfo(entity, ctx).tsType;
225232
const entityNamePlural = pluralize(entityName);
226233
const lcEntity = entityName.charAt(0).toLowerCase() + entityName.slice(1);
227234
const keysVar = `${lcEntity}Keys`;
@@ -254,13 +261,13 @@ export const ${keysVar} = {
254261
lists: () => [...${keysVar}.all(), "list"] as const,
255262
list: (filter?: ${entityName}Filter) => [...${keysVar}.lists(), filter ?? {}] as const,
256263
details: () => [...${keysVar}.all(), "detail"] as const,
257-
detail: (id: number) => [...${keysVar}.details(), id] as const,${relationKeyLine}
264+
detail: (id: ${pkType}) => [...${keysVar}.details(), id] as const,${relationKeyLine}
258265
};
259266
`;
260267

261268
const queries: Code = code`
262269
export function use${entityName}(
263-
id: number,
270+
id: ${pkType},
264271
opts?: Omit<${useQueryOptionsSym}<${entityName}Row>, "queryKey" | "queryFn">,
265272
): ${useQueryResultSym}<${entityName}Row> {
266273
const fetcher = ${useEntityFetcherSym}();
@@ -308,8 +315,8 @@ export function useCreate${entityName}(
308315
}
309316
310317
export function useUpdate${entityName}(
311-
opts?: Omit<${useMutationOptionsSym}<${entityName}Row, Error, { id: number; input: ${entityName}Update }>, "mutationFn">,
312-
): ${useMutationResultSym}<${entityName}Row, Error, { id: number; input: ${entityName}Update }> {
318+
opts?: Omit<${useMutationOptionsSym}<${entityName}Row, Error, { id: ${pkType}; input: ${entityName}Update }>, "mutationFn">,
319+
): ${useMutationResultSym}<${entityName}Row, Error, { id: ${pkType}; input: ${entityName}Update }> {
313320
const fetcher = ${useEntityFetcherSym}();
314321
const qc = ${useQueryClientSym}();
315322
return ${useMutationSym}({
@@ -327,8 +334,8 @@ export function useUpdate${entityName}(
327334
}
328335
329336
export function useDelete${entityName}(
330-
opts?: Omit<${useMutationOptionsSym}<void, Error, number>, "mutationFn">,
331-
): ${useMutationResultSym}<void, Error, number> {
337+
opts?: Omit<${useMutationOptionsSym}<void, Error, ${pkType}>, "mutationFn">,
338+
): ${useMutationResultSym}<void, Error, ${pkType}> {
332339
const fetcher = ${useEntityFetcherSym}();
333340
const qc = ${useQueryClientSym}();
334341
return ${useMutationSym}({
@@ -359,6 +366,9 @@ export function useDelete${entityName}(
359366

360367
function renderTphHooksFile(base: MetaObject, ctx: RenderContext, baseModule: string): string {
361368
const baseName = base.name;
369+
// The id parameter must follow the entity's DECLARED primary-key type — a uuid/string
370+
// PK typed as `number` is simply wrong for the data (and a tsc error at every call site).
371+
const pkType = getPkInfo(base, ctx).tsType;
362372
const lcBase = baseName.charAt(0).toLowerCase() + baseName.slice(1);
363373
const keysVar = `${lcBase}Keys`;
364374
// Single source of truth for discriminator field + subtypes + route segments.
@@ -399,20 +409,20 @@ export const ${keysVar} = {
399409
lists: () => [...${keysVar}.all(), "list"] as const,
400410
list: (filter?: ${baseName}Filter) => [...${keysVar}.lists(), filter ?? {}] as const,
401411
details: () => [...${keysVar}.all(), "detail"] as const,
402-
detail: (id: number) => [...${keysVar}.details(), id] as const,
412+
detail: (id: ${pkType}) => [...${keysVar}.details(), id] as const,
403413
subtypeLists: (sub: string) => [...${keysVar}.all(), sub, "list"] as const,
404414
// filter is loosely typed here (cache-key identity only); the per-subtype
405415
// hooks below type it precisely as <Sub>Filter.
406416
subtypeList: (sub: string, filter?: unknown) => [...${keysVar}.subtypeLists(sub), filter ?? {}] as const,
407417
subtypeDetails:(sub: string) => [...${keysVar}.all(), sub, "detail"] as const,
408-
subtypeDetail: (sub: string, id: number) => [...${keysVar}.subtypeDetails(sub), id] as const,
418+
subtypeDetail: (sub: string, id: ${pkType}) => [...${keysVar}.subtypeDetails(sub), id] as const,
409419
};
410420
`;
411421

412422
// Polymorphic reads — return the discriminated union.
413423
const polymorphic: Code = code`
414424
export function use${baseName}(
415-
id: number,
425+
id: ${pkType},
416426
opts?: Omit<${useQueryOptionsSym}<${baseName}>, "queryKey" | "queryFn">,
417427
): ${useQueryResultSym}<${baseName}> {
418428
const fetcher = ${useEntityFetcherSym}();
@@ -459,7 +469,7 @@ export function use${pluralize(subName)}(
459469
}
460470
461471
export function use${subName}(
462-
id: number,
472+
id: ${pkType},
463473
opts?: Omit<${useQueryOptionsSym}<${subName}>, "queryKey" | "queryFn">,
464474
): ${useQueryResultSym}<${subName}> {
465475
const fetcher = ${useEntityFetcherSym}();
@@ -490,8 +500,8 @@ export function useCreate${subName}(
490500
}
491501
492502
export function useUpdate${subName}(
493-
opts?: Omit<${useMutationOptionsSym}<${subName}, Error, { id: number; input: ${updateInput} }>, "mutationFn">,
494-
): ${useMutationResultSym}<${subName}, Error, { id: number; input: ${updateInput} }> {
503+
opts?: Omit<${useMutationOptionsSym}<${subName}, Error, { id: ${pkType}; input: ${updateInput} }>, "mutationFn">,
504+
): ${useMutationResultSym}<${subName}, Error, { id: ${pkType}; input: ${updateInput} }> {
495505
const fetcher = ${useEntityFetcherSym}();
496506
const qc = ${useQueryClientSym}();
497507
return ${useMutationSym}({
@@ -509,8 +519,8 @@ export function useUpdate${subName}(
509519
}
510520
511521
export function useDelete${subName}(
512-
opts?: Omit<${useMutationOptionsSym}<void, Error, number>, "mutationFn">,
513-
): ${useMutationResultSym}<void, Error, number> {
522+
opts?: Omit<${useMutationOptionsSym}<void, Error, ${pkType}>, "mutationFn">,
523+
): ${useMutationResultSym}<void, Error, ${pkType}> {
514524
const fetcher = ${useEntityFetcherSym}();
515525
const qc = ${useQueryClientSym}();
516526
return ${useMutationSym}({
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
{
2+
"metadata.root": {
3+
"package": "acme",
4+
"children": [
5+
{
6+
"object.entity": {
7+
"name": "Member",
8+
"children": [
9+
{ "field.uuid": { "name": "id" } },
10+
{ "field.string": { "name": "email", "children": [{ "view.text": {} }] } },
11+
{ "identity.primary": { "name": "id", "@fields": ["id"], "@generation": "uuid" } },
12+
{
13+
"layout.dataGrid": {
14+
"name": "default",
15+
"@pageSize": 25,
16+
"@filterable": true,
17+
"@columns": ["email"]
18+
}
19+
}
20+
]
21+
}
22+
},
23+
{
24+
"object.entity": {
25+
"name": "Subscriber",
26+
"children": [
27+
{ "field.long": { "name": "id" } },
28+
{ "field.string": { "name": "email", "children": [{ "view.text": {} }] } },
29+
{ "identity.primary": { "name": "id", "@fields": ["id"], "@generation": "increment" } },
30+
{
31+
"layout.dataGrid": {
32+
"name": "default",
33+
"@pageSize": 25,
34+
"@filterable": true,
35+
"@columns": ["email"]
36+
}
37+
}
38+
]
39+
}
40+
}
41+
]
42+
}
43+
}

server/typescript/packages/codegen-ts-tanstack/test/golden/__snapshots__/grid-filter/Subscriber.grid.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ export function useSubscriberActiveGrid() {
2929
for (const f of columnFilters) {
3030
filterObj[f.id] = f.value as unknown;
3131
}
32-
const sort =
33-
sorting.length > 0
34-
? `${sorting[0].id}:${sorting[0].desc ? "desc" : "asc"}`
35-
: undefined;
32+
const sortEntry = sorting[0];
33+
const sort = sortEntry
34+
? `${sortEntry.id}:${sortEntry.desc ? "desc" : "asc"}`
35+
: undefined;
3636
return buildFilterQs({
3737
...filterObj,
3838
...(sort !== undefined ? { sort } : {}),

server/typescript/packages/codegen-ts-tanstack/test/golden/__snapshots__/multi-grid/Program.grid.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ export function useProgramDefaultGrid() {
2828
for (const f of columnFilters) {
2929
filterObj[f.id] = f.value as unknown;
3030
}
31-
const sort =
32-
sorting.length > 0
33-
? `${sorting[0].id}:${sorting[0].desc ? "desc" : "asc"}`
34-
: undefined;
31+
const sortEntry = sorting[0];
32+
const sort = sortEntry
33+
? `${sortEntry.id}:${sortEntry.desc ? "desc" : "asc"}`
34+
: undefined;
3535
return buildFilterQs({
3636
...filterObj,
3737
...(sort !== undefined ? { sort } : {}),
@@ -79,10 +79,10 @@ export function useProgramCompactGrid() {
7979
for (const f of columnFilters) {
8080
filterObj[f.id] = f.value as unknown;
8181
}
82-
const sort =
83-
sorting.length > 0
84-
? `${sorting[0].id}:${sorting[0].desc ? "desc" : "asc"}`
85-
: undefined;
82+
const sortEntry = sorting[0];
83+
const sort = sortEntry
84+
? `${sortEntry.id}:${sortEntry.desc ? "desc" : "asc"}`
85+
: undefined;
8686
return buildFilterQs({
8787
...filterObj,
8888
...(sort !== undefined ? { sort } : {}),

server/typescript/packages/codegen-ts-tanstack/test/golden/__snapshots__/single-entity/Subscriber.grid.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ export function useSubscriberDefaultGrid() {
3030
for (const f of columnFilters) {
3131
filterObj[f.id] = f.value as unknown;
3232
}
33-
const sort =
34-
sorting.length > 0
35-
? `${sorting[0].id}:${sorting[0].desc ? "desc" : "asc"}`
36-
: undefined;
33+
const sortEntry = sorting[0];
34+
const sort = sortEntry
35+
? `${sortEntry.id}:${sortEntry.desc ? "desc" : "asc"}`
36+
: undefined;
3737
return buildFilterQs({
3838
...filterObj,
3939
...(sort !== undefined ? { sort } : {}),
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/**
2+
* The TanStack hooks template hardcoded `id: number` at every emission site — the
3+
* query-key `detail(id)` factory, `use<Entity>(id)`, and the update/delete mutation
4+
* variables — with nothing reading the entity's declared primary-key type.
5+
*
6+
* For a uuid/string PK (the normal case whenever `identity: "uuid"` is the project's
7+
* standard) the generated hooks therefore describe an `id: number` while the row's real
8+
* `<Entity>Row["id"]` is `string`. Application code calling
9+
* `useUpdate<Entity>().mutate({ id: someRealStringId, … })` is then a genuine `tsc` error
10+
* — the generated type is simply wrong for the data it describes, forcing consumers to
11+
* cast at every call site.
12+
*
13+
* codegen-ts already exports `getPkInfo(entity, ctx)` (the queries template uses it
14+
* correctly); the hooks template just never called it.
15+
*
16+
* Reported by a downstream consumer against 0.15.20.
17+
*/
18+
import { describe, test, expect } from "bun:test";
19+
import { resolve } from "node:path";
20+
import { tanstackQuery } from "../src/tanstack-query.js";
21+
import { makeRenderContext, buildPkMap, buildRelationMap } from "@metaobjectsdev/codegen-ts";
22+
import type { GenContext } from "@metaobjectsdev/codegen-ts";
23+
import { MetaDataLoader } from "@metaobjectsdev/metadata";
24+
import { FileSource } from "@metaobjectsdev/metadata/core";
25+
26+
/** Generate the hooks files for the fixture; returns path → content. */
27+
async function generate(): Promise<Map<string, string>> {
28+
const loader = new MetaDataLoader();
29+
const { root } = await loader.load([
30+
new FileSource(resolve(import.meta.dir, "fixtures", "pk-types.json")),
31+
]);
32+
const renderContext = makeRenderContext({
33+
dialect: "sqlite", loadedRoot: root, outDir: "/tmp",
34+
dbImport: "../db", extStyle: "none",
35+
pkMap: buildPkMap(root), relationMap: buildRelationMap(root),
36+
});
37+
const ctx: GenContext = {
38+
entities: root.objects(), loadedRoot: root,
39+
matches: () => true,
40+
config: { outDir: "/tmp", extStyle: "none", dbImport: "../db", dialect: "sqlite" },
41+
renderContext,
42+
warn: () => {},
43+
};
44+
const files = await tanstackQuery().generate(ctx);
45+
return new Map(files.map((f) => [f.path, f.content]));
46+
}
47+
48+
describe("tanstack hooks — the id parameter follows the entity's real PK type", () => {
49+
test("uuid PK emits `id: string`, never `id: number`", async () => {
50+
const member = (await generate()).get("Member.hooks.ts")!;
51+
expect(member).toContain("id: string");
52+
expect(member).not.toContain("id: number");
53+
});
54+
55+
test("uuid PK: the delete mutation's variable type is string, not number", async () => {
56+
const member = (await generate()).get("Member.hooks.ts")!;
57+
// useDeleteMember previously produced UseMutationResult<void, Error, number>
58+
expect(member).not.toMatch(/UseMutationResult<\s*void,\s*Error,\s*number\s*>/);
59+
expect(member).toMatch(/UseMutationResult<\s*void,\s*Error,\s*string\s*>/);
60+
});
61+
62+
test("uuid PK: the update mutation's variables carry a string id", async () => {
63+
const member = (await generate()).get("Member.hooks.ts")!;
64+
expect(member).toContain("{ id: string; input: MemberUpdate }");
65+
});
66+
67+
test("regression guard: an increment/long PK still emits `id: number`", async () => {
68+
const subscriber = (await generate()).get("Subscriber.hooks.ts")!;
69+
expect(subscriber).toContain("id: number");
70+
expect(subscriber).not.toContain("id: string");
71+
});
72+
});

server/typescript/packages/migrate-ts/src/diff/index.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,17 @@ function diffTableColumns(
265265
from: ac.nullable, to: ec.nullable, status: ALLOWED,
266266
});
267267
}
268-
if (!columnDefaultsEqual(ec.default, ac.default)) {
268+
// An `identity: "uuid"` PK's physical DEFAULT is synthesized purely at DDL-emit
269+
// time (sqlite/d1: `(lower(hex(randomblob(16))))`; postgres: `gen_random_uuid()`)
270+
// and is deliberately never recorded as a ColumnDefault on the expected side —
271+
// uuid generation is modeled as `identity`, not `default`. Introspection, however,
272+
// reads that DEFAULT back off the live table as a real `expr` default, so comparing
273+
// the two always disagrees — for every uuid-PK table, on every run, with zero
274+
// metadata changes. On SQLite/D1 the false positive is destructive rather than
275+
// merely noisy: there is no ALTER COLUMN, so the recreate-and-copy path rebuilds
276+
// the WHOLE table. Identity-driven values are not ordinary defaults — don't diff
277+
// them (`increment` gets this implicitly: an AUTOINCREMENT column has no DEFAULT).
278+
if (ec.identity !== "uuid" && !columnDefaultsEqual(ec.default, ac.default)) {
269279
const change: Change = {
270280
kind: "change-column-default", table, ...sx, column: name,
271281
status: ALLOWED,

0 commit comments

Comments
 (0)