refactor(react): one config-bag predicate, and a pin against a seventh spelling - #6769
Merged
Merged
Conversation
…h spelling "Is this value a real config bag (an object, not an array, not null)?" was asked in packages/react in six places in four spellings. All six now read one exported predicate, `utils/configBag.ts`, and `utils/configBag.pin.test.ts` fails when a new spelling appears anywhere in `packages/react/src`. The two sites that ask the same SHAPE question about a data ROW are deliberately not merged; the pin names them with that reason. No behaviour change: every spelling computed the same answer, and each site was ablated individually to confirm what it is load-bearing for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
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
|
os-sales
marked this pull request as ready for review
August 29, 2026 11:08
This was referenced Aug 29, 2026
finding(react): five spellings of one "is this a real config bag?" predicate in packages/react
#6761
Closed
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 free
to 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 #6761
Arm taken: converge. One exported predicate, all six occurrences reading it, and a pin that fails when a seventh spelling appears.
Re-derived on this branch's base (
b98352a15), and the count is six, not fiveThe card names five sites. On its own base (
b76ca6764) and on mine there are six:utils/unevaluatedExpression.tscarries two —scanBag's negated early return (the one the card names) and thehoisteddedupe read 40 lines below it, spelled positively. The file was counted once. The sixth is not a rounding error: ablating it silences a diagnostic (site 6 below).b98352a15SchemaRenderer.tsxmodule-privateisConfigBag(already the merge of thepropertiesguard +propsWithoutCanonicalKeys, #6752 / PR #6763)SchemaRenderer.tsxwinningVisibilityKey, inline!= nullutils/propsBagDiagnostic.tsprivate twin (#6708)utils/unevaluatedExpression.tsscanBag(#4795)utils/unevaluatedExpression.tshoisted— not in the card's inventory(#6752 merged what the card calls sites 1 and 2 into row 1 before this branch started, so the surviving textual count is six occurrences in four spellings.)
!= nulland truthiness cannot disagree here, and the difference is worth naming rather than smoothing over: they part company only on a falsy value, and every falsy value exceptdocument.allfailstypeof x === 'object'in the very next conjunct. The texts differ; the answers cannot.What landed
packages/react/src/utils/configBag.ts— the one definition, with the history and the equivalence argument.packages/react/src/utils/configBag.pin.test.ts— the part that matters. It scans every production source underpackages/react/srcfor thetypeof x+Array.isArray(x)conjunction on the same operand, in either order and either polarity, and requires the set to equal a declared allowlist. A seventh spelling fails it. A second leg pins that the three converged modules still import the predicate, so deleting the import and re-inlining trips leg 1 and deleting the check entirely trips leg 2.Deliberately NOT merged: the two row sites
SchemaRenderer'sboundRecordscope entry andusePredicateRecordContext(hooks/useExpression.ts) ask the same SHAPE question about a data row, not a config bag. Their answer carries its own pinned meaning — binding NOTHING rather than an empty row is what keeps a host-suppliedrecordfrom being shadowed. If "config bag" ever narrows (rejecting a class instance, say), those two must not follow; merging them would make that one edit with two rulings behind it. They are named in the pin's allowlist with that reason, so the next reader finds a decision rather than an oversight.No behaviour change, and the #6752 pin is the oracle
SchemaRenderer.degeneratePropsBag.test.tsx(from #6752 / PR #6763) is untouched by this PR —git diff --name-onlyover the commit does not list it — and green. It is also demonstrably the oracle: it is the file that goes red in three of the nine ablations below.Ablations — every call site, one at a time
Method per site: commit first, then replace
isConfigBag(x)at exactly one call site with bare truthiness, prove the mutation reached disk (anchor count 1 to 0, mutation count 0 to 1, plusgit diff --numstat), run the targeted suite, restore withgit checkout HEAD -- ABSOLUTE_PATH, prove the restore withgit diff HEADempty. Driver in atry/finallyso a kill mid-mutation still restores. No build step is involved:vitest.config.mtsaliases@object-ui/*to each package'ssrc/, and five of the nine ablations going red is the live proof that a source mutation reaches the run.Targeted suite = the 7 files that cover these modules; baseline
7 passed / 123 passed,VITEST_EXIT=0.propertiesevaluation guard (row 1)degeneratePropsBagschema.propertiesstops holding the authored valuepropsevaluation guard (row 1, #6752's twin)degeneratePropsBagpropsWithoutCanonicalKeys,propsbag (row 1)degeneratePropsBagpropsWithoutCanonicalKeys,propertiesbag (row 1)winningVisibilityKey(row 2)collectDroppedPropsKeys, authored bag (row 3)propsBagDiagnosticcollectDroppedPropsKeys, outgoing bag (row 3)scanBagearly return (row 4)unevaluatedExpressionDiagnostichoisteddedupe read (row 5)The four with no coverage, measured directly rather than assumed
Reading a diff shows structure, not which half is load-bearing (#6752's lesson). Each was measured through the real renderer or the exported function, guarded vs ablated, same probe both times:
propsWithoutCanonicalKeys'propertiesguard — load-bearing. Nodeprops: { '0': 'ZERO', title: 'T' }withproperties: 'ab': guarded the element receives0: "ZERO"; ablated it receives0: "a". Withproperties: ['X']:0: "ZERO"guarded,0: "X"ablated. A degenerate bag was subtracting the author's own key and the hoist's copy took its place.winningVisibilityKey's guard — NOT load-bearing. Five shapes ('visible',['${data.missing}'],42,true, and a real bag), rendered outcome andconsole.warntext byte-identical both ways. It is doubly dominated:hasOwnPropertyon a primitive or an array cannot hold a key namedvisible/hidden/ …, and its only consumer is already gated onrawPropertiesBag, which isisConfigBag-derived. Defence-in-depth, kept (converging it costs nothing and removes a spelling), but its reason is now stated honestly rather than assumed by symmetry.collectDroppedPropsKeys' outgoing guard — load-bearing for a direct caller, unreachable from the renderer. Direct calls: guarded returnsnullfor a string and for an array outgoing bag; ablated returns["0"..."8"]and["0","1"]— exactly the "tells the authorschema.0is undefined, which is true and useless" failure the module's docblock names for the authored bag. FromSchemaRendererit cannot fire, becausepropsWithoutCanonicalKeyshas returned an object at every exit since finding(react): a non-objectpropsis object-spread into indexed React props —props: "text"reaches the element as0,1,2, … #6752.hoisted) — load-bearing.collectUnevaluatedExpressions({ '0': '${data.x}' }, ['${data.x}']): guarded reports the residual expression on theschemachannel; ablated returns no findings at all — the dedupe read['${data.x}']['0'] === '${data.x}'matches and skips it. Ablating the spelling the card did not count silences a diagnostic.Commands and exit codes
All heavy runs serialized through the container's shared verify lock; verdict lines quoted, not a bare
$?.The full-suite and gate runs above were made at
f6b22b4db, this branch's final commit, on a clean tree.Two measurements that would otherwise be "green but nothing measured":
no-explicit-anyinSchemaRenderer.tsx, at lines this PR does not touch; the two new files report zero messages.type-checkreally covers the new files:tsc -p tsconfig.test.json --listFileslists bothutils/configBag.tsandutils/configBag.pin.test.ts.Declared narrowing: repo-wide
pnpm lintwas not run locally — eslint was scoped to the five changed files. The narrowing is a measurement, not an omission: (1) the population is eslint's own config,files: ['**/*.{ts,tsx}']plus itsignores, not my guess at what counts; (2) the file count comes from--format json(5 results); (3) the config declares noparserOptions.projectand noprojectService, so linting is not type-aware and this diff cannot move the verdict for any file it does not contain. CI runs the whole farm regardless.Generated by Claude Code