Skip to content

fix(row-model): ColumnValueOf resolves to unknown, not never - #462

Merged
blove merged 1 commit into
mainfrom
blove/cell-editing-code-fix-3f5756
Aug 17, 2026
Merged

fix(row-model): ColumnValueOf resolves to unknown, not never#462
blove merged 1 commit into
mainfrom
blove/cell-editing-code-fix-3f5756

Conversation

@blove

@blove blove commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

The defect

value in rows-mode onRowChange was typed never. never is assignable to everything, so this guard — shipped in the editing page's worked example — compiled green while checking nothing:

if (columnId === "quantity" && typeof value === "number" && value < 0)

Root cause: ColumnValueOf fell back to never for any column that declares no accessor. No example in the docs corpus uses accessor columns, so this hit every rows-mode onRowChange the docs teach — not just this one page.

Found while investigating a report that the cell-editing page showed "invalid code". The file typechecked and linted clean; the defect was only visible by probing the type.

The fix

ColumnValueOf now falls back to unknown, which forces the guard instead of accepting it.

never is still load-bearing inside the distribution — it is the union identity, so non-matching members of a mixed column tuple must vanish rather than widen every answer. The fallback is applied outside the distribution via [X] extends [infer TResolved]; a naked extends infer would distribute, and distributing over never short-circuits the whole conditional to never, which is precisely the case the fallback exists to catch.

Both behaviors are pinned in column-value-of.test.ts — including the mixed-tuple precision assertions, which were already green before the fix and so will catch any widening.

Fallout, all fixed

  • apps/bench passed columnId as never to distinctValues, forcing the id to match no column. BenchColumn declares an accessor, so plain columnId resolves correctly — the cast was papering over this same hole.
  • grid/editing.mdx's value row claimed "inferred from columnId", true only for accessored columns. Replaced with an accurate note plus a narrowing snippet.
  • The example passed <PretableSurface<StockItem>, which bought nothing (TRow is inferred from rows) and read as broken JSX.
  • The docs-guard roster excused that table on the grounds that it documents PretableSurfaceProps.onCellEdit — a prop that exists nowhere in the repo. That stale excuse is why the false claim survived. Replaced with the real limitation: the member reader handles interfaces only and cannot bind a mapped-type-indexed union. Follow-up filed to teach it that shape.

Verification

Rebased onto origin/main (10 commits ahead, incl. #447 touching row-model); all gates re-run against the new base. The .api.md files merged cleanly and were regenerated to confirm the merged text matches what the generator produces.

  • pnpm typecheck clean; pnpm lint clean (1 pre-existing unrelated TanStack Virtual warning)
  • 2,073 package tests + 554 website tests pass
  • api:check clean, with no ae-forgotten-export warning (this repo had zero; the fix was restructured to avoid introducing the first)
  • Type-performance gate passes

One number worth a look: the extra conditional costs ~5% more type instantiations (deterministic: 126,366 → 132,161 on columns-500, against a 151,640 budget). Memory on columns-500 reads 98.9–101.6 MiB across runs against a 109.9 MiB budget — ~92% of ceiling, up from ~83%. Memory is noisy run-to-run; instantiations are not. I did not re-baseline the budgets, since that would silently loosen a gate.

🤖 Generated with Claude Code

`ColumnValueOf` fell back to `never` for any column that declares no
`accessor`. `never` is assignable to everything, so every runtime guard
written against such a value compiled green while checking nothing:

    if (columnId === "quantity" && typeof value === "number" && value < 0)

That is the guard shipped in the editing page's worked example, and it
could never have been wrong. No example in the docs corpus uses accessor
columns, so `value` was `never` in every rows-mode `onRowChange` the docs
teach.

`unknown` is the honest fallback: it forces the guard instead of
accepting it.

`never` is still load-bearing INSIDE the distribution — it is the union
identity, so non-matching column members must vanish rather than widen
every answer for a mixed tuple. The fallback is applied outside the
distribution via `[X] extends [infer TResolved]`; a naked `extends infer`
would distribute, and distributing over `never` short-circuits the whole
conditional to `never`, which is precisely the case the fallback exists
to catch. Both behaviors are pinned in column-value-of.test.ts.

Fallout:

- apps/bench passed `columnId as never` to `distinctValues`, which forced
  the id to match no column. `BenchColumn` declares an accessor, so plain
  `columnId` resolves correctly; the cast was papering over this hole.
- grid/editing.mdx's `value` row claimed "inferred from `columnId`",
  true only for accessored columns. Replaced with an accurate note.
- The example passed `<PretableSurface<StockItem>`, which bought nothing
  (TRow is inferred from `rows`) and read as broken JSX.
- The docs-guard roster excused that table on the grounds that it
  documents `PretableSurfaceProps.onCellEdit` — a prop that exists
  nowhere in the repo. That stale excuse is why the false claim survived.
  Replaced with the real limitation: the member reader handles interfaces
  only and cannot bind a mapped-type-indexed union.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
pretable Ignored Ignored Aug 17, 2026 3:30am

Request Review

@blove
blove enabled auto-merge (squash) August 17, 2026 03:31
@github-actions

Copy link
Copy Markdown
Contributor

Vercel preview ready

Preview: https://pretable-n089t4ye6-cacheplane.vercel.app
Commit: ae1f386d87407586247cd2966c5d0f465eca6d0a

Updated automatically by the deploy-preview job.

@blove
blove merged commit fca7a78 into main Aug 17, 2026
20 checks passed
@blove
blove deleted the blove/cell-editing-code-fix-3f5756 branch August 17, 2026 03:45
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.

1 participant