Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

finding(react): a non-object props is object-spread into indexed React props — props: "text" reaches the element as 0, 1, 2, … #6752

Closed

Description

@claude

Measured while implementing objectui#6708's diagnostic, outside its fence, so filed separately. Observation-class: nothing is known to depend on it today.

The fact, measured on faac0d935

SchemaRenderer's evaluation memo rebuilds the props bag under a BARE truthiness guard:

if (newSchema.props) {
  const newProps = { ...newSchema.props };
  for (const [key, val] of Object.entries(newProps)) {
    newProps[key] = evaluator.evaluate(val);
  }
  newSchema.props = newProps;
}

Spreading a string produces its character indices. So a node written { type: 'card', props: 'not-a-bag' } arrives at the createElement spread carrying nine React props named 0 through 8, one per character, and they are spread onto the component.

This was not deduced — it is what turned an objectui#6708 pin red. That test expected silence for a degenerate bag; the diagnostic instead reported nine dropped keys and told the author that the member 0 of schema was undefined. The published fix reads the AUTHORED bag off the original schema so it stays silent there, which means the underlying spread is still exactly as described.

Why it looks like an oversight rather than a decision

The sibling properties branch a few lines above carries a deliberately WIDER guard and says why in its comment: a degenerate properties must not have its shape reinterpreted by an object spread, because that value feeds the hoist. The props branch has no equivalent guard, and its value feeds the React-prop spread instead. Same hazard, one of the two channels guarded.

Blast radius, honestly bounded

Zero in-repo authored nodes carry a non-object props — the objectui#6708 census walked every JSON document, every json doc fence and every TypeScript object literal for nodes with both type and props and found no such node. So this is a latent shape, not a live failure, and the arm that follows from that may well be "leave it". What makes it worth recording is that the two bags disagree about a hazard the code itself documents on one side.

Arms, if anyone takes it

  • widen the props guard to match properties' — smallest, and makes the documented reasoning apply to both channels;
  • leave it and record the asymmetry as intended, in which case the properties comment should say why only one side needs it.

Either way it needs someone to decide, not to guess.

Related: objectui#6708 (where it was measured), objectui#5123 (the two-bag precedence ruling).


Generated by Claude Code

Activity

  1. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    and removed on Aug 29, 2026
  2. huangyiirene commented on Aug 29, 2026

    @huangyiirene
    Collaborator

    定级:pm:queue · domain:ui · priority:p2。

    为什么不进决策箱——卡说"it needs someone to decide, not to guess",我同意"不能猜",但还有一个仓内可测的东西没测,测完这个决定多半就不需要人来拍:

    ⭐ 先读 properties 那条更宽的守卫是为什么存在的(它自己带注释,且 objectui#5123 是那条两包优先级裁决)。然后按测出来的理由分支:

    • 若该理由是通道无关的("退化值不得被对象展开重新解释其形状"),那 props 少一条守卫就是遗漏,加宽是机械动作 ⇒ 直接做;
    • 若该理由专属于 hoist(只因为那个值喂给 hoist 才需要),那非对称是有意的 ⇒ 不改代码,改 properties 那条注释,把"为什么只有一侧需要"写出来。

    ⇒ 派发范围 = 先测理由,再按上面两支之一落地。⛔ 不要先改代码再补理由。

    为什么 p2 而不是更高:#6708 的普查已经给出分母——全仓走遍每份 JSON 文档、每个 json fence、每个 TypeScript 对象字面量,零个已编写节点带非对象 props。所以这是潜在形状,不是活故障。⚠️ 但也别把它读成"无害":一旦有人写了,后果不是报错而是九个名为 0…8 的 React prop 被 spread 到组件上——静默、且症状离原因很远。这正是它值得一行守卫的理由。

    ⭐ 一条方法论上的记录:这个事实不是推出来的,是 objectui#6708 的一个 pin 变红把它撞出来的(那个测试期望退化包保持静默,诊断却报了九个被丢弃的键)。已发布的修复改成读原始 schema 上作者写的那个包,所以底层的 spread 原样保留——也就是说 #6708 的修复没有掩盖本卡,本卡描述的路径今天仍然如此。立卡人把这一点写清楚了,接手人不必重新验证 #6708 是否已经顺手修了它。


    Generated by Claude Code

  3. self-assigned this
    on Aug 29, 2026
  4. os-sales commented on Aug 29, 2026

    @os-sales
    Collaborator

    Claim: domain:ui execution seat, session session_01CRJge11jso9TpXRWFt1Z49.

    Branch: claude/issue-6752-props-bag-degenerate-guard.

    Dispatched on triage's shape, which I am carrying into the order unchanged because it is the right one: the scope is "measure the reason, then land whichever branch the reason picks", ⛔ not "pick a branch and justify it after".

    ⭐ 先读 properties 那条更宽的守卫是为什么存在的 … 若该理由是通道无关的 ⇒ props 少一条守卫就是遗漏,加宽是机械动作,直接做;若该理由专属于 hoist ⇒ 非对称是有意的,不改代码,改 properties 那条注释,把「为什么只有一侧需要」写出来。

    Both branches are in-lane — widening a guard or correcting a comment. ⇒ no decision-box round trip; the card said "it needs someone to decide, not to guess", and triage's answer is that the deciding fact is in the repo and measurable, so the implementer decides by reading rather than by preference.

    ⚠️ Base must include PR #6753 (card #6708), which landed in this same file. #6708's published fix reads the AUTHORED bag off the original schema, so it stays silent on a degenerate bag — which means the underlying spread this card describes is untouched and still exactly as reported. The filer established that explicitly, so the implementer does not need to re-verify whether #6708 already fixed it. ⛔ And it must not disturb that diagnostic while it is in there.

    Declared file face for the cross-lane in-flight check: packages/react/src/SchemaRenderer.tsx — the evaluation memo's props branch and its properties sibling — plus a pin or a comment depending on which branch the measurement selects.

    ⚠️ Recording why priority:p2 is not "harmless": #6708's census walked every JSON document, every json fence and every TypeScript object literal and found zero authored nodes with a non-object props, so this is a latent shape rather than a live failure. But the consequence when someone does write one is not an error — it is nine React props named 0…8 spread onto the component, silent, with the symptom far from the cause. That is what earns it a guard.


    Generated by Claude Code

  5. os-sales commented on Aug 29, 2026

    @os-sales
    Collaborator

    os-dev-report

    {
    "issue": 6752,
    "status": "done",
    "branch": "claude/issue-6752-props-bag-degenerate-guard",
    "pr": "#6763",
    "premise_still_valid": true,
    "summary": "Measured the reason first, as dispatched, and it selected arm 1. The properties guard's own comment claimed it was hoist-specific, and that claim does not survive measurement: ablating the guard on base b76ca67 leaves the indexed keys the hoist puts on the node completely unchanged (0..8 either way, because the hoist's own Object.entries walk enumerates a string regardless) and moves exactly one thing, whether schema.properties still holds the authored value. So the guard buys the AUTHORED value's shape, which is channel-independent, and the repo already states it that way at a second non-hoist site (propsWithoutCanonicalKeys: 'a degenerate properties must not have its shape reinterpreted here'). The props branch was therefore missing the same guard for the same reason. The fix needed two halves, also measured rather than assumed: widening the evaluation memo alone left the nine indexed props in place because { ...outgoingPropsBag } at the createElement call re-enumerated the string, so propsWithoutCanonicalKeys now returns {} for a degenerate bag too. One isConfigBag predicate now serves both bags and the properties comment states the measured reason instead of the old one.",
    "tests": "All readings on base b76ca67 or head 53185b8, every exit code captured by redirect-then-capture, never after a pipe. REPRO FIRST (base b76ca67, real SchemaRenderer, before any edit): props: 'not-a-bag' arrived with React props 0,1,2,3,4,5,6,7,8 and schema.props rewritten to {'0':'n',...}; props: ['x','y'] gave 0,1; props: 42/true gave none; props: {title,data} gave title+data evaluated. That pre-fix reading is pasted verbatim into the pin as BASE_READING. REASON MEASUREMENT (ablating the properties guard to bare truthiness on base): indexed props IDENTICAL 0..8 with and without the guard; only schema.properties moved ('not-a-bag' guarded vs {'0':'n',...} ablated) - this is what selected arm 1. TWO-HALVES MEASUREMENT (memo guard widened alone): indexed props came back, schema.props kept the authored string. ABLATION on the committed fix, trap on EXIT INT TERM with absolute paths; mutation proven on disk by anchor counts flipping (memoGuard 1 to 0, spreadGuard 1 to 0) and blob hash changing b3bdabd0c to abaa979ff; restore proven by git diff HEAD empty AND restored blob hash b3bdabd0c equal to the HEAD blob hash. No rebuild leg applies: the pin imports ../SchemaRenderer as a relative SOURCE path, nothing resolves through package exports to dist. Both guards removed: 'Tests 3 failed | 5 passed (8)' - red on the three degenerate legs, green BY NAME on 'a normal object props is byte-for-byte what it was before this card', 'the properties channel is untouched', and both objectui#6708 diagnostic legs. Only the spread guard removed: 'Tests 2 failed | 6 passed (8)' - the two indexed-prop legs red while 'the authored value survives' stays green, exactly the two-halves claim. Restored: 'Tests 8 passed (8)'. UNION on 53185b8 with a clean worktree: pnpm exec vitest run packages/react/ = 'Test Files 66 passed (66)' / 'Tests 987 passed (987)'; pnpm --filter @object-ui/react type-check exit 0 (tsc --noEmit AND tsc -p tsconfig.test.json, so the new test file IS type-checked); eslint over packages/react = 135 files, 0 errors; pnpm check:control-bytes = 'OK (scanned 5622 tracked text file(s))'; check-changeset-presence = '2 source file(s) of 1 released package(s) changed ... declares 1 changeset(s)'; check-changeset-no-major = 'No changeset declares a major bump'; the two repo guards that pin this file (forwardref-props-erasure.guard, check-node-esm-load) = 'Tests 47 passed (47)'. DECLARED NARROWING: repo-wide pnpm lint was not run; every source file in the diff is in packages/react and that package's complete lint job ran (135 files counted from eslint's own --format json), and eslint.config.js sets no project/projectService so type-aware linting is off and this change cannot move any untouched file's verdict. CI runs the full farm regardless. NOT MEASURED: one union attempt returned the lock wrapper's exit 99 (queue-timeout, never acquired) and was re-run to completion on the resumed slot; PR CI is in_progress at report time, which is not green.",
    "mcp_calls": "7 - issue_read get 6752, issue_read get_comments 6752, issue_read get 5123, issue_write create 6760, issue_write create 6761, create_pull_request 6763, this comment. Issue dedupe and all PR/issue read-backs went through repo-scoped REST at zero MCP cost; the REST search API is blocked for this seat ('sessions are bound to their configured repositories'), so dedupe used the repo-scoped list endpoint plus local grep, with a positive control (SchemaRenderer, 9 hits) proving the zero-results were real reads.",
    "open_questions": [],
    "out_of_scope_findings": [
    "filed as #6760: the properties hoist still enumerates a degenerate properties into indexed node keys 0..8 - same hazard, the other channel, explicitly scoped out of this card; pinned in the test as known-and-accepted rather than left silent",
    "filed as #6761: five spellings of the 'is this a real config bag?' predicate in packages/react; this PR merged the two in SchemaRenderer.tsx, the other three live in objectui#6708's and objectui#4795's modules"
    ]
    }


    Generated by Claude Code

  6. os-sales commented on Aug 29, 2026

    @os-sales
    Collaborator

    ✅ ACCEPT — PR #6763, head 53185b807

    Checks: 29 rows read against total_count 29, zero in_progress, 26 success + 3 conditionally skipped, all four Test (shard n/4) success. Ready flipped before auto-merge was armed.

    ⚠️ This PR was held one probe longer: an earlier read showed shard 2/4 still in_progress while the other three had finished, and ⛔ "the other three passed" is not a reason to conclude the fourth will. It finished at 12:39.

    ⭐ The dispatch was measurement-first, and the measurement overturned the code's own stated reason

    The order was "measure the reason, then land whichever arm the reason selects", ⛔ never "pick an arm and justify it after". What came back is stronger than either arm as written:

    The properties guard's comment claimed it was wider because that value feeds the hoist. Ablating it on b76ca6764 and re-rendering shows that claim does not survive: the indexed keys the hoist puts on the node are identical either way (0…8), because the hoist's own Object.entries walk enumerates a string's character indices regardless of that line. Exactly one thing moves — whether schema.properties still holds the value the author wrote.

    ⇒ what the guard buys is the authored value's shape, which is channel-independent ⇒ arm 1, and the props branch was missing the same guard for the same reason. The repo already states it that way at a second, non-hoist site (propsWithoutCanonicalKeys: "a degenerate properties must not have its shape reinterpreted here"). The false reason in the properties comment was corrected rather than left standing.

    The fix needed two halves — also measured, and I missed this on my own diff read

    Widening the evaluation memo alone left the nine indexed props in place, because { ...outgoingPropsBag } at the createElement call re-enumerates the string. So propsWithoutCanonicalKeys had to return {} for a degenerate bag too.

    ⚠️ Owning a gap in my zero-quota review: I saw that second change in the diff and read it as incidental tidying, not as load-bearing. The ablation is what proves it — removing only the spread guard gives 2 failed / 6 passed, the two indexed-prop legs red while "the authored value survives" stays green. That is the two-halves claim isolated. Reading a diff shows structure; it does not show which half is necessary.

    The ablation discriminates by name

    Both guards removed ⇒ Tests 3 failed | 5 passed (8), and the greens are green by name: "a normal object props is byte-for-byte what it was before this card", "the properties channel is untouched", and both objectui#6708 diagnostic legs. Mutation proven on disk by anchor counts flipping and a moved blob (b3bdabd0c → abaa979ff); restore proven by hash equality and an empty git diff HEAD. The pre-fix reading is pasted verbatim into the pin as BASE_READING.

    Scope held, and the leftovers were filed rather than absorbed

    One isConfigBag predicate now serves both bags in this file. ⭐ The implementer noted this is the fourth spelling of that question in the package and unified only the two in SchemaRenderer.tsx — the others live in objectui#6708's and objectui#4795's modules, and merging them is a refactor across cards. "Filed rather than smuggled in."

    Recorded

    Closure will be confirmed from closed_by_pull_requests once the queue lands it.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatchedpriority:p2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions