Migrate device table to TanStack Table v9 - #1660
Conversation
|
@esphbot review |
There was a problem hiding this comment.
Pull request overview
Migrates the device table to TanStack Table v9 while preserving controlled sorting, filtering, visibility, and pagination behavior.
Changes:
- Upgrades
@tanstack/lit-tableto v9.1.2. - Introduces a shared v9 feature configuration and composed table types.
- Updates table implementation and tests for v9 APIs and renamed types.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
package.json |
Upgrades TanStack Lit Table. |
pnpm-lock.yaml |
Locks v9 and its dependencies. |
src/components/dashboard/table-features.ts |
Defines shared table features and types. |
src/components/dashboard/device-table.ts |
Migrates table state and controller APIs. |
src/components/dashboard/device-table-grid.ts |
Uses composed v9 table types. |
src/components/dashboard/table-columns.ts |
Migrates column definitions and sort functions. |
src/components/dashboard/render-content.ts |
Updates visibility event typing. |
src/components/dashboard/prefs.ts |
Updates preference state typing. |
src/pages/dashboard.ts |
Updates dashboard visibility state typing. |
test/components/dashboard/table-columns.test.ts |
Migrates cell-context test types. |
test/components/dashboard/prefs.test.ts |
Migrates visibility-state test types. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
esphbot
left a comment
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
|
Both suggestions addressed in 270c207. Added a test pinning the auto resolved alphanumeric order on the platform column; the fixture includes host2/host10 and a case mixed value so a silent basic fallback fails the assertion rather than passing with the same order. Tightened the table-features docstring to name the unregistered datetime and basic outcomes. Moved DeviceRow into a leaf device-row.ts so table-features and table-columns no longer import each other; table-columns re-exports the type, so call sites are unchanged. |
|
@esphbot review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
src/components/dashboard/table-features.ts:42
- The new test only forces
autoto choosealphanumericbecause its sampled values contain digits. Removing the separately registeredtextcomparator would therefore leave this test green, even though plain-string columns such as Area and Comment would fall back to case-sensitivebasicsorting. Please add a plain-string case whose expected order distinguishestextfrombasicso both entries in the slim registry are covered.
sortFns: { alphanumeric: sortFn_alphanumeric, text: sortFn_text },
|
@esphbot review |
Verified all three on One gap remains, non-blocking. Test pins |
PR Review — Migrate device table to TanStack Table v9Both prior suggestions are genuinely addressed. Verified against the real v9 packages — merge-ready with one small coverage nit. Specifically good in this round:
Verification on
🟢 Suggestions
1. New sort test pins `alphanumeric` only — dropping `text` from the registry stays green
|
esphbot
left a comment
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
|
Suggestion 1 was already addressed in c0f1e39, which the review notes it could not see; it pins the text entry by sorting the digit-free comment column over kitchen, Bedroom, attic and asserting attic, Bedroom, kitchen, an order that fails under both basic and textCaseSensitive. Suggestion 2 is fixed in the commit above, the two device-row type imports are now one statement. @esphbot review |
|
@esphbot review |
What does this implement/fix?
Takes the TanStack Table v9 major that dependabot proposed in #1649; v9 is a breaking rewrite, so the bump alone fails typecheck and tests. The row model factories move into a shared
tableFeaturesobject (newtable-features.ts),VisibilityStatebecomesColumnVisibilityState, column defs usesortFn, and the table types gain the features generic. Behavior is unchanged; sorting, filtering, column visibility and pagination keep the same controlled state flow.Supersedes #1649.
Related issue or feature (if applicable):
Types of changes
bugfixnew-featureenhancementbreaking-changerefactordocsmaintenancecidependenciesChecklist
pnpm run lintpasses.pnpm run testpasses.