Skip to content

Commit d138a6e

Browse files
committed
docs(cli): correct the reason the driver constants are transcribed
CORRECTING THE RECORD. Three commit messages on this branch, and three sites in the code, give as their FIRST reason for transcribing the driver's constants that those members reach no exported surface. That is false, and this queue composes the squash body from the branch's commit messages, so the sentences below would land in `main` as written: * 9cc1a76 -- "The transcription is still necessary, for two other reasons: the constant is `protected static`, and #5726 forbids a CLI production module any static value import of a driver package." * 11d0e8d -- "`MAX_VARCHAR_CHARS`, `MAX_KEYABLE_VARCHAR_CHARS`, `keyableTextLength`, `declaredVarcharLength` and `computeTenantField` are `protected` and reach no exported surface". * 722880a -- "Replaced with the real reasons: `MAX_VARCHAR_CHARS` is `protected static` and reaches no exported surface, and #5726 leaves a CLI production module only `await import()`". `protected` is a COMPILE-TIME visibility modifier. It removes a member from neither the exported class nor the published types. Measured on this worktree's built `packages/drivers/driver-sql/dist`: index.d.ts:5593 protected static readonly MAX_VARCHAR_CHARS = 16383; index.d.ts:5536 protected static readonly MAX_KEYABLE_VARCHAR_CHARS = 768; index.d.ts:5625 protected declaredVarcharLength(field: any): number | null; index.d.ts:5626 protected keyableTextLength(field: any): number | null; index.d.ts:3501 protected computeTenantField(schema: ...); require('.../driver-sql/dist/index.js').SqlDriver.MAX_VARCHAR_CHARS -> 16383 hasOwnProperty.call(SqlDriver, 'MAX_VARCHAR_CHARS') -> true All five are on the exported `SqlDriver`. The pin test already depends on this: it reaches the driver's own `protected` judgments by subclassing, which it could not do if they were absent from the published types. THE REAL CONSTRAINT, AND IT IS A CHOICE #5726 forbids a CLI production module any static value import of an `@objectstack/driver-*` package -- `schema-migrate.lazy-driver-import.test.ts` scans every non-test `.ts` under `packages/cli/src` -- and what it leaves open is `await import()` at the point of use. These generators are SYNCHRONOUS, so they cannot take it. That is the whole reason, and it is a property of how this package is written rather than of the constants: make the generators async and the transcription can go. This is the THIRD round on the same claim. Round 3's review flagged it, round 4 retracted it in the PR body and left it standing at three sites in the code, which is what produced this round. The PR body is not what the next author reads; the comment beside the constant is. WHAT MOVES -- comments and docblocks only. No behaviour, no test, no pin, no count, no changeset: * `generate.ts`, `MAX_VARCHAR_CHARS`'s docblock: the "protected static, so not on the driver package's exported surface at all" bullet is gone. The reason is now #5726 plus the synchronous generators, and the retracted claim is kept as a banned one beside the "does not depend on the driver at runtime" ban that preceded it, so nobody restates it a fourth time. * `generate.ts`, `isOrganizationScopedUnique`'s docblock: it drew a contrast -- "Unlike {@link MAX_VARCHAR_CHARS}, this one IS on `driver-sql`'s exported surface" -- that the measurement above dissolves. Both reach the exported surface, and both are spelled here for the one reason. Its tail also named the LEAF differential as what makes the spelling safe; the authority since 722880a is `SqlDriver.initObjects` read back with `PRAGMA table_info`, with the leaf differential kept beneath it and explicitly not the authority. * `generate.ts`, `indexKeyColumns`: the same stale attribution -- "the differential ... which now recomputes this whole set from the driver's own exported builders" -- now names the real chain and marks the leaf as not the authority. * `generate-string-family-width.pin.test.ts`: the restatement 722880a put there is retracted in place, beside the earlier false reason that comment already bans. `MAX_KEYABLE_VARCHAR_CHARS`'s docblock inherits by reference -- "Transcribed and pinned for exactly the reasons {@link MAX_VARCHAR_CHARS} gives" -- so it is corrected by the block it cites and needed no edit. Verified: `pnpm --filter @objectstack/cli typecheck` exit 0; the four pin files `Test Files 4 passed (4)` / `Tests 81 passed (81)`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
1 parent fd79a12 commit d138a6e

2 files changed

Lines changed: 47 additions & 22 deletions

File tree

packages/cli/src/commands/generate-string-family-width.pin.test.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -730,13 +730,18 @@ describe('#16091 — the character column both generators emit is the driver\'s'
730730
// ⛔ NOT "because `packages/cli` does not depend on the driver at runtime"
731731
// — it does, and this file imports it 500 lines up. That sentence was this
732732
// pin's own first answer, it is false, and `generate.ts` now carries it
733-
// with a ⛔ so nobody restates it. The ceiling is transcribed because
734-
// `MAX_VARCHAR_CHARS` is `protected static` on `SqlDriver` and reaches no
735-
// exported surface, and because #5726 leaves a CLI production module only
736-
// `await import()` for a driver package — which these SYNCHRONOUS
737-
// generators cannot use. This is the assertion that makes the transcription
738-
// safe: the two must be the same number, and a driver that moves fails here
739-
// rather than leaving the generators quietly wrong.
733+
// with a ⛔ so nobody restates it. ⛔ NOR "because `MAX_VARCHAR_CHARS` is
734+
// `protected static` on `SqlDriver` and reaches no exported surface" —
735+
// the second false answer, and this file refutes it by construction:
736+
// `protected` is compile-time visibility only, the member is on the
737+
// exported `SqlDriver` and in the package's `dist/index.d.ts`, and the
738+
// subclass below reaches the driver's `protected` judgments precisely
739+
// because they are still there. The ceiling is transcribed for ONE reason:
740+
// #5726 leaves a CLI production module only `await import()` for a driver
741+
// package, and these generators are SYNCHRONOUS, so they cannot use it — a
742+
// choice this package makes, not a law. This is the assertion that makes
743+
// the transcription safe: the two must be the same number, and a driver
744+
// that moves fails here rather than leaving the generators quietly wrong.
740745
const m = GENERATE_SOURCE.match(/^const MAX_VARCHAR_CHARS = (\d+);$/m);
741746
expect(m, 'generate.ts no longer declares MAX_VARCHAR_CHARS at top level').not.toBeNull();
742747
expect(Number(m![1])).toBe(MAX_CHARS);

packages/cli/src/commands/generate.ts

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,22 +1201,34 @@ const STRING_FAMILY_TYPES: ReadonlySet<string> = new Set(['email', 'url', 'phone
12011201
* (MySQL 8.0.46 refuses `varchar(16384)` with `ERROR 1074`; it is the LOWEST of
12021202
* the three dialects' ceilings and is applied to all of them deliberately).
12031203
*
1204-
* Transcribed rather than imported, for two independent reasons — the second
1205-
* one holds even if the first is ever lifted:
1204+
* Transcribed rather than imported for ONE reason, and that reason is a CHOICE
1205+
* this package makes rather than anything about the constant:
12061206
*
1207-
* - it is `protected static` on `SqlDriver`, so it is not on the driver
1208-
* package's exported surface at all;
12091207
* - #5726 forbids a CLI production module any static value import of an
12101208
* `@objectstack/driver-*` package, and `schema-migrate.lazy-driver-import.test.ts`
1211-
* enforces it. oclif `import()`s every command module on every invocation,
1212-
* so one such edge here charges an unbuilt driver to whatever command the
1213-
* operator actually ran.
1209+
* enforces it over every non-test `.ts` under `packages/cli/src`. oclif
1210+
* `import()`s every command module on every invocation, so one such edge
1211+
* here charges an unbuilt driver to whatever command the operator actually
1212+
* ran. What #5726 leaves open is `await import()` at the point of use —
1213+
* and these generators are SYNCHRONOUS, so they cannot take it. Make them
1214+
* async and the transcription can go.
12141215
*
12151216
* ⛔ NOT "because `packages/cli` does not depend on the driver at runtime" —
12161217
* it does: `@objectstack/driver-sql` is in this package's `dependencies` at
12171218
* `workspace:^`. A missing dependency was never the reason, and stating it as
12181219
* one invites the next reader to "simplify" the transcription away.
12191220
*
1221+
* ⛔ NOR "because it is `protected static`, so it reaches no exported surface"
1222+
* — this block gave that as its FIRST reason and it is false. `protected` is
1223+
* compile-time visibility only; it removes the member from neither the exported
1224+
* class nor the published types. `SqlDriver` is exported from
1225+
* `@objectstack/driver-sql`, `SqlDriver.MAX_VARCHAR_CHARS` is an own static
1226+
* property that reads `16383` at runtime, and the built `dist/index.d.ts`
1227+
* declares it as `protected static readonly MAX_VARCHAR_CHARS`. The pin test
1228+
* reaches this and the driver's other `protected` judgments by subclassing,
1229+
* which is the same point. The constant IS reachable; what is unavailable
1230+
* here is a SYNCHRONOUS import of it.
1231+
*
12201232
* Pinned rather than trusted: `generate-string-family-width.pin.test.ts` reads
12211233
* the constant out of `sql-driver.ts` and fails here if the two part.
12221234
*/
@@ -1343,16 +1355,21 @@ function isUniqueScopeDeclared(unique: unknown): boolean {
13431355
* ruling (2026-08-13), not an oversight, so the two readings stay apart here
13441356
* too.
13451357
*
1346-
* ⚠️ Unlike {@link MAX_VARCHAR_CHARS}, this one IS on `driver-sql`'s exported
1347-
* surface, so "not exported" is not the reason it is spelled here — say the
1348-
* real one instead of borrowing that block's: these generators are SYNCHRONOUS,
1358+
* ⚠️ "Not exported" is not the reason this is spelled here — and it is not the
1359+
* reason {@link MAX_VARCHAR_CHARS} is either: both reach `driver-sql`'s
1360+
* exported surface, that block says why, and neither is spelled here for it.
1361+
* The reason is one and the same for both: these generators are SYNCHRONOUS,
13491362
* and #5726 leaves a CLI production module only `await import()` for a driver
13501363
* package, which a synchronous function cannot use. Spec's own
13511364
* `isOrganizationUnique` is not a substitute either: it detects the WORD and
13521365
* not the scope, so it omits the bare `true` this predicate exists to include.
1353-
* What makes the spelling safe is the oracle in
1354-
* `generate-string-family-width.pin.test.ts`, which recomputes this whole key
1355-
* set from the driver's own exported builders and fails when the two disagree.
1366+
* What makes the spelling safe is the AUTHORITY in
1367+
* `generate-string-family-width.pin.test.ts` — `SqlDriver.initObjects` on an
1368+
* in-memory database, read back with `PRAGMA table_info` — which fails when
1369+
* the column the platform creates and the one these generators emit disagree.
1370+
* The leaf differential over the driver's exported builders is kept beneath it
1371+
* and is explicitly NOT the authority: recomposing the leaves' answers here is
1372+
* exactly what once left every layer between them and the real chain unread.
13561373
*/
13571374
function isOrganizationScopedUnique(unique: unknown): boolean {
13581375
return unique === true || unique === 'organization';
@@ -1443,8 +1460,11 @@ function indexKeyColumns(obj: Record<string, any>): ReadonlySet<string> {
14431460
// nullSafeColumns: ['zzz'] }` the driver keys `{f}` and this keyed
14441461
// `{organization_id, f}` — a column bounded here that the platform leaves
14451462
// unbounded, this card's defect pointed the other way. Found by the
1446-
// differential in `generate-string-family-width.pin.test.ts`, which now
1447-
// recomputes this whole set from the driver's own exported builders.
1463+
// differential in `generate-string-family-width.pin.test.ts`, whose
1464+
// authority is now the real chain — `SqlDriver.initObjects` on an
1465+
// in-memory database, read back with `PRAGMA table_info`. The leaf
1466+
// differential that recomputes this set from the driver's own exported
1467+
// builders is kept beneath that chain and is NOT the authority.
14481468
const preNormalized = Array.isArray(idx?.nullSafeColumns) && idx.nullSafeColumns.length > 0;
14491469
if (!preNormalized && idx?.unique === 'organization' && tenantField && !listed.includes(tenantField)) {
14501470
out.add(tenantField);

0 commit comments

Comments
 (0)