Skip to content

[finding] ListView renders its load FAILURE through DataEmptyState while DataErrorState sits unused next to it #7143

Description

@os-warren

Found while measuring the empty-state surfaces for #7132, which fixed the symptom and deliberately left the structure alone.

packages/plugin-list/src/ListView.tsx renders its load-error panel with DataEmptyState — the component named for the empty case — passing it a destructive icon, error copy and a retry action:

{loadError && data.length === 0 ? (
  <DataEmptyState
    data-testid="list-error-state"
    data-error-kind={loadErrorKind}
    icon={loadErrorKind === 'network' ? <AlertTriangle .../> : <ShieldAlert .../>}

Meanwhile DataErrorState — same file as DataEmptyState, already role="alert", already carrying a title/message/retry-button shape — has no consumer on this surface.

What #7132 did and did not do

#7132 measured that both branches carried no role at all, so a 403 saying "You don't have access" and a young object saying "Nothing here yet" were the same node shape. It gave DataEmptyState a role="status" default and made this call site declare role="alert" so the new default could not announce an outage as a routine status.

That closes the accessibility collision. It does not close the structural one: the error panel is still an "empty state" by component identity, and the next person to reason about either component has to know that one of them is doing the other's job.

Why it was not folded into #7132

DataErrorState is not a drop-in replacement — it hardcodes its own icon and has no icon / iconWrapperClassName / showIcon props, so a swap is a visual change plus a props-surface question, not a rename. Existing pins select on data-testid="list-error-state" and on the panel's classes (ListView.loadErrorKind.test.tsx, ListView.elementDataSource.test.tsx). That is a real refactor with its own blast radius and its own visual review, which is exactly what #7132's dispatch ruled out as scope creep.

Suggested shape, not a decision

Either extend DataErrorState to accept the icon props this call site needs and migrate, or record in view-states.tsx that the layout is deliberately shared and the two components differ only by declared role. Both are defensible; the current state is defensible only because nobody has written down which one it is.

Refs: #7132 · #7063 · #7064.

Activity

  1. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Sep 1, 2026
  2. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    PM triage — pm:blocked, not dispatched

    Assessed as a refill candidate for the domain:ui seat. Two independent blockers, and the second is worth surfacing rather than routing around.

    1. Hard file collision with an open, unlanded PR

    PR #7144 (objectui#7132) is open and not yet landed, and it touches both files this card would touch — packages/components/src/custom/view-states.tsx and packages/plugin-list/src/ListView.tsx. Dispatching now would put two lanes on the same two files, one of them against a base that is about to move under it.

    Mechanical, and it clears itself the moment objectui#7144 lands.

    2. ⚠️ The two suggested shapes are not equivalent in cost — one is quota-blocked

    The card offers both without ranking them, which is right. But they land on opposite sides of a gate:

    shape contract impact status
    Extend DataErrorState with icon / iconWrapperClassName / showIcon and migrate widens the public props surface of a published component → Clause ② IS engaged ⛔ needs CONTRACT_REVIEW_TIER; fable quota is EXHAUSTED, so it cannot enter the merge queue below tier
    Record in view-states.tsx that the layout is deliberately shared and the two differ only by declared role none dispatchable once objectui#7144 lands

    I am not picking the cheap one by default. Choosing the doc-only branch because the other is quota-blocked would be deciding an architecture question on budget grounds and then presenting it as a design conclusion — and this card explicitly declines to pre-make that choice ("Suggested shape, not a decision"). If the migration is the right answer, it should wait for tier rather than be quietly replaced by a comment.

    So the fork is recorded here and carried to the maintainer alongside the standing fable-quota question, rather than resolved by me.

    What unblocks this

    1. objectui#7144 lands → collision clears.
    2. A maintainer picks the shape, or confirms the doc-only route is acceptable as the intended answer rather than the affordable one.

    Until then it stays pm:blocked and unassigned. The card is correct as filed and needs no edits.

    Noting for the record that objectui#7132's dev was right not to fold this in: DataErrorState hardcodes its icon, existing pins select on data-testid="list-error-state" and on panel classes, and the swap is a visual change with its own review — exactly the scope creep that dispatch fenced out.


    Generated by Claude Code

  3. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    Blocker 1 cleared, blocker 2 remains — pm:blocked → needs-user-decision

    PR #7144 landed as d1842abcf, so the file collision is gone: view-states.tsx and ListView.tsx are no longer being edited by an open PR.

    The second blocker is untouched, and it is the one that needs a person:

    shape contract impact status
    Extend DataErrorState with icon / iconWrapperClassName / showIcon and migrate widens the public props surface of a published component → Clause ② engaged ⛔ needs CONTRACT_REVIEW_TIER; fable quota EXHAUSTED, so it cannot enter the merge queue
    Record in view-states.tsx that the layout is deliberately shared and the two differ only by declared role none dispatchable today

    I am still not taking the cheap branch by default. Choosing the doc-only route because the migration is currently unaffordable would be deciding an architecture question on budget and then presenting it as a design conclusion — and this card explicitly declines to pre-make that choice. If the migration is right, it should wait for tier rather than be quietly replaced by a comment.

    What objectui#7144 did and did not settle

    It fixed the accessibility collision, measured: on main both branches carried no role at all, so "You don't have access" and "Nothing here yet" were the same node shape. That call site now declares role="alert" while the primitive defaults to role="status" — verified post-landing at ListView.tsx role="alert" 1→2, with the DataEmptyState control still live.

    It did not fix the structural one. The error panel is still an "empty state" by component identity, and the next person reasoning about either component still has to know that one is doing the other's job.

    What a maintainer needs to decide

    Either pick the shape, or confirm the doc-only route is the intended answer rather than the affordable one — in which case it becomes dispatchable immediately and I will run it.

    Carried alongside the standing fable-quota question (objectui#6896 / PR objectui#7111 is built but unlandable for the same reason).


    Generated by Claude Code

  4. huangyiirene commented on Sep 1, 2026

    @huangyiirene
    Collaborator

    裁决:迁移 —— 扩展 DataErrorState 并换用;形状本裁决钉死,复审由总监席承接(维护者 2026-09-01,总监批 #27)

    项目总监席 · session session_01KGtaLpkW1mycWgkbSb3H6t · 维护者对本批逐字:「同意」。

    1. 迁移获批:DataErrorState 增 icon / iconWrapperClassName / showIcon 三个 props(语义镜像 DataEmptyState 既有形状,⛔ 不发明第二种拼法),ListView 的加载失败面板换用 DataErrorState;role="alert" 语义不动(fix(components,plugin-list): let DataEmptyState declare role="status", so an empty result is not shaped like a failed one #7144 已落);现有 pins(data-testid="list-error-state" 与面板 class 选择器)随迁移更新;视觉走一次常规复核;
    2. 文档注记分支排除(只把「谁在干谁的活」写下来,不解决它);
    3. 配额堵点通路(本裁决的第二半,车道照办):条款②(发布组件 props 面加宽)的档位要求由项目总监席承接 —— 形状已在第 1 条钉死,实施按车道可用档机械执行,PR 挂 needs:contract-review,由总监席(CONTRACT_REVIEW_TIER 机读认定在案)复审后落地,与 objectstack#13897 同一条路;
    4. 同路适用于被同一配额卡住的存量单:finding(types/plugin-charts): the static ChartSchema node's two data keys are declared-but-unread — ChartDataSeries.data is required and never read, and categories is read as a SERIES list, not axis labels #6896 / PR feat(types)!: retire ChartDataSeries.data, correct categories' prose (#6896) #7111(built-but-unlandable)车道可直接转总监席复审;finding(app-shell): ~141 KB of the console eager closure is previews/inspectors pinned by four SYNCHRONOUS studio-design registry reads — a different contract from the ComponentRegistry (arm C of #6776) #6795 A 部分的 Restart-when 第 2 条(复审档位通路)自本裁决起视为满足。

    状态转移(同笔)

    needs-user-decision → pm:queue;finding domain:ui 不动。


    Generated by Claude Code

  5. os-project-manager commented on Sep 3, 2026

    @os-project-manager
    Collaborator

    Claim: domain:ui execution seat, session session_01EMrWaQw3XS5DxTHxp4yRyC, branch claude/issue-7143-dataerrorstate-migration.

    Dispatched to one os-dev agent this round, against the ruling in comment 5494762960 (director seat, 2026-09-01, maintainer verbatim 「同意」, batch #27) rather than against the body — the body says "Suggested shape, not a decision", which is now superseded: the shape is pinned and the doc-note branch is excluded.

    File surface declared: packages/components/src/custom/view-states.tsx, packages/plugin-list/src/ListView.tsx, and the pins that select on data-testid="list-error-state" and the panel classes (ListView.loadErrorKind.test.tsx, ListView.elementDataSource.test.tsx), plus a changeset.

    ⭐ Clause ② is engaged and this seat is not at tier — that is handled, not ignored. Item 3 of the ruling moves the CONTRACT_REVIEW_TIER requirement onto the project director seat: the shape is already pinned by item 1, so the lane implements mechanically at its available tier, the PR carries needs:contract-review, and the director seat reviews before it lands. ⛔ This seat will not self-review this PR and will not enqueue it on its own verdict; it goes to the director seat, the same path as objectstack#13897.

    Two constraints the dispatch adds on top of the ruling, both stated rather than folded in:

    1. ⛔ No second spelling. The ruling says the three new props mirror DataEmptyState's existing shape. The dispatch requires the implementer to read that component's actual prop names and types first and mirror them exactly — inventing a parallel vocabulary is the defect this card family exists to prevent.
    2. Measure the bytes. packages/components lands in the ui-components eager chunk, which is the one with roughly 2.3 KB of headroom — far more slack than the framework chunk that has this lane's other two PRs parked, but not unlimited. The implementer reports the measured before/after rather than asserting a props addition is free. ⛔ If it does not fit, nothing is bought back by weakening a gate or a ceiling — it comes back here.

    Generated by Claude Code

  6. claude commented on Sep 3, 2026

    @claude
    Contributor

    os-dev-report

    {
      "issue": 7143,
      "status": "done",
      "branch": "claude/issue-7143-dataerrorstate-migration",
      "pr": "https://github.com/objectstack-ai/objectui/pull/7430",
      "premise_still_valid": true,
      "summary": "Implemented the migration per the ruling in comment 5494762960 (director seat, 2026-09-01, maintainer verbatim, batch #27), not the issue body. DataErrorState gains icon / showIcon / iconWrapperClassName, mirrored from DataEmptyState in the same file: same names, same types, same default semantics — including iconWrapperClassName resolving with `??` so it REPLACES the wrapper's default class rather than merging, which makes \"\" a meaningful strip value. The only intended difference is the class the `??` falls back to (bg-destructive/10 instead of bg-muted). ListView's load-failure panel now renders through DataErrorState; role=\"alert\" untouched. DECLINED, and said so in the PR body: `illustration` (no failure analogue) and `action` (the component already spells that affordance onRetry / retryLabel, and the retry Button rides `children`, which keeps its data-testid and RotateCw glyph — an `action` prop would be a second spelling). ONE ADDITION BEYOND THE THREE PROPS, flagged for the director seat rather than buried: the icon wrapper now carries data-slot=\"data-error-state-icon\", mirroring data-empty-state-icon — without it the wrapper iconWrapperClassName governs has no name, and the migration would DROP an identifier instead of renaming it. It is one line and strikes cleanly. PR is draft, carries needs:contract-review, is NOT self-reviewed, has no auto-merge and was not enqueued.",
      "tests": "GREEN, final tree, one locked run from the repo root: `pnpm exec vitest run --maxWorkers=2` over 7 files (2 new pins + data-empty-state-role-7132, ListView.emptyVsErrorRole-7132, ListView.loadErrorKind, ListView.elementDataSource, app-shell objectListApiDisabled-4408) -> `Test Files 7 passed (7)` / `Tests 57 passed (57)`. REVERSE VERIFICATION (ablation): the implementation was committed FIRST, then both source files reverted to the base commit 42c129b64 and the 2 new pin files re-run -> `Tests 8 failed, 7 passed (15)`, exit 1. The mutation was confirmed ON DISK by grep count (data-error-state-icon 0 in view-states, DataErrorState 0 in ListView), not by an editor exit code; the script carried a `trap ... EXIT INT TERM` with absolute paths, restored with `git checkout HEAD -- PATHS` (never the bare form, which re-reads the polluted index), and proved the restore by `git hash-object` matching the HEAD blobs with `git diff HEAD` empty. NO REBUILD IS INVOLVED and none was needed: vitest aliases @object-ui/components to packages/components/src and the component suite imports the file by relative path, so nothing here resolves through dist/. That ablation FOUND A REAL DEFECT in my own new test: the showIcon={false} arm originally asserted only that the named wrapper was absent, and it PASSED on the base — where the selector does not exist either — so it could not fail. It now names the glyph too, and the re-run moved 7 red to 8 red. TYPE-CHECK: `pnpm --filter @object-ui/components --filter @object-ui/plugin-list run type-check` exit 0 — but only AFTER rebuilding the dependency closure. Run before that it reported `src/ListView.tsx(3814,13): error TS2322 ... not assignable to IntrinsicAttributes & DataErrorStateProps`, which reads like a real bug and is in fact plugin-list resolving @object-ui/components through a stale baseline dist/*.d.ts. Recorded in the PR body. Both new test files are genuinely type-checked (both packages chain `tsc -p tsconfig.test.json`; --listFiles confirms each new file is in its project). OTHER GATES, exit code captured before any pipe: check-control-bytes 0 (`OK (scanned 6115 tracked text file(s); skipped 85 binary)`); check-changeset-presence 0; check-changeset-no-major 0; check-changeset-fixed 0; `pnpm exec eslint . --no-inline-config` — FULL repo scan, not a narrowed one: 4,201 files, exit 1, with 0 errors in the 6 files this PR touches (the 93 errors across 78 other files are pre-existing on main; the root scan is stricter than CI's per-package `turbo run lint`). EAGER CLOSURE, measured both sides on real console builds (`pnpm turbo run build --filter='./packages/*'` then `pnpm --filter @object-ui/console build`, then `node scripts/check-eager-closure-budget.mjs`, exit 0 both times): ui-components 396,598 -> 396,762 gzipped bytes, +164, against the 399,000 ceiling — headroom 2,402 -> 2,238 bytes (387.3 KB -> 387.5 KB of 389.6 KB). framework and vendor-objectstack byte-identical (523,823 and 948,329). Aggregate 3178.3 -> 3178.4 KB of 3191.4 KB, +149 bytes. IT FITS. No ceiling raised, no baseline re-pinned, no gate weakened. All measurements are on the pushed commit 829b93e43.",
      "mcp_calls": "0 — the whole run went through git and the repo-scoped REST channel (a probe read of issue 7143 returned 200 at second zero); no MCP GitHub call was made, including for the PR, the label and this comment",
      "open_questions": [
        {
          "question": "NON-BLOCKING, already routed: the icon wrapper's new data-slot=\"data-error-state-icon\" is an addition beyond the three props item 1 of the ruling pins. Keep it or strike it?",
          "options": [
            "A — keep it (as implemented): mirrors DataEmptyState's data-empty-state-icon, gives the wrapper iconWrapperClassName now governs a name a test and a host stylesheet can select, and makes the ListView migration a RENAME of that identifier rather than a DROP",
            "B — strike it: implement exactly the three props and nothing else, accepting that the migration removes data-empty-state-icon from this panel with no replacement and that the new props can only be pinned by DOM position"
          ],
          "recommendation": "A, and it is already implemented as A with the deviation called out in its own PR section so the director seat can reverse it in one line. Long-term soundness leads: the file's own convention is that every icon block is named by data-slot, and shipping an overridable-but-anonymous wrapper is the kind of half-declared surface this repo keeps removing. It also makes AI-authored consumers harder to get wrong — a named slot is discoverable, DOM position is not. Scope discipline is the axis that argues for B, and it is the reason this is flagged rather than folded in; the real-need axis is thin either way (zero consumers today), which is exactly why it should not outvote the convention. No decision is needed to land: A is reversible in one line and both readings are green."
        }
      ],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  7. os-project-manager commented on Sep 3, 2026

    @os-project-manager
    Collaborator

    Delivered as PR #7430 — handed to the project director seat, ⛔ NOT accepted by this seat

    domain:ui execution seat, session session_01EMrWaQw3XS5DxTHxp4yRyC. Card stays pm:dispatched; it is not blocked and not waiting on this lane.

    Implemented against the ruling in comment 5494762960, not the body: migration taken, doc-note branch excluded, the three props mirrored from DataEmptyState in the same file, role="alert" untouched.

    Clause ② is engaged, so per item 3 of that ruling the tier requirement sits with the project director seat. This seat is not at tier and has accordingly not reviewed, not enqueued, and not enabled auto-merge. PR #7430 is draft and carries needs:contract-review. The reviewer-facing notes are on the PR itself (comment 5519267201), including one factual correction to the PR body that the review should not be built on — a blanket "no test reads either slot value" that is false as written, though the conclusion survives once narrowed to this panel.

    Two things from the round worth keeping on the card regardless of how the review goes:

    • ⭐ The trap the ruling's "⛔ 不发明第二种拼法" was protecting against was real and subtle. iconWrapperClassName is resolved with ??, so it replaces the wrapper's default class — "" strips the styling. A cn(default, override) implementation would have type-checked, looked correct, and quietly kept bg-destructive/10 underneath every override, including plugin-list's mb-3, which exists precisely to remove that box. Mirroring the spelling without mirroring the resolution would have been a silent visual regression.
    • The measured visual delta is nothing but two data-slot attribute renames — both primitives already carried identical root classes, title and body elements. That is the small, specific question the ruling's visual review actually has to answer.

    Generated by Claude Code

  8. os-project-manager commented on Sep 3, 2026

    @os-project-manager
    Collaborator

    Contract review — PR #7430 at head 829b93e43 — VERDICT: PASS

    VERDICT: PASS
    Implemented-by: session_01EMrWaQw3XS5DxTHxp4yRyC
    Reviewed-by: session_01ShyhexkB2d1AeRZ85tgAAe
    

    Director seat (objectstack #12708), summon #10, 2026-09-03. Reviewer served at claude-fable-5-1 (get_session re-read this stroke). Item 3 of the ruling on this card (5494762960, director batch #27, 2026-09-01, 「同意」) places the CONTRACT_REVIEW_TIER review with the director seat; the domain:ui seat handed it over (5519267201). Read against the diff at merge base 42c129b64: view-states.tsx, ListView.tsx, the changeset, the two edited pins and the two new pin files.

    ① Derived judgments — against ruling item 1

    1. The three props mirror DataEmptyState exactly — same names, same types, same defaults, and the semantics that could have diverged silently does not: iconWrapperClassName resolves with ??, so it REPLACES the wrapper's default class ("" strips it), the same resolution the empty state uses; only the fallback class differs (the error state's own destructive square). A cn(default, override) reading would have kept bg-destructive/10 under plugin-list's mb-3; the new pin asserts the resolved class by exact value on both components. Judged correct, and ⛔ 不发明第二种拼法 held.
    2. The migration. ListView's load-failure panel is drawn by DataErrorState; role="alert" stays spelled at the call site (pinned against this call site, not the primitive's default); data-testid="list-error-state", data-error-kind, the per-kind glyph in its stripped wrapper and the retry button survive; the genuine empty branch stays DataEmptyState / role="status" as the control. Rendered delta: two data-slot attribute renames, every class unchanged — that is the whole of the visual review the ruling asked for, and the reviewer accepts it as such.
    3. illustration and action deliberately not mirrored (no failure analogue; onRetry / retryLabel / children already spell the affordance). Judged correct — mirroring them would have been the second spelling.
    4. The one addition beyond the ruling's three props: data-slot="data-error-state-icon" on the icon wrapper. Flagged by the implementer rather than folded in. Kept: it mirrors the empty state's data-empty-state-icon, names the wrapper the new prop governs so a test or host stylesheet can select it, and makes the migration a rename of that identifier rather than a drop. One line, no prop, no vocabulary.
    5. The ui seat's correction to the PR body (one test does read [data-slot="data-empty-state"], on a directly rendered empty state, not this panel) is accepted; the conclusion — no in-repo selector on either slot value reaches this panel — is the one that matters and it holds. No body edit is a landing condition.

    ② Semver

    @object-ui/components: minor (additive optional props on a published component, the repo's standing level) and @object-ui/plugin-list: minor (rendered output a host stylesheet can observe changes). Correct.

    ③ Boundary flags

    • Bytes measured on real builds: ui-components +164 B gzip (396,598 → 396,762) against a 399,000 ceiling; framework and vendor-objectstack byte-identical. No ceiling raised, no baseline re-pinned.
    • The reverse verification found and fixed a vacuous arm in the PR's own test (showIcon={false} asserting only the wrapper's absence). Right shape.
    • Not on the governed register. CI: all 30 check runs on 829b93e43 are success or skipped.

    Disposition: needs:contract-review cleared on this card and on PR #7430 in this stroke; PR flipped to ready with auto-merge (squash). pm:dispatched stays until the merge closes the card through Fixes #7143. Ruling item 4's path (objectui#6896 / PR #7111 to the director seat) is unchanged by this verdict.


    Generated by Claude Code

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

Metadata

Metadata

Labels

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

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions