Skip to content

feat(DataTable): add type-aware cell renderers on Column#247

Merged
erickteowarang merged 6 commits into
mainfrom
feat/data-table/type-aware-cells
May 13, 2026
Merged

feat(DataTable): add type-aware cell renderers on Column#247
erickteowarang merged 6 commits into
mainfrom
feat/data-table/type-aware-cells

Conversation

@erickteowarang
Copy link
Copy Markdown
Contributor

@erickteowarang erickteowarang commented May 13, 2026

Summary

  • Adds type (text / number / money / date / badge / link) and a typeOptions field to Column<TRow>. When type is set, the cell is rendered automatically from accessor(row) (or row[id] when accessor is omitted), eliminating the inline boilerplate that every list table re-implements for currency, date, and badge formatting.
  • render is always optional and continues to win when both are present, so the existing escape hatch is preserved.
  • Empty values (null / undefined / "") render a muted placeholder uniformly across types; link cells render plain text when typeOptions.href returns nullish.

Type safety

Column<TRow> is a discriminated union keyed off type (ColumnBase<TRow> & ColumnTypeBranch<TRow>). typeOptions narrows per branch to a dedicated interface (NumberCellOptions, MoneyCellOptions<TRow>, DateCellOptions, BadgeCellOptions, LinkCellOptions<TRow>), and the following become compile errors rather than runtime no-ops:

// ❌ badgeVariantMap is not a money option
column({ type: "money", accessor: (r) => r.total, typeOptions: { badgeVariantMap: {} } });

// ❌ link columns must provide typeOptions.href
column({ type: "link", accessor: (r) => r.title });

// ❌ text columns reject typeOptions entirely
column({ type: "text", accessor: (r) => r.title, typeOptions: { locale: "en-US" } });

Three @ts-expect-error assertions are checked in via the test suite to lock these guarantees in.

render lives on ColumnBase (not per-branch) so callback contextual typing still works on the common column({ ...inferred, render: (row) => ... }) spread pattern — row infers without annotation. ColumnBase and ColumnTypeBranch are both exported publicly so api-extractor rolls them into the dist .d.ts.

Accessor return type is also narrowed per branch

accessor lives on each branch (not on ColumnBase) so each typed renderer can constrain what the column produces. Returning an array or a plain object from a typed accessor is a compile error instead of silently rendering [object Object] or a stringified list:

// ❌ text accessor cannot return an array
column({ type: "text", accessor: (r) => r.tags });

// ❌ money accessor cannot return an object
column({ type: "money", accessor: (r) => r.priceDetail });

Per-branch return types:

type accessor return type
(omitted) unknown — pairs with render
text string | number | boolean | bigint | null | undefined
number number | null | undefined
money number | null | undefined
date Date | string | number | null | undefined
badge string | number | boolean | null | undefined
link string | number | boolean | null | undefined

Why this is part of a three-PR series

This is 1 / 3 in a series porting patterns from a downstream consumer's ReconciliationList into the platform DataTable. The remaining two PRs add align and truncate on Column as independent, mechanical changes — landing as separate PRs to keep each diff small and reviewable.

Notes for reviewers

  • New module: packages/core/src/components/data-table/cell-renderers.tsx. Pure functions per type; renderTypedCell dispatches once per cell in DataTable.Body and narrows col.typeOptions per-branch via switch (col.type) — no casts.
  • money formatting accepts a string or (row) => string for currency, mirroring the currencyKey pattern in the source component but in a typesafe way. Falls back to "USD" if the resolved currency code is invalid (Intl throws otherwise).
  • link cells use react-router's Link (already re-exported from app-shell) so SPA navigation is preserved.
  • Docs at docs/components/data-table.md get:
    • Updated Column reference (shared fields + per-type fields tables) reflecting the discriminated-union shape
    • New "Cell types" section listing each type's value handling and options interface
    • New "Adding a typed column" recipes section walking through each built-in type, the override-with-render escape hatch, and how to layer type on top of inferColumns

Public type additions

Type Purpose
ColumnCellType "text" | "number" | "money" | "date" | "badge" | "link"
ColumnBase<TRow> Shared column fields
ColumnTypeBranch<TRow> Per-type branches with narrowed typeOptions and accessor
NumberCellOptions Options for type: "number"
MoneyCellOptions<TRow> Options for type: "money" (currency accepts a function)
DateCellOptions Options for type: "date"
BadgeCellOptions Options for type: "badge"
LinkCellOptions<TRow> Options for type: "link" (href required)
BadgeVariant Variant union accepted by <Badge>

Test plan

  • pnpm --filter @tailor-platform/app-shell type-check — clean
  • pnpm --filter @tailor-platform/app-shell test — 992 passed (was 974; +18 new cases: per-type rendering, accessor fallback, custom-render override, and 4 type-level @ts-expect-error assertions including array/object accessor rejection per typed branch)
  • pnpm --filter @tailor-platform/app-shell lint — clean
  • pnpm --filter @tailor-platform/app-shell build — dist .d.ts includes ColumnBase and ColumnTypeBranch
  • pnpm -r type-check — workspace clean (nextjs + vite demos typecheck unchanged against narrowed accessor)
  • pnpm fmt:check — clean across 342 files
  • Reviewer smoke-test: drop column({ label, type: "money", accessor }) into the data-table demo and confirm the column renders without a render prop

🤖 Generated with Claude Code

Adds `type` (text/number/money/date/badge/link) and `typeOptions` to
`Column<TRow>`. When set, the cell is rendered automatically from
`accessor(row)` (or `row[id]`), eliminating boilerplate for the common
cases. `render` becomes optional when `type` is provided and continues
to win when both are present, preserving the per-column escape hatch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 13, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@tailor-platform/app-shell@247
npm i https://pkg.pr.new/@tailor-platform/app-shell-sdk-plugin@247
npm i https://pkg.pr.new/@tailor-platform/app-shell-vite-plugin@247

commit: d7179c0

@erickteowarang erickteowarang requested a review from IzumiSy May 13, 2026 02:29
Comment thread packages/core/src/components/data-table/types.ts Outdated
@IzumiSy
Copy link
Copy Markdown
Contributor

IzumiSy commented May 13, 2026

/review

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by API Design Review for issue #247

Comment thread packages/core/src/components/data-table/types.ts
Comment thread packages/core/src/components/data-table/cell-renderers.tsx Outdated
Comment thread packages/core/src/components/data-table/types.ts Outdated
Erick Teowarang and others added 3 commits May 13, 2026 13:12
Restructure Column<TRow> as ColumnBase<TRow> & ColumnTypeBranch<TRow>,
keyed off `type`. typeOptions now narrows to a per-type interface
(NumberCellOptions, MoneyCellOptions, etc.), so wrong-shape options
are a compile error instead of being silently ignored at runtime.

Two additional safety gains:
- `type: "link"` requires typeOptions.href (was silently degraded to
  plain text when missing).
- Untyped columns still require `render` (matches pre-feature behavior);
  passing neither type nor render is now a compile error rather than
  rendering an em-dash placeholder.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add an "Adding a typed column" section that walks through how to define
columns for each built-in type — text, number, money, date, badge, link
— with the parameters they accept. Includes the override-with-render
escape hatch and a recipe for layering type on top of inferColumns.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The discriminated-union refactor put `render` inside each type branch,
which broke contextual typing on the common spread-then-override pattern
(`column({ ...inferred, render: (row) => ... })`) — TS couldn't pick a
branch before typing the callback, leaving `row` as implicit any.

Two fixes:

1. Move `render` onto `ColumnBase` (always optional). Callback contextual
   typing now resolves uniformly across all type branches. The "render
   required when type is omitted" compile check is dropped — callers with
   neither type nor render fall through to the em-dash placeholder, which
   matches pre-feature behavior.

2. Export `ColumnBase` and `ColumnTypeBranch` publicly so api-extractor
   includes them in the rolled-up `.d.ts`. They were tagged `@internal`,
   which stripped them from the dist and left `Column<TRow>` referencing
   undeclared types.

All other type-safety guarantees from the discriminated union remain
(wrong-shape `typeOptions` errors, `link` requires `href`, `text`
rejects `typeOptions`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@IzumiSy
Copy link
Copy Markdown
Contributor

IzumiSy commented May 13, 2026

@copilot does badge type render multiple badges when the given data is array?

Copy link
Copy Markdown
Contributor

Copilot AI commented May 13, 2026

@copilot does badge type render multiple badges when the given data is array?

No — currently badge renders a single <Badge> only. If the value is an array, it gets stringified (e.g. ['a','b'] becomes "a,b") and shown as one badge.

Copilot AI requested a review from IzumiSy May 13, 2026 04:20
@IzumiSy
Copy link
Copy Markdown
Contributor

IzumiSy commented May 13, 2026

@erickteowarang Can you cover the corner case like what happens when the data is array/object? I guess there should be some limitation or adoption for data structure. Supporting array will be needed anyway (I remember that I have been reported that DescriptionCard cannot renders the multiple badges, but users wanted that)

@erickteowarang
Copy link
Copy Markdown
Contributor Author

@erickteowarang Can you cover the corner case like what happens when the data is array/object? I guess there should be some limitation or adoption for data structure. Supporting array will be needed anyway (I remember that I have been reported that DescriptionCard cannot renders the multiple badges, but users wanted that)

Do you mean if someone sends in an array of like, badges to the columns?

It won't crash, but it will result in text not appearing properly (e.g if you send in an array of string to the text type, it'll return a,b, and an object will be [object Object])

I can add some defensive coding for that use case (e.g just render array/object data as a placeholder string). Future array support should probably be done in a separate PR though.

…type

The discriminated-union refactor narrowed `typeOptions` per `type`, but
`accessor` stayed on `ColumnBase` as `(row: TRow) => unknown`. That meant
an accessor returning an array or plain object would silently flow into
the typed renderers and produce visible garbage — `text` rendered
`[object Object]`, `badge` and `link` rendered a badge / link labeled
`[object Object]`, etc.

Move `accessor` onto each branch of `ColumnTypeBranch` and narrow its
return type to values the matching renderer can display:

- `text`           → string | number | boolean | bigint | null | undefined
- `number`/`money` → number | null | undefined
- `date`           → Date | string | number | null | undefined
- `badge`/`link`   → string | number | boolean | null | undefined
- untyped          → unchanged (`unknown` — pairs with `render`)

`null` and `undefined` stay in every typed branch on purpose: each
built-in renderer maps them to the `—` placeholder so columns can be
defined over sparse / optional fields without coercion. A row with
`placedAt: null` next to a row with `placedAt: "2026-04-09…"` renders
the formatted date for one and the placeholder for the other — same
runtime behavior as before, now reflected in the types.

Verified against the workspace: examples and tests typecheck without
changes. The contextual-typing problem that pushed `render` back onto
`ColumnBase` in 589a252 doesn't bite here because the dominant pattern
is `{ ...infer(field), render: ... }` (override `render`, not `type`),
which keeps the spread inside the untyped branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@erickteowarang
Copy link
Copy Markdown
Contributor Author

erickteowarang commented May 13, 2026

@IzumiSy Updated now to make it ultra specific for each column type.

I think this makes it safer (will fail on compile time) and more extendable (so if Badge accepts an array in the future, we can just adjust the type)

Copy link
Copy Markdown
Contributor

@IzumiSy IzumiSy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@erickteowarang erickteowarang merged commit c4fbfa2 into main May 13, 2026
5 checks passed
@erickteowarang erickteowarang deleted the feat/data-table/type-aware-cells branch May 13, 2026 06:43
erickteowarang pushed a commit that referenced this pull request May 14, 2026
Brings in #247 (type-aware cell renderers), #248 (column align), #251
(oxlint upgrade), #253 (accessor narrowing docs), and #254 (inferColumns
no longer carries an accessor). Conflicts:

- types.ts: dropped the duplicate `accessor` declaration from
  `ColumnBase` (it now lives per-branch in `ColumnTypeBranch` per #247)
  and kept both new fields — `align` (from #248) and `truncate` (from
  this branch). Updated `truncate`'s JSDoc to describe the Tooltip
  wiring rather than the `title` attribute.
- field-helpers.ts: kept main's spread-based `column()` so the
  discriminated union survives.
- data-table.tsx: combined `align` and `truncate` into the same cell
  classes; rebuilt `content` via `col.render ?? renderTypedCell(...)`
  (from #247).
- data-table.test.tsx / docs: kept both feature describe blocks and
  combined doc tables.

Per @IzumiSy's review (#249), the truncate tooltip now uses the
app-shell `<Tooltip>` component (with a `Tooltip.Provider` mounted at
`DataTable.Root`) instead of the browser `title` attribute. The cell
is wrapped in `Tooltip.Trigger` only when `accessor` returns a
stringifiable primitive — objects / arrays / no accessor still apply
the truncate CSS but skip the tooltip wiring.

Tests: 1010 passing (was 992 + 8 new truncate tests). Lint, fmt,
workspace typecheck all clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants