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.

props 与 properties 同现时,alias 优先级按「读法」相反 —— 配置袋读到 properties,React prop 读到 props #5123

Closed

Description

@yinlianghui

发现于 #4799(PR #5122)的实施途中,顺手实测,不在该单半径内,未在该 PR 修。

事实

一个节点同时写了 props 与 properties 两种拼写时,渲染器读哪一个,取决于它用哪条通道读,而两条通道的 alias 优先级正好相反:

  • 配置袋通道(schema.properties.x):element:* 渲染器的 readProps() 是
    { ...schema.props, ...schema.properties } —— properties 赢。
  • React prop 通道(组件直接收到的 x prop):SchemaRenderer 在 React.createElement
    处先 spread ...componentProps(其中含 evaluation memo 把 properties.* 提升到顶层的那份
    拷贝),再 spread ...(evaluatedSchema.props || {}) 覆盖 —— props 赢。

真实 SchemaRenderer 渲染探针实测(节点 props: { content: 'FROM_PROPS' } 与
properties: { content: 'FROM_PROPERTIES' } 同现,同一次渲染里同时读两条通道):

bagRead(properties.content)="FROM_PROPERTIES"  reactPropRead(content)="FROM_PROPS"

同一个节点、同一个键,两个答案。哪个上屏纯粹取决于该渲染器碰巧是 readProps() 家族
(elements.tsx / text-input.tsx / record-picker.tsx / data-list.tsx / metadata-viewer.tsx)
还是直接吃 spread 进来的 prop。

与 #4799 的关系

#4799 修的是「properties 根本不参与表达式求值」;PR #5122 落地后两边的值都会被求值,
但上面这条优先级分歧是独立的、且 PR #5122 既没有引入也没有改变它(该 PR 显式保持
readProps 的 properties 优先级不变)。因此单列。

为什么值得看一眼

这是 alias 家族(#4795 / #4797 / #4799 / #4786)的同一根问题的另一面:props 被注释明确称作
legacy alias、properties 是规范拼写,但「规范拼写赢」这件事只在一半的读法上成立。按
contract-first(AGENTS.md #0.1),两条通道要么给同一个答案,要么其中一条根本不该存在。

复现

packages/react/src/SchemaRenderer.tsx 的 React.createElement 调用处(spread 顺序)
对照 packages/components/src/renderers/basic/elements.tsx 的 readProps()。上面的读数由一个
临时探针组件取得(同时打印 schema.properties.content 与它自己收到的 content prop),
探针未入库。

备注

未认领,交 PM 分诊定级 —— 触发条件是作者两种拼写同时写,频次未知;但一旦发生,症状是
「换个渲染器同一份 JSON 结果不同」,不容易被归因。

Activity

  1. added theissue type on Aug 18, 2026
  2. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    Maintainer ruling (2026-08-18, live PM chat with the triage seat, session session_01EUZUjvnLsqwHVkXkTv8TKQ; verbatim: 「其他接受你的建议」, applying the triage recommendation delivered for this card).

    Ruled: properties wins on BOTH channels — one answer per key. The canonical spelling's precedence must hold on the React-prop channel too (today ...(evaluatedSchema.props || {}) spreads last and inverts it); the config-bag channel's properties-wins order is already correct and stays. This restores the declared posture (props is annotated legacy alias) rather than changing it.

    Dispatch requirements: (1) before the fix, scan the repo + examples for nodes carrying BOTH spellings and attach the count — any real co-occurrence sites get called out in the PR since their rendered output may change; (2) pin the invariant cross-channel (same key, both read paths, one winner) so the inversion cannot regrow; (3) note the adjacency to #4795's pending question ② (whether the properties envelope is an official ui:* authoring channel) — this ruling fixes precedence, it does not answer that. Promoted to pm:queue (type stays Bug).


    Generated by Claude Code

  3. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    Deferred this round — recording the reason and the trap now rather than when it comes up again (deferring is not shelving).

    Why deferred: this card's fix surface is packages/react/src/SchemaRenderer.tsx + packages/components/src/renderers/basic/elements.tsx. #5253 is in flight in packages/components (src/renderers/form/form.tsx). The maintainer's 2026-08-18 subdirectory/file-level serialization exemption is app-shell-only and ⛔ does not extend to packages/components, so whole-package serialization applies and these two cannot run concurrently. Not a judgement on the card — purely the queue.

    The trap the next dispatch needs to know about, so it is not rediscovered:

    The two channels disagree in opposite directions, and the fix is not "pick one spelling" —

    channel precedence site
    config bag (schema.properties.x) properties wins readProps() = { ...schema.props, ...schema.properties }
    React prop (x arriving as a prop) props wins SchemaRenderer's createElement: ...componentProps first, then ...(evaluatedSchema.props || {}) overwrites

    So whichever way you unify it, one of the two currently-working readings changes behaviour. The readProps() family is five named files (elements.tsx, text-input.tsx, record-picker.tsx, data-list.tsx, metadata-viewer.tsx); everything else takes the spread path. That asymmetry is why this is not a one-line alias fix and why it wants its ruling read before dispatch, not during.

    Also relevant to whoever picks it up: PR #5122 (#4799) neither introduced nor changed this — it explicitly preserved readProps's properties precedence. So the card's premise is not a side effect of that merge, but the tree has moved since 2026-08-18 02:22 and the spread order at createElement must be re-read on current main rather than trusted from this body.

    Still pm:queue, unassigned, no label change.


    Generated by Claude Code

  4. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    Claim: PM loop round 12 — the deferral above is now resolved
    Session: session_01RV6yuVCxymHYE16PL9vQkE
    Branch: claude/issue-5123-properties-wins-both-channels
    Worktree: objectui-issue-5123
    Domain: repo:objectui (react / components)
    File surface: packages/react/src/SchemaRenderer.tsx (the createElement spread order) + the readProps() family in packages/components/src/renderers/ + tests (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus
    Clause-②: no — the maintainer's ruling frames this as restoring the declared posture (props is an annotated legacy alias), not changing it. No key gains or loses acceptance; only which of two co-present spellings wins. Flagging it here because it is the closest call of this round: the rendered output can change for any node that writes both, which is exactly why requirement (1) below is a hard pre-condition rather than a nicety.
    Serial constraints cleared: the blocker named in my deferral comment above is gone. packages/components was held by #5253; its PR #5276 merged, so the package is free. packages/react was never held. The three other in-flight cards are in packages/plugin-report (#5225), packages/core + plugin-charts (#4507), and apps/console (#4303) — all disjoint. Open PRs #5277 (packages/fields) and #5278 (app-shell/src/providers) likewise disjoint.

    The maintainer's ruling and its three binding dispatch requirements are carried into the order verbatim — in particular requirement (1): count the co-occurrence sites before touching the fix, because those are the nodes whose rendered output this change can alter, and an unmeasured zero there is not a reading.


    Generated by Claude Code

  5. os-support-ai commented on Aug 19, 2026

    @os-support-ai
    Collaborator
    {
      "issue": 5123,
      "status": "done",
      "branch": "claude/issue-5123-properties-wins-both-channels",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5284",
      "premise_still_valid": true,
      "summary": "Re-read the createElement spread order on current main (97fba31a2) rather than trusting the card: unchanged, and both channels still disagree in opposite directions. Fixed the React-prop channel only, exactly as ruled — the alias spread is narrowed via propsWithoutCanonicalKeys(evaluatedSchema.props, evaluatedSchema.properties) so `props` no longer overrides a key the canonical `properties` bag also declares; readProps()'s config-bag order is untouched. Implemented as a SUBTRACTION from the props bag rather than a re-spread of `properties` after it, because `properties.*` is already in componentProps having passed the metadata strip — re-spreading the raw bag would push stripped metadata (e.g. dataSource) back out as React props, the objectstack#5576 regression. Requirement (1) measured before the fix: ZERO co-occurrence nodes in shipped metadata or source; the single site in the whole repo is the objectui#4799 test fixture, whose rendered output does not change (it asserts the bag read, which is unchanged) and which passes unmodified. Verified the readProps() family on current main: the five named files are all present and all still `{ ...fromProps, ...fromProperties }` — plus a SIXTH that has appeared, packages/plugin-detail/src/renderers/record-alert.tsx, which merges `{ ...schema, ...schema.properties }`; that is a different expression of the same properties-wins order, so it agrees with the ruling and needed no change (its suite is green). Requirement (3) honoured: #4795 question ② is untouched and explicitly not answered, in both the PR body and both test files.",
      "tests": "All measured at aca492561, the FINAL commit, re-run after the last commit. (1) `pnpm exec vitest run packages/react packages/components` from the REPO ROOT (not a package-level filter, objectui#3378): 213 files, 2152 tests, all passed, exit 0. (2) `pnpm exec vitest run packages/plugin-detail packages/layout packages/plugin-designer` (the packages that actually author a `properties:` bag, plus the sixth readProps family member): 112 files, 1057 tests, all passed. (3) `pnpm --filter @object-ui/react --filter @object-ui/components type-check` exit 0 — the hyphenated script name was echoed for BOTH packages, so this is not a zero-match silent pass. (4) same filter `lint` exit 0, 0 errors (340/891 pre-existing warnings). (5) same filter `build` exit 0. (6) gates: check-control-bytes (4672 files), check-changeset-presence, check-changeset-no-major, check-changeset-fixed, check-type-check-coverage, check-lint-coverage — all pass. Changeset: patch on @object-ui/react, never major. NEW TESTS: 12 across two files, both asserting the SAME key through BOTH read paths (requirement 2) — the central assertion is an equality between channels, expect(bagRead).toBe(reactPropRead), so re-inverting either side fails even if per-channel expectations are edited. PRE-FIX MEASUREMENT: the react file was run before the fix and reproduced the card's probe exactly — bagRead='FROM_PROPERTIES' while reactPropRead='FROM_PROPS', 3 failed / 5 passed. REVERSE VERIFICATION, predicted before running: 5 of 12 red (3 co-occurrence cases in react, plus badge spread-path and cross-family equality in components), 7 single-spelling/carve-out cases staying green. OBSERVED: exactly that — 5 failed, 7 passed, the 5 being the 5 named ones ('AssertionError: expected FROM_PROPS to be FROM_PROPERTIES'). ABLATION REBUILD STATEMENT: no rebuild is in the ablation's resolution path, and it cannot hide a stale result — the root vitest.config.mts aliases @object-ui/react to packages/react/src (line 252), so both suites resolve from SOURCE, never dist/. Not taken on trust: the ablation ran with dist/ STILL CONTAINING THE FIXED BUILD (verified: 2 occurrences of the new helper in packages/react/dist/SchemaRenderer.js while src had 0). It went red anyway — positive proof dist is not consulted; a stale artifact in the path would have produced a false GREEN. Fix then restored by `git checkout <branch> -- <path>` and confirmed byte-identical to HEAD (`git diff --quiet` clean) before the final measurement. Reverse verification was run from a COMMITTED state, so the restore had a recovery point.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Requirement (1) — the co-occurrence measurement, with its counter-probe

    The ruling made this a pre-condition, so recording the numbers here as well as in the PR. Method: JSON.parse + recursive object walk for every .json; a brace-depth, string- and comment-aware object-literal scan for every .ts/.tsx/.js/.jsx/.mjs/.md/.mdx. Both passes validated against a positive control (a synthetic node written both ways) — both detected it, so the detector is not dead.

    corpus files parsed nodes with BOTH only props only properties
    examples/ (JSON) 432 0 0 0
    whole repo (JSON) 546 0 1 2
    whole repo (source literals) 3924 scanned 1 (test fixture) — —

    Counter-probe — the zeros are readings, not an empty scan:

    • examples/: the same walk parsed 2277 "type" keys across 428 files. That corpus authors config flat on the node ({ "type": "statistic", "label": …, "value": … }) and contains zero occurrences of either spelling as a key — so the zero there is structural, not a miss.
    • Whole repo: 800 files contain props, 344 contain properties, 12 contain readProps; 3 JSON nodes carry exactly one of the two spellings; 28 non-test source literals declare a properties: bag against 1 declaring a props: bag.

    Blast radius: no authored site changes rendered output. The one co-occurrence node in the repo is packages/react/src/__tests__/SchemaRenderer.propertiesExpressions.test.tsx (the objectui#4799 pin). Its output does not move — it asserts the config-bag read, which this change deliberately leaves alone — and it passes unmodified in the full run above. The dispatch's honest risk ("one of the two currently-working readings changes behaviour") therefore lands on no existing site, which is why this did not need the stop-and-report escalation.

    Notes for the PM

    • A sixth readProps() implementation exists on current main — packages/plugin-detail/src/renderers/record-alert.tsx, { ...schema, ...schema.properties }. Not one of the five named in the deferral comment. It already agrees with the ruling (properties wins), so it needed no edit, but the "five named files" list is now stale and worth correcting wherever it is carried forward.
    • PR fix(react): canonical properties wins on the React-prop channel too — one answer per key on both channels (#5123) #5284 is draft, Fixes #5123. Labels applied by the bot and read back: package: react, package: components, tests. No skip-changeset label was created or applied — this PR ships a real changeset, and per the dispatch that label mechanism does not exist in this repo.
    • Base main advanced during the run (branched at 97fba31a2, PR base now 92b327a67). CI convergence is left to the PM per the 2026-08-10 ruling; report delivered at draft-PR time, no CI polling.

    Generated by Claude Code

  6. os-support-ai commented on Aug 19, 2026

    @os-support-ai
    Collaborator

    ACCEPT — PR #5284 flipped ready, enqueued, merged. All 21 checks resolved: 19 success, 2 skipped by design.

    Requirement (1), the maintainer's hard pre-condition, came back zero — and it is the right kind of zero. No node in shipped metadata or source carries both spellings. The single co-occurrence site in the whole repo is #4799's test fixture, and its rendered output does not change (it asserts the bag read, which this PR leaves alone); it passes unmodified. So the blast radius the ruling was guarding against is genuinely empty here.

    Requirement (2) is pinned the way a cross-channel invariant has to be: the central assertion is an equality between channels — expect(bagRead).toBe(reactPropRead) — so re-inverting either side fails even if someone later edits the per-channel expectations. 12 tests across two files. Requirement (3): #4795's question ② is untouched and explicitly not answered, in the PR body and both test files.

    Three things here are better than what my dispatch order asked for:

    1. It avoided a regression neither the ruling nor I anticipated. The obvious way to make properties win on the React-prop channel is to re-spread properties after props. That would have been wrong: properties.* is already in componentProps, having passed the metadata strip — re-spreading the raw bag would push stripped metadata (e.g. dataSource) back out as React props, which is the objectstack#5576 regression. It implemented the fix as a subtraction from the props bag instead.

    2. A sixth readProps() family member exists, and it was checked. My order said the family was five named files and that a sixth would matter. packages/plugin-detail/src/renderers/record-alert.tsx merges { ...schema, ...schema.properties } — a different expression of the same properties-wins order, so it already agrees with the ruling and needed no change. Verified, not assumed.

    3. The ablation proved dist was not in the resolution path rather than asserting it. It ran with packages/react/dist/SchemaRenderer.js still containing the fixed build (2 occurrences of the new helper in dist while src had 0) and went red anyway. A stale artifact in the path would have produced a false green — so this is a positive control, not a claim. Predicted 5 of 12 red by name; observed exactly those 5.

    The pre-fix measurement also reproduced the card's original probe exactly — bagRead='FROM_PROPERTIES' while reactPropRead='FROM_PROPS' — so the defect this closes is the one that was filed.


    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

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions