Skip to content

Commit ba72049

Browse files
committed
fix(cli): multiple: true takes a JSON column in both migration generators
`os generate types` honoured the flag and neither migration generator did, so one authored `Field.lookup({ multiple: true })` produced an array TS type and a scalar `VARCHAR(36)` / `table.uuid` column from the same config in the same run. The column authority is `driver-sql`, and its answer is the flag alone: `createColumn` short-circuits on `field.multiple` above its own per-type switch, `isJsonField` is `JSON_COLUMN_TYPES.has(type) || !!field.multiple`, and `fieldHasColumn` opens with the same check. Both generators now answer it in the same place — before the type is consulted — so the element type gets no vote. Deliberately NOT the spec's `isMultiValueField`: that is the ADR-0104 D1 value contract, gated on `MULTI_CAPABLE_TYPES`, and it would answer VARCHAR for a `text` field the driver gives a JSON column — the same drift one notch narrower. The per-type vocabularies are untouched; the disputed scalar answers stay byte-for-byte and are pinned as a scope fence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
1 parent a59b393 commit ba72049

4 files changed

Lines changed: 77 additions & 4 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
`os generate migration` now gives a `multiple: true` field a JSON column, in both formats. One authored field used to produce two incompatible answers from one config in one run: `Field.lookup({ reference: 'account', multiple: true })` emitted `account?: string[]` from `os generate types` and a scalar `VARCHAR(36)` / `table.uuid('account')` column from the two migration generators, because `multiple` appeared exactly four times in `generate.ts` and all four were on the TypeScript side — `fieldTypeToSql` did not even take the parameter. Nothing warned: the scaffold looks right, the generated TypeScript IS right, and only the column is wrong, so the first symptom was a write of an array into a scalar column. That is the `#field-zoo` failure one layer out — there the DDL switch and `isJsonField` had drifted into two lists inside the driver; here the platform and the *generated* DDL were the two lists.
6+
7+
The authority is the driver's, and it is the flag alone. `SqlDriver.createColumn` short-circuits on `field.multiple` **above** its own `switch (type)`; `isJsonField` is `JSON_COLUMN_TYPES.has(type) || !!field.multiple`; and `fieldHasColumn` opens with `if (field?.multiple) return true` under the comment "Mirrors `SqlDriver.createColumn` exactly … including `multiple` (a JSON column)". Three statements of one rule: a flagged field is a JSON column whatever its element type would have been. Both generators now answer it the same way and in the same place — before the type is consulted at all.
8+
9+
Deliberately **not** the spec's `isMultiValueField`. That predicate is the ADR-0104 D1 *value* contract ("is the persisted value an array") and gates on `MULTI_CAPABLE_TYPES`, so asking it here would answer `VARCHAR` for a `text` field flagged `multiple: true` while the driver gives that same field a JSON column — the identical drift one notch narrower. `FieldSchema` does not refuse the combination either (`multiple` is a plain `z.boolean()` on every field; only `radio` + `multiple` is refused by name), and the generators sit downstream of validation. The two questions have two different owners: the value shape is the spec's, the column is the driver's.
10+
11+
Nothing about the existing per-type vocabularies changes. The scalar answers — including the five that are separately disputed — are byte-for-byte what they were, and a new pin asserts that as a scope fence rather than leaving it to a reading of the diff. `generate-multiple-json-column.pin.test.ts` drives all three generators on one config and pins the agreement across every member of the spec's `MULTI_CAPABLE_TYPES` plus a type outside it, so the type-blindness of the rule is an assertion rather than a comment; it also reads the driver's two statements of the rule, so moving them there fails here.

packages/cli/src/commands/generate.ts

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,7 +1027,33 @@ const FIELD_TYPE_SQL_MAP: Record<string, string> = {
10271027
address: 'JSONB',
10281028
} satisfies Record<FieldType, string>;
10291029

1030-
function fieldTypeToSql(fieldType: string): string {
1030+
/**
1031+
* The column one field takes.
1032+
*
1033+
* `multiple` is answered FIRST, before the type is looked up at all, because
1034+
* that is what the platform does. `SqlDriver.createColumn` short-circuits on
1035+
* `field.multiple` ABOVE its own `switch (type)`; `isJsonField` is
1036+
* `JSON_COLUMN_TYPES.has(type) || !!field.multiple`; and `fieldHasColumn`
1037+
* opens with `if (field?.multiple) return true` under the comment "Mirrors
1038+
* `SqlDriver.createColumn` exactly ... including `multiple` (a JSON column)".
1039+
* Three statements of one rule: a flagged field is a JSON column whatever its
1040+
* element type would have been, so the element type gets no vote here either
1041+
* (#14829). Before this, one authored `Field.lookup({ multiple: true })`
1042+
* produced `account?: string[]` from `os generate types` and a scalar
1043+
* `VARCHAR(36)` column from this generator, in the same run.
1044+
*
1045+
* WARNING: this is deliberately NOT the spec's `isMultiValueField`. That is the
1046+
* ADR-0104 D1 VALUE contract ("is the persisted value an array"), gated on
1047+
* `MULTI_CAPABLE_TYPES`; asking it here would answer VARCHAR for a `text`
1048+
* field the driver gives a JSON column - the same drift one notch narrower.
1049+
* The column question belongs to the driver, and the driver's answer is the
1050+
* flag alone. `generate-multiple-json-column.pin.test.ts` pins both halves.
1051+
*
1052+
* The JSON spelling is READ from this table's own `json` entry rather than
1053+
* restated, so the two cannot drift about what a JSON column is spelled here.
1054+
*/
1055+
function fieldTypeToSql(fieldType: string, multiple?: boolean): string {
1056+
if (multiple) return FIELD_TYPE_SQL_MAP.json;
10311057
return FIELD_TYPE_SQL_MAP[fieldType] || 'TEXT';
10321058
}
10331059

@@ -1063,7 +1089,7 @@ export function generateMigrationSql(config: Record<string, unknown>): string {
10631089

10641090
const fieldLines: string[] = [];
10651091
for (const [fieldName, fieldDef] of Object.entries(fields)) {
1066-
const sqlType = fieldTypeToSql(String(fieldDef.type || 'text'));
1092+
const sqlType = fieldTypeToSql(String(fieldDef.type || 'text'), !!fieldDef.multiple);
10671093
const notNull = fieldDef.required ? ' NOT NULL' : '';
10681094
fieldLines.push(` "${fieldName}" ${sqlType}${notNull}`);
10691095
}
@@ -1117,8 +1143,21 @@ export function generateMigrationTs(config: Record<string, unknown>): string {
11171143
for (const [fieldName, fieldDef] of Object.entries(fields)) {
11181144
const fType = String(fieldDef.type || 'text');
11191145
const required = fieldDef.required ? '.notNullable()' : '.nullable()';
1120-
let colMethod: string;
11211146

1147+
// #14829 - `multiple` before the type, exactly as `SqlDriver.createColumn`
1148+
// does it: the driver short-circuits on the flag above its own per-type
1149+
// switch, so a flagged field is a JSON column whatever its element type
1150+
// would have been. Emitted here rather than as a switch arm because the
1151+
// switch cases on the TYPE and the type has no vote in this decision;
1152+
// the spelling is this generator's own JSON arm, stated once more.
1153+
// See `fieldTypeToSql` for why the authority is the driver's flag rule
1154+
// and not the spec's `isMultiValueField` value predicate.
1155+
if (fieldDef.multiple) {
1156+
lines.push(` table.jsonb('${fieldName}')${required};`);
1157+
continue;
1158+
}
1159+
1160+
let colMethod: string;
11221161
switch (fType) {
11231162
case 'text': case 'email': case 'phone': case 'url': case 'select':
11241163
case 'password': case 'color':

scripts/cross-package-test-inputs.mjs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,27 @@ export const CROSS_PACKAGE_TEST_INPUTS = {
391391
// One file, not `packages/create-objectstack/**`: the test reads that
392392
// template and nothing else across the boundary.
393393
'packages/create-objectstack/src/templates/blank/pnpm-workspace.yaml',
394+
// The two files that hold the COLUMN authority the CLI's migration
395+
// generators mirror, READ by
396+
// src/commands/generate-multiple-json-column.pin.test.ts (#14829). That
397+
// pin asserts a `multiple: true` field gets a JSON column from both
398+
// `os generate migration` formats because `SqlDriver.createColumn`
399+
// short-circuits on the flag ABOVE its per-type switch, and
400+
// `fieldHasColumn` mirrors that decision for the drift differ. Source-read
401+
// rather than imported: `createColumn` is `protected` and needs a knex
402+
// table builder, so driving it would mean a live driver and a built
403+
// `dist`, while the SHAPE of its decision — flag first, type second — is
404+
// exactly what has to stay true and is legible in the source.
405+
//
406+
// The declaration is the whole point of the pin, not paperwork around it:
407+
// if the driver moves that rule and cli's suite does not re-run, the two
408+
// sides drift again in silence, which is the #14829 defect returning by
409+
// the cache. Two files rather than `packages/drivers/driver-sql/src/**`:
410+
// the pin reads these two and nothing else across the boundary, and that
411+
// package's `src` is edited often enough that the subtree glob would put
412+
// cli's whole suite on every driver commit.
413+
'packages/drivers/driver-sql/src/sql-driver.ts',
414+
'packages/drivers/driver-sql/src/schema-drift.ts',
394415
],
395416
},
396417
'@objectstack/client': {

turbo.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,9 @@
107107
"$TURBO_ROOT$/scripts/cross-package-test-inputs.mjs",
108108
"$TURBO_ROOT$/packages/spec/src/system/translation.zod.ts",
109109
"$TURBO_ROOT$/scripts/check-cross-package-test-inputs.mjs",
110-
"$TURBO_ROOT$/packages/create-objectstack/src/templates/blank/pnpm-workspace.yaml"
110+
"$TURBO_ROOT$/packages/create-objectstack/src/templates/blank/pnpm-workspace.yaml",
111+
"$TURBO_ROOT$/packages/drivers/driver-sql/src/sql-driver.ts",
112+
"$TURBO_ROOT$/packages/drivers/driver-sql/src/schema-drift.ts"
111113
]
112114
},
113115
"@objectstack/client#test": {

0 commit comments

Comments
 (0)