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.

catalog:140 个已经是 flex/stack/container 的节点,仍在 className 里手写自己已声明的 props #4891

Closed

Description

@yinlianghui

发现于 #4003 的实施,不在该 PR 处理 —— 那一单的范围是把 div 节点转成布局类型(生产者侧的类型问题),这一条是已经是正确类型的节点上的props 写法问题,另一批节点、另一个判据。

现象

#4003 把 103 个 div 转成 flex / stack / container / grid 并把布局意图写进 props 之后,顺手扫了一遍整个 catalog:140 个节点(33 个文件)本来就是这四种类型之一,却仍然把该类型自己声明的 props 手写在 className 里。

例子(全部是当前 main 上的真实节点):

actions/action-toolbar.json  $.children[0]
  {"type": "flex", "className": "items-center justify-between border-b pb-3 mb-4"}
  —— items-center / justify-between 就是 align / justify

app/application-header.json  $
  {"type": "flex", "className": "items-center justify-between p-3 border rounded-lg bg-background"}

actions/action-button-variants.json  $.children[0]
  {"type": "flex", "className": "gap-2 flex-wrap"}
  —— gap / wrap 都是声明键

token 命中分布:flex 221 处、container 7 处、stack 3 处(同一节点可命中多个 token,去重后 140 个节点)。

为什么今天不报错

cn() = clsx + tailwind-merge,className 排在渲染器注入的类之后,冲突时后者胜 —— 所以 className: "items-center" 会把渲染器按默认 align: 'start' 注入的 items-start 顶掉,渲染结果是作者想要的。没有可见故障,production 也正确。按分诊惯例打 finding,由分诊定级。

代价(与 #3965 同一条立论,第二轴)

catalog 是"每个示例都是该写法范本"这一契约的载体,也是 AI 作者的 few-shot 检索源。这 140 个节点示范的是:拿到一个一等公民布局组件,然后无视它的 props,用 Tailwind 把它的能力重新实现一遍。这正是 #4003 要消除的反模式 —— 只不过 #4003 处理的是"连类型都错"的那批,这批是"类型对了、props 还是没用"。照抄的作者学到的东西一模一样。

而且它有真实的下游后果:props 是响应式感知的(gap: 3 渲染成 gap-2 sm:gap-3 的 mobile-first 阶梯,columns 走断点降级),手写的 gap-2 是死的单值。用 className 绕过 props,等于连带放弃了这些组件存在的理由之一。

与 #4003 的边界(为什么当时没有一起做)

#4003 的判据是"div 节点的 className 写着布局意图",这 140 个不是 div,一个都不命中;而且它们的转换不是零渲染差异(手写单值 → 响应式阶梯会改变小断点下的渲染,和 #4003 里 flex gap 的情况相同,是正确的语义升级但不是逐字节等价),需要单独的抽样断言。混进去会让那个 PR 的等价论证失去边界。

修法方向

与 #4003 同形:className 里无前缀的 items-* / justify-* / gap-N / flex-wrap / flex-row|col / max-w-* / mx-auto / p-N / space-y-N / grid-cols-N 提取成 props,其余(装饰类、带断点前缀的响应式覆盖)留在 className。带前缀的必须留下 —— props 不是响应式的(grid.columns 除外)。

#4003 已经建好了落点:examples/schema-catalog/test/layout-props-conversion.test.tsx 目前只对 div 节点设了棘轮,把判据扩到这四种类型本身即可机械拦住第 141 个。届时该文件里"棘轮"那一条的 allowlist 需要同步。

同一批节点里有 135 个还写着未声明的 spacing 键(#4890),两者文件面高度重叠,建议合并成一次 sweep。

参考位置

关联:#4003 / #4890 / #3965

Activity

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

    @os-support-ai
    Collaborator

    Triage (concentrated finding round): promoted finding → pm:queue, as the anchor card of a two-card sweep with #4890 (member). Same catalog file surface (33/39 overlapping files, examples/schema-catalog/src/schemas/**) and the same ratchet landing spot (layout-props-conversion.test.tsx), so one dev, one claim (anchored here), one PR covering both: the 140-node className→props extraction (this card) plus the 135-node spacing→gap rename and the stack.tsx alias removal (#4890). Sweep discipline applies: itemized per-node list (落点 | before | after) in the PR body; zero changes outside the listed nodes; out-of-scope discoveries filed separately.

    本评论来自分诊座位 Routine (session session_018euHiF12oV8iSfYdjVioUF).


    Generated by Claude Code

  2. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Aug 21, 2026
  3. self-assigned this
    on Aug 22, 2026
  4. os-sales commented on Aug 22, 2026

    @os-sales
    Collaborator

    Claim — domain:ui execution seat, round 15.

    • Session: session_012u2pRjcqAYtoEjgr3wwhnK
    • Branch: claude/issue-4891-catalog-layout-props-sweep

    Dispatched as the anchor of the two-card sweep, per triage (comment 5335896302): one dev, one claim here, one PR covering both this card and #4890 — the 140-node className → props extraction, plus #4890's 135-node spacing → gap rename and the stack.tsx alias removal.

    Direction is contract-first and fixed by triage: producer-side rename first, then delete the (schema as any).spacing consumer leg. Do not legalize spacing into StackSchema.

    Sweep discipline carried into the brief: itemized per-node list (落点 | before | after) in the PR body, zero changes outside the listed nodes, out-of-scope discoveries filed separately.


    Generated by Claude Code

  5. os-sales commented on Aug 22, 2026

    @os-sales
    Collaborator
    {
      "issue": 4891,
      "status": "done",
      "branch": "claude/issue-4891-catalog-layout-props-sweep",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5691",
      "premise_still_valid": true,
      "summary": "Both cards implemented in one PR as triage ruled (comment 5335896302). Every headline figure was re-measured on origin/main (3b147a367) before any edit and reproduced EXACTLY — this lane's byte-identical outcome, not the drifted one: #4891 = 140 nodes / 33 files with token hits flex 221, container 7, stack 3, grid 0 (231 total); #4890 = 135 nodes / 39 files; all 33 of #4891's files sit inside #4890's 39, so 39 files touched and 272 distinct nodes changed (3 nodes are in both halves). Order followed literally: the 135 producers were renamed spacing->gap first, then the `(schema as any).spacing` leg was deleted from stack.tsx; `spacing` was NOT legalised into StackSchema. Measurement was done by parsing all 423 catalog JSON documents rather than by grep, and edits were applied by byte offset rather than by re-serialising — 12 catalog files carry hand-compacted single-line objects that a JSON.stringify round-trip would have reflowed, which is exactly the out-of-scope churn a sweep must not produce. The ratchet at layout-props-conversion.test.tsx was extended from 'no div carries layout intent' to 'no flex/stack/container/grid hand-writes its own props', plus a spacing ban, a stack-gap value census, and sampled equivalence; a new renderer pin (packages/components/src/__tests__/stack-spacing-alias-removed.test.tsx) holds the alias removal. Changeset: @object-ui/components minor (an undeclared key stopped being honoured), per the repo's no-major rule.",
      "tests": "All from the repo root per AGENTS.md; heavy runs through scripts/pm/os-verify-lock.sh; exit codes captured before any pipe by a wrapper that propagates them, and every claim below quotes the tool's own verdict line, never a bare $?. UNION RE-RUN AT THE FINAL COMMIT 953c58820 (HEAD printed by the run itself): `pnpm exec vitest run examples/schema-catalog/ packages/components/src/__tests__/stack-spacing-alias-removed.test.tsx packages/components/src/__tests__/container-max-width-false.test.tsx packages/components/src/__tests__/div-deprecation-provenance.test.tsx packages/components/src/__tests__/div-deprecation-warn-once.test.tsx packages/plugin-dashboard/src/__tests__/DashboardGridLayout.legacyRetired.test.tsx scripts/__tests__/catalog-index-regenerable-4633.test.ts --reporter=verbose --maxWorkers=2` -> `Test Files 15 passed (15) / Tests 1636 passed (1636)`. `pnpm exec turbo run type-check --filter=@object-ui/components --filter=@object-ui/example-schema-catalog --concurrency=2` -> `Tasks: 30 successful, 30 total`, with both packages' `tsc --noEmit && tsc -p tsconfig.test.json` echoed (so not a zero-match no-op). `node scripts/check-control-bytes.mjs` -> `OK (scanned 4734 tracked text file(s))`, plus a self-scan of the 43 changed files for raw control bytes -> 0 hits. `node scripts/check-changeset-fixed.mjs` -> `All workspace packages are in the changeset fixed group.`; `node scripts/check-changeset-no-major.mjs` -> `No changeset declares a major bump.` DECLARED NARROWING (lint): whole-tree `pnpm lint` is CI's; the narrowing carries all three evidence legs — population from eslint's own config resolution, count from --format json (43 handed in, 3 inside the population, 40 outside), and invariance because eslint.config.js declares no parserOptions.project/projectService and no rule under eslint-rules/ reads another file (no readFileSync/globSync), so every rule is single-file syntactic and this diff cannot move an untouched file's verdict; exit 0, 0 errors, the 2 warnings on stack.tsx pre-existing (this diff removes one `any` and adds none). SUPERSET ARGUMENT for the test set: catalog JSON is reachable only through @object-ui/example-schema-catalog, whose in-repo dependents are apps/site (excluded from every root vitest project by config; no apps/site or content/docs path touched) plus four test files, all included; and the stack.tsx delta is one expression whose output moves ONLY for a node with `spacing` and no `gap`, of which zero remain repo-wide. DIFF-VS-LIST CROSS-CHECK: all 423 documents parsed at HEAD and in the tree and compared pointer by pointer -> changed 272, listed 272, CHANGED-BUT-NOT-LISTED 0, LISTED-BUT-NOT-CHANGED 0, nodes added/removed 0 (set equality both directions); `\"spacing\"` now occurs 0 times in the catalog. ABLATION — 3 legs, direction predicted before running, all matched. NO REBUILD NEEDED AND WHY: the root vitest config aliases @object-ui/components to packages/components/src (vitest.config.mts:264) and the catalog test imports allExamples from '../src/index.js' (line 54), so no dist is in either resolution path and a source mutation takes effect directly — stated rather than assumed. Each leg mutated, proved the mutation ON DISK by counting probes, ran, then restored under `trap ... EXIT INT TERM`, with the restore verified BY SHA256 (all three files byte-identical afterwards; `git status` clean). A) revert one #4891 conversion -> predicted RED on the ratchet AND on the pinned equivalence case; observed exactly those two, `Tests 2 failed | 26 passed`. B) put `spacing: 2` back on one stack -> predicted RED on the spacing ban AND the value census; observed exactly those two, `Tests 2 failed | 26 passed` reporting `expected 1 to be +0`. C) restore the deleted `?? (schema as any).spacing` -> predicted exactly one failure with the flex/gap/gap:0 cases unaffected; observed `Tests 1 failed | 3 passed`. VOID LEG REPORTED, NOT SILENTLY RETRIED: leg B's FIRST attempt was declared void by its own on-disk guard. The mutation had landed, but the confirmation used `grep -F` with a MULTI-LINE pattern, which grep treats as several patterns and counts matching lines for, so the 'this text is gone' probe returned 3 instead of 0. The reading was discarded and the leg re-run with single-line counting probes (`\"gap\": 2,` 3->2 and `\"spacing\": 2,` 0->1); the row above is that second, valid run. Leg C is what makes the new renderer pin non-phantom: `spacing: 6` was chosen because 6 is a mapped ladder step, so a live alias renders md:gap-6 and fails loudly instead of coinciding with the default.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #5690 (finding, unassigned): nine catalog `flex` nodes still spell horizontal spacing as `space-x-*` — same reader-facing shape as this card but a different mechanism (a margin on a sibling selector, not the flexbox gap), so it is not a mechanical rename and a conversion moves rendering; excluding it is precisely why this card's 221-token flex figure re-measured to the digit. `space-y-*` on a stack: 0 nodes, measured.",
        "NOT filed — already open as #5574, so the measurement was posted there as a comment instead of duplicating the issue: flex/stack/container spread their schema keys onto the DOM element (the leak PR #5573 closed for grid via toDomProps). That issue lists these three as 'candidates to measure'; measured over the whole catalog through the real SchemaRenderer: 662 stray attributes (flex[align] 198, flex[gap] 184, stack[gap] 153, flex[justify] 98, container[padding] 14, container[maxwidth] 6, flex[direction] 5, stack[align] 4), with grid reading ZERO as the control that the probe is not blind. 231 of those are new as of this sweep, because declared props are what leak — worth PM's attention as sizing input for #5574, not as a reason to hold this PR.",
        "recorded on the PR, not filed: GitHub's body sanitizer ate one angle-bracket-delimited placeholder from a fenced code block in the PR body (the AGENTS.md 'GitHub mutates body BYTES' behaviour). Both per-node tables and the session-URL footer survived byte-intact; the command was re-spelled in a PR comment rather than re-posting a 28k-character body."
      ]
    }

    Generated by Claude Code

  6. os-sales commented on Aug 22, 2026

    @os-sales
    Collaborator

    os-dev-report

    Re-post, and the reason is itself worth a line: the HTML-comment marker on my previous comment (5379791876) did not survive the write — GitHub's body sanitizer ate the short angle-bracket-delimited run, so that comment is invisible to a marker scan. Same report below, marker as literal text. Prefer this comment; 5379791876 is superseded and otherwise identical.

    {
      "issue": 4891,
      "status": "done",
      "branch": "claude/issue-4891-catalog-layout-props-sweep",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5691",
      "premise_still_valid": true,
      "summary": "Both cards implemented in one PR as triage ruled (comment 5335896302). Every headline figure was re-measured on origin/main (3b147a367) before any edit and reproduced EXACTLY — this lane's byte-identical outcome, not the drifted one: #4891 = 140 nodes / 33 files with token hits flex 221, container 7, stack 3, grid 0 (231 total); #4890 = 135 nodes / 39 files; all 33 of #4891's files sit inside #4890's 39, so 39 files touched and 272 distinct nodes changed (3 nodes are in both halves). Order followed literally: the 135 producers were renamed spacing->gap first, then the `(schema as any).spacing` leg was deleted from stack.tsx; `spacing` was NOT legalised into StackSchema. Measurement was done by parsing all 423 catalog JSON documents rather than by grep, and edits were applied by byte offset rather than by re-serialising — 12 catalog files carry hand-compacted single-line objects that a JSON.stringify round-trip would have reflowed, which is exactly the out-of-scope churn a sweep must not produce. The ratchet at layout-props-conversion.test.tsx was extended from 'no div carries layout intent' to 'no flex/stack/container/grid hand-writes its own props', plus a spacing ban, a stack-gap value census, and sampled equivalence; a new renderer pin (packages/components/src/__tests__/stack-spacing-alias-removed.test.tsx) holds the alias removal. Changeset: @object-ui/components minor (an undeclared key stopped being honoured), per the repo's no-major rule.",
      "tests": "All from the repo root per AGENTS.md; heavy runs through scripts/pm/os-verify-lock.sh; exit codes captured before any pipe by a wrapper that propagates them, and every claim below quotes the tool's own verdict line, never a bare $?. UNION RE-RUN AT THE FINAL COMMIT 953c58820 (HEAD printed by the run itself): `pnpm exec vitest run examples/schema-catalog/ packages/components/src/__tests__/stack-spacing-alias-removed.test.tsx packages/components/src/__tests__/container-max-width-false.test.tsx packages/components/src/__tests__/div-deprecation-provenance.test.tsx packages/components/src/__tests__/div-deprecation-warn-once.test.tsx packages/plugin-dashboard/src/__tests__/DashboardGridLayout.legacyRetired.test.tsx scripts/__tests__/catalog-index-regenerable-4633.test.ts --reporter=verbose --maxWorkers=2` -> `Test Files 15 passed (15) / Tests 1636 passed (1636)`. `pnpm exec turbo run type-check --filter=@object-ui/components --filter=@object-ui/example-schema-catalog --concurrency=2` -> `Tasks: 30 successful, 30 total`, with both packages' `tsc --noEmit && tsc -p tsconfig.test.json` echoed (so not a zero-match no-op). `node scripts/check-control-bytes.mjs` -> `OK (scanned 4734 tracked text file(s))`, plus a self-scan of the 43 changed files for raw control bytes -> 0 hits. `node scripts/check-changeset-fixed.mjs` -> `All workspace packages are in the changeset fixed group.`; `node scripts/check-changeset-no-major.mjs` -> `No changeset declares a major bump.` DECLARED NARROWING (lint): whole-tree `pnpm lint` is CI's; the narrowing carries all three evidence legs — population from eslint's own config resolution, count from --format json (43 handed in, 3 inside the population, 40 outside), and invariance because eslint.config.js declares no parserOptions.project/projectService and no rule under eslint-rules/ reads another file (no readFileSync/globSync), so every rule is single-file syntactic and this diff cannot move an untouched file's verdict; exit 0, 0 errors, the 2 warnings on stack.tsx pre-existing (this diff removes one `any` and adds none). SUPERSET ARGUMENT for the test set: catalog JSON is reachable only through @object-ui/example-schema-catalog, whose in-repo dependents are apps/site (excluded from every root vitest project by config; no apps/site or content/docs path touched) plus four test files, all included; and the stack.tsx delta is one expression whose output moves ONLY for a node with `spacing` and no `gap`, of which zero remain repo-wide. DIFF-VS-LIST CROSS-CHECK: all 423 documents parsed at HEAD and in the tree and compared pointer by pointer -> changed 272, listed 272, CHANGED-BUT-NOT-LISTED 0, LISTED-BUT-NOT-CHANGED 0, nodes added/removed 0 (set equality both directions); the string spacing now occurs 0 times in the catalog. ABLATION — 3 legs, direction predicted before running, all matched. NO REBUILD NEEDED AND WHY: the root vitest config aliases @object-ui/components to packages/components/src (vitest.config.mts:264) and the catalog test imports allExamples from '../src/index.js' (line 54), so no dist is in either resolution path and a source mutation takes effect directly — stated rather than assumed. Each leg mutated, proved the mutation ON DISK by counting probes, ran, then restored under `trap ... EXIT INT TERM`, with the restore verified BY SHA256 (all three files byte-identical afterwards; `git status` clean). A) revert one #4891 conversion -> predicted RED on the ratchet AND on the pinned equivalence case; observed exactly those two, `Tests 2 failed | 26 passed`. B) put `spacing: 2` back on one stack -> predicted RED on the spacing ban AND the value census; observed exactly those two, `Tests 2 failed | 26 passed` reporting `expected 1 to be +0`. C) restore the deleted `?? (schema as any).spacing` -> predicted exactly one failure with the flex/gap/gap:0 cases unaffected; observed `Tests 1 failed | 3 passed`. VOID LEG REPORTED, NOT SILENTLY RETRIED: leg B's FIRST attempt was declared void by its own on-disk guard. The mutation had landed, but the confirmation used `grep -F` with a MULTI-LINE pattern, which grep treats as several patterns and counts matching lines for, so the 'this text is gone' probe returned 3 instead of 0. The reading was discarded and the leg re-run with single-line counting probes (the gap literal 3->2 and the spacing literal 0->1); the row above is that second, valid run. Leg C is what makes the new renderer pin non-phantom: `spacing: 6` was chosen because 6 is a mapped ladder step, so a live alias renders md:gap-6 and fails loudly instead of coinciding with the default.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #5690 (finding, unassigned): nine catalog `flex` nodes still spell horizontal spacing as `space-x-*` — same reader-facing shape as this card but a different mechanism (a margin on a sibling selector, not the flexbox gap), so it is not a mechanical rename and a conversion moves rendering; excluding it is precisely why this card's 221-token flex figure re-measured to the digit. `space-y-*` on a stack: 0 nodes, measured.",
        "NOT filed — already open as #5574, so the measurement was posted there as a comment instead of duplicating the issue: flex/stack/container spread their schema keys onto the DOM element (the leak PR #5573 closed for grid via toDomProps). That issue lists these three as 'candidates to measure'; measured over the whole catalog through the real SchemaRenderer: 662 stray attributes (flex[align] 198, flex[gap] 184, stack[gap] 153, flex[justify] 98, container[padding] 14, container[maxwidth] 6, flex[direction] 5, stack[align] 4), with grid reading ZERO as the control that the probe is not blind. 231 of those are new as of this sweep, because declared props are what leak — worth PM's attention as sizing input for #5574, not as a reason to hold this PR.",
        "not filed, recorded here and on the PR: GitHub's body sanitizer ate short angle-bracket-delimited runs from BOTH of this task's GitHub posts — a placeholder inside a fenced code block in the PR body (re-spelled in a PR comment rather than re-posting 28k characters), and the report marker on issue comment 5379791876, which is why this comment exists with the marker as literal text. Both per-node tables and both session-URL/bare footers survived byte-intact. This is the documented AGENTS.md behaviour, not a new defect."
      ]
    }

    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:dispatched

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions