Uh oh!
There was an error while loading. Please reload this page.
refactor(test-support): one shared home for the Zod wrapper-key list - #7022
Merged
Conversation
The literal wrapper-key list `['in','out','innerType','schema','left','right']` had five copies: three TypeScript test files and two `.mjs` CI gate scripts. The copies spanned a language boundary, so the objectui#5872 class-(1) pattern was unavailable across it -- `@object-ui/test-support`'s `exports["."]` is TypeScript source, and a bare `node scripts/check-*.mjs` has no build artefact to reach. Per the 2026-08-31 ruling on objectui#6923, the DATA gets a build-free home and the walks stay with their callers: - `packages/test-support/src/zod-wrapper-keys.json` holds the list; - `packages/test-support/src/zod-wrapper-keys.ts` holds the reasoning and types it for TypeScript consumers; `index.ts` re-exports `ZOD_WRAPPER_KEYS`; - a new `exports` subpath, `./zod-wrapper-keys`, points straight at the JSON so a bare-node gate can resolve it. The workspace root declares the package so the bare specifier resolves from `scripts/`. The gates read it via `createRequire` rather than an attributed JSON import: this module is loaded both by `node` and by Vite's SSR transform (its own pin tests), and under the latter the attributed import yields no default export -- measured, as "__vite_ssr_import_N__.default is not iterable". Constraint 4 of the ruling -- empty the list and the gate must go RED -- is paid by `scripts/__tests__/zod-wrapper-keys.shared.test.ts`, which drives one fixture per key through both gates' real entry points and asserts an unlisted spelling raises `ExtractionError`. It uses fixtures rather than the installed spec on purpose: measured against @objectstack/spec@17.2.0, `ui.ActionSchema` and `automation.FlowNodeSchema` need a wrapper hop but `data.FieldSchema` and `data.ObjectSchema` expose `.shape` at depth 0, so a spec-anchored counter-test would be vacuous for the designer gate today. Sharing a FUNCTION across the language boundary is explicitly outside that ruling and is not attempted here; the two walks stay local and stay different. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GgDDqh6YnkXqsnVTCa7wHk
Running this file's own ablation caught it: `it.each(onDisk)` generates its cases FROM the list under test, so with the list emptied it generated NONE. The suite reported fewer tests and stayed green on exactly the part that carries the discrimination -- this card's own defect, reproduced inside its counter-example. Replaced with one looping test per gate, each asserting the non-vacuity floor IN THE SAME TEST. No data, no cases, but the floor still fails. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GgDDqh6YnkXqsnVTCa7wHk
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
zhuangjianguo
marked this pull request as ready for review
August 31, 2026 13:30
Uh oh!
There was an error while loading. Please reload this page.
os-sam pushed a commit
that referenced
this pull request
Aug 31, 2026
Append-collision with PR #7022 (`refactor(test-support): one shared home for the Zod wrapper-key list`), which landed on main after this branch was cut and touches the same package barrel and README. Both sides kept, in two files: - `packages/test-support/src/index.ts` -- exports BOTH #7022's `ZOD_WRAPPER_KEYS` (with its docblock) and this branch's `enumOptions, shapeEnumOptions`. - `packages/test-support/README.md` -- this branch's rewritten `spec-enum-options.ts` entry (two exports, one walk) AND #7022's two `zod-wrapper-keys` entries, in main's ordering. One sentence of this branch's text was corrected rather than kept verbatim: it said the wrapper-key walk is "NOT confined here yet and still hand-copied", which #7022 made false. The merged sentence carries #7022's fact (the wrapper-key list is now shared as DATA) and this branch's remaining half (array-element unwrapping is still hand-copied). `pnpm-lock.yaml` auto-merged; `pnpm install` left it unchanged, so it needed no regeneration and none was hand-written. No behaviour from either side was dropped or altered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#6923
Implements the 2026-08-31 ruling — option B narrowed to data only. The five numbered constraints are each answered below; constraint 4 is the one that carries the weight, and it changed the shape of the test twice.
What the census actually found
The dispatch flagged its own census as incomplete and told me not to inherit it. Re-derived on
origin/main, the literal list['in','out','innerType','schema','left','right']had exactly five copies:scripts/check-action-forward-parity.mjs.mjsgatescripts/check-designer-field-key-parity.mjs.mjsgatepackages/core/src/actions/__tests__/actionKeys.pin.test.tspackages/app-shell/.../inspectors/flow-node-config.spec-reconciliation.test.tspackages/app-shell/.../previews/flow-canvas-seeds.spec-parse.test.tsxThe PM's zero-hit probe for single-quoted
'innerType'is falsified — three of those sites spell it that way. The otherinnerTypehits in the tree are single-spelling walks (while (cur?._def?.innerType)), not this list, and are deliberately left alone: they are a different reader class.After this change the literal list exists in one place; a repo-wide grep for the pattern returns zero other hits.
The shape
packages/test-support/src/zod-wrapper-keys.json— the data. Zero build step.packages/test-support/src/zod-wrapper-keys.ts— the reasoning, and the typedZOD_WRAPPER_KEYSthe TypeScript side reads.index.tsre-exports it (constraint 3).exportsgains"./zod-wrapper-keys": "./src/zod-wrapper-keys.json"(constraint 2).private: trueunchanged.Two decisions the ruling left open, and why they went this way
JSON, not
.mjs. Both were allowed..mjswas rejected on a measurement:index.tsre-exporting from a.mjsis TS7016 in every consumer's program (the root config setsallowJs: false), so it would have cost eitherallowJsin each of the nine dependent packages or a hand-written.d.mts— the "second source of truth, free to drift silently" thattsconfig.scripts.json's own header already argues against.resolveJsonModuleis already on repo-wide, so JSON types itself with no declaration file and no build. Verified:--listFilesshowszod-wrapper-keys.tsinside both@object-ui/core's and@object-ui/app-shell's type-check programs, with noallowJsanywhere.node scripts/check-*.mjs可直接解析". Measured:@object-ui/test-supportwas a devDependency of nine packages but not of the workspace root, so there was nonode_modules/@object-uiat the root and the bare specifier resolved from nowhere inscripts/. Adding it to rootdevDependenciesis what creates the link. That is the mechanism the ruling names, so it is in scope; flagging it because it is a manifest change the card did not anticipate.check:phantom-depsdocuments this exact root-declaration allowance for tooling imports and stays green.packages/corealso had to declare the package (its sibling consumers already did).createRequire, notimport … with { type: "json" }. Load-bearing, and measured: these gate modules are imported both bynode(the gate run) and by Vite's SSR transform (their own pin tests). Under the latter the attributed JSON import yields no default export and the walk dies with__vite_ssr_import_N__.default is not iterable.createRequireis the idiomloadSpecSchemasin the same file already uses.Constraint 4 — the counter-example, and the two things it caught
⭐ A test that only proves "both sides import the same list" passes just as well when the list is empty.
scripts/__tests__/zod-wrapper-keys.shared.test.tsdrives one fixture per key through each gate's real entry point (specShapeKeys; andschemaAcceptSet's ownimportSpecseam, since that gate keeps its walk private and the ruling says to leave it there), plus a negative: an unlisted wrapper spelling must raiseExtractionError, never return a clean empty verdict.Why fixtures and not the installed spec — this is the finding that decided the design. Measured against
@objectstack/spec@17.2.0, walking with the list emptied:ui.ActionSchemanullautomation.FlowNodeSchemanulldata.FieldSchemadata.ObjectSchemaFieldSchema/ObjectSchemaexpose.shapeat depth 0 and never need a wrapper hop. So a counter-test anchored on the installed schemas would be vacuous for the designer gate today, and could go vacuous for the others the next time upstream unwraps something — silently, which is the whole complaint. Fixtures cannot rot that way.The ablation, and the defect it found in the counter-test itself
Emptying
zod-wrapper-keys.jsonto[](mutation confirmed on disk:innerTypecount 0,[]count 1, blob2aac729c→fe51488c), on the committed implementation:Restore verified by observation, not exit code:
git hash-objectback to2aac729c(= the HEAD blob) andgit diff HEADempty.The first ablation run found a real defect in this PR's own test. The per-key checks were written as
it.each(onDisk)— which generates its cases from the list under test, so with the list emptied it generated none. The suite reported fewer tests and stayed green on precisely the part carrying the discrimination: this card's defect, reproduced inside its own counter-example. Fixed inc83904d5b— one looping test per gate, with the non-vacuity floor asserted in the same test, so no-data still fails. Re-ablated: 12 failing tests instead of 10, and bothevery entry in the list is load-bearingtests now among them, including the designer one — the leg the real designer gate cannot supply today.Constraint 5 — the boundary is respected
Only data moved. Each caller keeps its own walk, and they are legitimately not identical: the designer gate reads
node._def ?? node.def ?? node._zod?.def, the action gate readss._def ?? s.def. Unifying those is sharing a function across the language boundary, which the ruling explicitly does not open a door for. Both gate diffs are two lines: an import and a loop head.Verification
Union run after the final commit, at
c83904d5b, tree clean.Gates, exit code captured before any pipe:
check-action-forward-parity,check-designer-field-key-parity,check-phantom-dependencies,check-pre-install-import-graph,check-control-bytes,check-changeset-presence— allEXIT=0. Also green:check-readme-exports(on a built tree),check-governed-queue-guard --testover all 16 changed paths (NOT GOVERNED),check-self-import,check-side-effects-array,check-vi-mock-specifiers,check-vi-mock-inherit,check-changeset-{fixed,no-major,overwrite},check-lint-coverage,check-type-check-coverage.Type-check:
type-check:scripts, and@object-ui/{test-support,core,app-shell}— allEXIT=0(the last two only after building their dependency closures; unbuilt they fail on missingpackages/types/dist, which is an unbuilt-tree condition, not this diff).--listFilesconfirms all five edited/added test files are genuinely inside those programs rather than excluded.Lint — a declared narrowing, with its evidence.
eslint --no-inline-config --format jsonover all 9 changed lintable files: 9 linted, 0 ignored, 0 errors, 0 warnings. The narrowing is measurable rather than a gap becauseeslint.config.jsconfigures no type-aware linting (noprojectService, noparserOptions.project), so a file's verdict depends only on its own text plus the flat config — this diff cannot move the verdict on any file it did not touch. The repo-wide sweep remains CI's run.Changeset: empty frontmatter — test files, CI gate scripts and a
private: truepackage only; nothing releases.check-changeset-presenceconfirms the exemption is complete.Two reds hit locally, neither from this diff, both already tracked
check:published-dist—@object-ui/fieldsshipsdist/__tests__/numberInputBrowserReadings.d.ts. Already filed twice:check:published-distis RED onmain:@object-ui/fieldsshipsdist/__tests__/numberInputBrowserReadings.d.tsin its tarball #6943, finding(fields):@object-ui/fieldsemitsdist/__tests__/numberInputBrowserReadings.d.tsinto its published tarball, socheck:published-distis red on the release path #6861.check-sdui-registration-pins.test.ts— fails on any tree wherepackages/app-shell/distexists (my earlier build created it; removing it makes the test pass). Already filed: finding(test-infra):check-sdui-registration-pins.test.tsfails on any tree wherepackages/app-shell/distexists #6893.⛔ No new issue filed: dedup ran against the 294 open issues via the repo-scoped REST list plus a local grep, with a control term that had to hit and did. #6699 is untouched.
Generated by Claude Code