feat(solid): add solid createTableHook and v9 examples#6198
Open
riccardoperra wants to merge 4 commits intoalphafrom
Open
feat(solid): add solid createTableHook and v9 examples#6198riccardoperra wants to merge 4 commits intoalphafrom
riccardoperra wants to merge 4 commits intoalphafrom
Conversation
|
Contributor
📝 WalkthroughWalkthroughThis update adds new Solid.js table examples showcasing the Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant createTableHook
participant TableContext
participant AppTable
participant AppCell
participant Component
App->>createTableHook: Initialize with config
createTableHook->>TableContext: Create table context
createTableHook->>App: Return createAppTable, AppColumnHelper, hooks
App->>App: Create column definitions
App->>createTableHook: createAppTable(options)
createTableHook->>AppTable: Build AppTable wrapper
App->>AppTable: Subscribe to state (optional selector)
AppTable->>TableContext: Provide table instance
App->>AppCell: Render cells with data
AppCell->>TableContext: Access pre-bound cell components
Component->>TableContext: useCellContext()
TableContext-->>Component: Return cell with FlexRender
sequenceDiagram
participant App
participant useQuery
participant Server
participant Table
participant UI
App->>useQuery: fetchData(pageIndex, pageSize)
useQuery->>Server: GET /data?page=X&size=Y
Server-->>useQuery: { rows, pageCount, rowCount }
useQuery->>Table: Update with paginated data
Table->>UI: Render rows
UI->>UI: User clicks next page
UI->>App: setPageIndex(pageIndex + 1)
App->>useQuery: Re-fetch with new pageIndex
useQuery->>Server: GET /data?page=X+1&size=Y
Server-->>useQuery: { rows, pageCount, rowCount }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 3ae6f62
☁️ Nx Cloud last updated this comment at |
riccardoperra
force-pushed
the
feat/solid-composable-tables
branch
from
b0ccb17 to
8198588
Compare
Base automatically changed from
feat/signal-feature-interoperability-adapters
to
alpha
March 9, 2026 13:13
Add new examples: - basic app table - composble tables Update config.json to render new solid table examples
riccardoperra
force-pushed
the
feat/solid-composable-tables
branch
from
8198588 to
5e15ca3
Compare
Add new examples: - basic-external-state - basic-external-stpore - basic-use-table - with-tanstack-query Update config.json to render new solid table examples
riccardoperra
force-pushed
the
feat/solid-composable-tables
branch
from
958d194 to
3ae6f62
Compare
riccardoperra
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (5)
examples/solid/composable-tables/vite.config.ts (1)
4-9: Addserver.allowedHostsfor CodeSandbox compatibility and remove deprecated option.Same issues as the other Vite configs in this PR:
- Missing
server: { allowedHosts: true }for CodeSandbox DNS rebinding protection.polyfillDynamicImportmay be deprecated in Vite 7.x.Suggested fix
export default defineConfig({ plugins: [solidPlugin()], + server: { + allowedHosts: true, + }, build: { target: 'esnext', - polyfillDynamicImport: false, }, })Based on learnings: "In any Vite config file, add server: { allowedHosts: true } to enable DNS rebinding protection in CodeSandbox environments when using newer Vite versions (5.4.12+ or 6.0.9+)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/solid/composable-tables/vite.config.ts` around lines 4 - 9, Update the Vite config created with defineConfig (where solidPlugin() is used) to add a server property with allowedHosts: true for CodeSandbox DNS rebinding compatibility, and remove the deprecated build.polyfillDynamicImport option (or replace it with the current recommended alternative if needed); locate the config object passed to defineConfig and add server: { allowedHosts: true } and delete the polyfillDynamicImport key under build (or migrate it to the modern equivalent).examples/solid/basic-external-store/vite.config.ts (1)
4-9: Addserver.allowedHostsfor CodeSandbox compatibility.For CodeSandbox environments using newer Vite versions, adding
server: { allowedHosts: true }enables DNS rebinding protection and ensures the dev server works correctly.🔧 Suggested fix
export default defineConfig({ plugins: [solidPlugin()], + server: { + allowedHosts: true, + }, build: { target: 'esnext', - polyfillDynamicImport: false, }, })Based on learnings: "In any Vite config file (e.g., vite.config.ts), add server: { allowedHosts: true } to enable DNS rebinding protection in CodeSandbox environments when using newer Vite versions (5.4.12+ or 6.0.9+)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/solid/basic-external-store/vite.config.ts` around lines 4 - 9, The Vite config export using defineConfig and the build block is missing a server.allowedHosts setting required for CodeSandbox; update the exported config (the object passed to defineConfig in vite.config.ts) to include a server property with allowedHosts: true (i.e., add server: { allowedHosts: true }) alongside the existing plugins and build entries so the dev server works correctly in CodeSandbox with newer Vite versions.docs/config.json (1)
1004-1006: Minor formatting inconsistency on line 1006.The closing bracket placement differs from other sections in the file. Other framework sections use consistent formatting with the closing bracket on a separate line.
🔧 Suggested formatting fix
{ "label": "solid", "children": [ { "to": "framework/solid/examples/bootstrap", "label": "Solid Bootstrap" }, { "to": "framework/solid/examples/composable-tables", "label": "Composable Tables" }, - { "to": "framework/solid/examples/with-tanstack-query", "label": "With TanStack Query" }] + { "to": "framework/solid/examples/with-tanstack-query", "label": "With TanStack Query" } + ] },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/config.json` around lines 1004 - 1006, The closing bracket for the array containing the Solid framework example entries is on the same line as the last element ("framework/solid/examples/with-tanstack-query") causing an inconsistency; update the array so the final element stays on its own line and move the closing ] to its own line (matching the other framework sections) so the block uses the same multiline formatting as the rest of the file.examples/solid/basic-use-table/src/App.tsx (1)
137-156: Footer section renders empty rows since no columns define footers.All footer definitions were removed from columns, but the
<tfoot>section still iterates over footer groups. This renders empty<th>elements. Consider removing the footer section if footers aren't needed, or this may be intentional to demonstrate the full table structure.🔧 Option: Remove unused footer section
</tbody> - <tfoot> - <For each={table.getFooterGroups()}> - {(footerGroup) => ( - <tr> - <For each={footerGroup.headers}> - {(header) => ( - <th> - {header.isPlaceholder - ? null - : flexRender( - header.column.columnDef.footer, - header.getContext(), - )} - </th> - )} - </For> - </tr> - )} - </For> - </tfoot> </table>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/solid/basic-use-table/src/App.tsx` around lines 137 - 156, The <tfoot> block is rendering empty <th> cells because no columns define footers; either remove the entire <tfoot> block or conditionally render it by checking table.getFooterGroups() has groups with actual footers before iterating; specifically update the code around table.getFooterGroups(), the <tfoot> element, and the header.column.columnDef.footer usage (and/or header.isPlaceholder check) so you only render the footer rows when a footer renderer exists for columns.examples/solid/composable-tables/src/components/table-components.tsx (1)
13-17: UseuseStorefor reactive pagination updates inPaginationControls.Direct reads from
table.store.state.paginationare not reactive in Solid components—they capture a snapshot value at evaluation time and won't update when the store changes. UseuseStore(table.store, state => state.pagination)to create a reactive accessor that automatically tracks updates.Example refactor
-import { For, createMemo } from 'solid-js' +import { For } from 'solid-js' +import { useStore } from '@tanstack/solid-store' import { useTableContext } from '../hooks/table' @@ export function PaginationControls() { const table = useTableContext() - const pagination = createMemo(() => table.store.state.pagination) + const pagination = useStore(table.store, (state) => state.pagination)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/solid/composable-tables/src/components/table-components.tsx` around lines 13 - 17, PaginationControls currently reads table.store.state.pagination via createMemo which captures a snapshot and won't update reactively; replace that with Solid's useStore to derive a reactive accessor: call useStore(table.store, s => s.pagination) inside PaginationControls (after obtaining table from useTableContext()) and use that accessor wherever pagination is consumed instead of the createMemo result so the component updates when table.store changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/solid/basic-external-state/package.json`:
- Line 10: The "lint" NPM script uses the eslint binary but eslint is not
declared in devDependencies; update package.json to add eslint as a
devDependency (or replace the script to invoke npx eslint) so the "lint" script
("lint": "eslint ./src") will work consistently; e.g., add "eslint" under
devDependencies and run the corresponding install to ensure the CLI is available
for that script.
In `@examples/solid/basic-external-state/src/App.tsx`:
- Around line 1-11: The named imports from '@tanstack/solid-table' are not
alphabetized: move createTable so the import list is sorted alphabetically
(i.e., place createTable before flexRender) to satisfy the sort-imports lint
rule; update the import statement that currently lists createTable after
flexRender so all symbols like createColumnHelper, createPaginatedRowModel,
createSortedRowModel, createTable, flexRender, rowPaginationFeature,
rowSortingFeature, sortFns, tableFeatures appear in proper alphabetical order.
- Around line 88-104: Replace the clickable div used for sortable headers with a
real button: where the code currently checks header.column.getCanSort() and
renders a div with onClick={header.column.getToggleSortingHandler()}, render a
<button type="button"> when header.column.getCanSort() is true and retain a
plain non-interactive element for non-sortable headers; keep using
flexRender(header.column.columnDef.header, header.getContext()) for the label
and preserve the sort indicator computed from header.column.getIsSorted(), and
attach the toggle handler to the button (using
header.column.getToggleSortingHandler()) while keeping existing CSS/classes for
cursor and selection behavior.
- Around line 134-160: The pager buttons currently use only symbol labels which
are inaccessible; update each button element that invokes table.setPageIndex,
table.previousPage, table.nextPage and the last-page setPageIndex call to
include descriptive aria-label attributes (e.g., aria-label="First page",
"Previous page", "Next page", "Last page") so assistive technologies get
meaningful names; keep existing disabled logic and onClick handlers unchanged,
just add the aria-labels to the corresponding button elements.
In `@examples/solid/basic-external-store/package.json`:
- Line 10: The package.json "lint" npm script references eslint but eslint is
not declared in devDependencies; either add eslint (and any needed
plugins/configs) to devDependencies so the "lint" script can run (e.g., install
and list "eslint" under devDependencies) or remove/replace the "lint" script
entry if you don't intend to provide linting; update the "lint" script name
"lint" and the package.json devDependencies accordingly and ensure any
referenced configs/plugins are also added.
In `@examples/solid/basic-external-store/vite.config.ts`:
- Around line 6-9: Remove the obsolete build.polyfillDynamicImport setting from
the Vite config: open the vite.config.ts and delete the polyfillDynamicImport:
false line within the build object (the symbol to edit is the build block
containing target: 'esnext' and polyfillDynamicImport). Leave the rest of the
build config unchanged so only the deprecated option is removed.
In `@examples/solid/basic-use-table/vite.config.ts`:
- Around line 4-9: Remove the invalid Vite build option polyfillDynamicImport
from the defineConfig({... build: {...}}) block (it was removed after Vite v2)
and instead add a server configuration with allowedHosts set to true for
CodeSandbox compatibility; locate the defineConfig call and the build object
(and the solidPlugin() entry) and remove polyfillDynamicImport, then add a
top-level server: { allowedHosts: true } property to the exported config.
In `@examples/solid/composable-tables/src/components/cell-components.tsx`:
- Around line 56-77: The three icon-only action buttons inside the cell (the
buttons that call alert(`View/Edit/Delete: ${row.original.firstName}
${row.original.lastName}`)) need explicit type and accessible names: add
type="button" to each button to prevent accidental form submission and add an
aria-label (e.g., aria-label={`View ${row.original.firstName}
${row.original.lastName}`}, aria-label={`Edit ...`}, aria-label={`Delete ...`})
instead of relying on title alone; update the buttons that trigger the
View/Edit/Delete alerts accordingly so each has both type="button" and a
descriptive aria-label referencing row.original.firstName and
row.original.lastName.
In `@examples/solid/composable-tables/src/components/header-components.tsx`:
- Around line 39-43: The onChange handler for the text input uses e.target.value
which is not type-safe in Solid; change the handler to use e.currentTarget.value
and pass that to header.column.setFilterValue to ensure correct typing and avoid
event-delegation issues (affecting the input with value={columnFilterValue()},
onChange handler, and header.column.setFilterValue; placeholder referencing
header.column.id stays the same).
- Around line 63-74: The footer currently captures rows once via
table.getFilteredRowModel().rows causing stale aggregation; inside the FooterSum
component move the call to table.getFilteredRowModel().rows into the sum()
function so the reactive read happens within the tracked scope (use
header.getContext().table and header.column.id to compute values), and change
the display conditional to check for row presence (e.g.
table.getFilteredRowModel().rows.length > 0) so a legitimate zero total renders
as "0" instead of "—".
In `@examples/solid/composable-tables/src/components/table-components.tsx`:
- Around line 58-67: Replace uses of e.target.value with e.currentTarget.value
in the two Solid onChange handlers so TypeScript knows the event is from the
input/select element; specifically update the page input handler that computes
page = Number(e.currentTarget.value) - 1 before calling
table.setPageIndex(page), and update the page size select handler to call
table.setPageSize(Number(e.currentTarget.value)); this ensures type-safe access
to .value for the handlers referencing table.setPageIndex, table.setPageSize and
pagination().
In `@examples/solid/with-tanstack-query/vite.config.ts`:
- Around line 4-8: The Vite config lacks a server.allowedHosts setting which
causes DNS rebinding blocks in CodeSandbox/newer Vite; update the exported
default config (the defineConfig call where plugins: [solidPlugin()] and
build.target are set) to include a server property with allowedHosts: true so
newer Vite versions accept external hosts (older Vite will ignore it); add
server: { allowedHosts: true } alongside the existing build block in the same
object returned by defineConfig.
In `@packages/solid-table/src/createTableHook.tsx`:
- Around line 908-913: The current assignment returns the raw FlexRender
(CellFlexRender) which bypasses the AppCell/AppHeader/AppFooter wrappers and so
any hook-attached members (e.g., TextCell, SortIndicator) are missing when
callers directly render table.FlexRender with fresh cell/header/footer objects;
update the logic that sets table.FlexRender (and analogous header/footer
FlexRender returns) to wrap the incoming props by merging the appropriate
cellComponents into props.cell/props.header/props.footer before delegating to
CellFlexRender (or Header/Footer FlexRender), i.e., create a small wrapper
function that does Object.assign(props.cell, cellComponents) (or for
header/footer) and then calls CellFlexRender so templates like cell =>
<cell.TextCell /> will always see the extended members.
In `@packages/solid-table/src/FlexRender.tsx`:
- Around line 77-85: FlexRender currently always calls flexRender for header and
footer, causing placeholder headers/footers to render duplicated content; update
the Match arms in FlexRender so they skip rendering when the header() or
footer() is a placeholder (e.g., check header().isPlaceholder and
footer().isPlaceholder) and only call flexRender when not a placeholder. Apply
the same guard logic for the wrappers that call header.FlexRender() and
footer.FlexRender() in createTableHook.tsx (the
header.FlexRender/footer.FlexRender paths) so placeholder rows are not forwarded
or rendered.
In `@packages/solid-table/src/index.tsx`:
- Line 5: The barrel export removed the legacy name createTableHelper and broke
consumers; restore compatibility by re-exporting the implementation under the
old name (e.g., add an export that maps createTableHook to createTableHelper) or
add a small compatibility shim that imports createTableHook and exports it as
createTableHelper so both createTableHook and createTableHelper are available
from the barrel.
---
Nitpick comments:
In `@docs/config.json`:
- Around line 1004-1006: The closing bracket for the array containing the Solid
framework example entries is on the same line as the last element
("framework/solid/examples/with-tanstack-query") causing an inconsistency;
update the array so the final element stays on its own line and move the closing
] to its own line (matching the other framework sections) so the block uses the
same multiline formatting as the rest of the file.
In `@examples/solid/basic-external-store/vite.config.ts`:
- Around line 4-9: The Vite config export using defineConfig and the build block
is missing a server.allowedHosts setting required for CodeSandbox; update the
exported config (the object passed to defineConfig in vite.config.ts) to include
a server property with allowedHosts: true (i.e., add server: { allowedHosts:
true }) alongside the existing plugins and build entries so the dev server works
correctly in CodeSandbox with newer Vite versions.
In `@examples/solid/basic-use-table/src/App.tsx`:
- Around line 137-156: The <tfoot> block is rendering empty <th> cells because
no columns define footers; either remove the entire <tfoot> block or
conditionally render it by checking table.getFooterGroups() has groups with
actual footers before iterating; specifically update the code around
table.getFooterGroups(), the <tfoot> element, and the
header.column.columnDef.footer usage (and/or header.isPlaceholder check) so you
only render the footer rows when a footer renderer exists for columns.
In `@examples/solid/composable-tables/src/components/table-components.tsx`:
- Around line 13-17: PaginationControls currently reads
table.store.state.pagination via createMemo which captures a snapshot and won't
update reactively; replace that with Solid's useStore to derive a reactive
accessor: call useStore(table.store, s => s.pagination) inside
PaginationControls (after obtaining table from useTableContext()) and use that
accessor wherever pagination is consumed instead of the createMemo result so the
component updates when table.store changes.
In `@examples/solid/composable-tables/vite.config.ts`:
- Around line 4-9: Update the Vite config created with defineConfig (where
solidPlugin() is used) to add a server property with allowedHosts: true for
CodeSandbox DNS rebinding compatibility, and remove the deprecated
build.polyfillDynamicImport option (or replace it with the current recommended
alternative if needed); locate the config object passed to defineConfig and add
server: { allowedHosts: true } and delete the polyfillDynamicImport key under
build (or migrate it to the modern equivalent).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 191cb063-d573-4e47-8cbb-e2d4db163031
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (67)
docs/config.jsonexamples/solid/basic-app-table/.gitignoreexamples/solid/basic-app-table/README.mdexamples/solid/basic-app-table/index.htmlexamples/solid/basic-app-table/package.jsonexamples/solid/basic-app-table/src/App.tsxexamples/solid/basic-app-table/src/index.cssexamples/solid/basic-app-table/src/index.tsxexamples/solid/basic-app-table/tsconfig.jsonexamples/solid/basic-app-table/vite.config.tsexamples/solid/basic-external-state/.gitignoreexamples/solid/basic-external-state/README.mdexamples/solid/basic-external-state/index.htmlexamples/solid/basic-external-state/package.jsonexamples/solid/basic-external-state/src/App.tsxexamples/solid/basic-external-state/src/index.cssexamples/solid/basic-external-state/src/index.tsxexamples/solid/basic-external-state/src/makeData.tsexamples/solid/basic-external-state/tsconfig.jsonexamples/solid/basic-external-state/vite.config.tsexamples/solid/basic-external-store/.gitignoreexamples/solid/basic-external-store/README.mdexamples/solid/basic-external-store/index.htmlexamples/solid/basic-external-store/package.jsonexamples/solid/basic-external-store/src/App.tsxexamples/solid/basic-external-store/src/index.cssexamples/solid/basic-external-store/src/index.tsxexamples/solid/basic-external-store/src/makeData.tsexamples/solid/basic-external-store/tsconfig.jsonexamples/solid/basic-external-store/vite.config.tsexamples/solid/basic-use-table/.gitignoreexamples/solid/basic-use-table/README.mdexamples/solid/basic-use-table/index.htmlexamples/solid/basic-use-table/package.jsonexamples/solid/basic-use-table/src/App.tsxexamples/solid/basic-use-table/src/index.cssexamples/solid/basic-use-table/src/index.tsxexamples/solid/basic-use-table/tsconfig.jsonexamples/solid/basic-use-table/vite.config.tsexamples/solid/composable-tables/.gitignoreexamples/solid/composable-tables/README.mdexamples/solid/composable-tables/index.htmlexamples/solid/composable-tables/package.jsonexamples/solid/composable-tables/src/App.tsxexamples/solid/composable-tables/src/components/cell-components.tsxexamples/solid/composable-tables/src/components/header-components.tsxexamples/solid/composable-tables/src/components/table-components.tsxexamples/solid/composable-tables/src/hooks/table.tsexamples/solid/composable-tables/src/index.cssexamples/solid/composable-tables/src/index.tsxexamples/solid/composable-tables/src/makeData.tsexamples/solid/composable-tables/tsconfig.jsonexamples/solid/composable-tables/vite.config.tsexamples/solid/with-tanstack-query/.gitignoreexamples/solid/with-tanstack-query/README.mdexamples/solid/with-tanstack-query/index.htmlexamples/solid/with-tanstack-query/package.jsonexamples/solid/with-tanstack-query/src/App.tsxexamples/solid/with-tanstack-query/src/fetchData.tsexamples/solid/with-tanstack-query/src/index.cssexamples/solid/with-tanstack-query/src/index.tsxexamples/solid/with-tanstack-query/tsconfig.jsonexamples/solid/with-tanstack-query/vite.config.tspackages/solid-table/src/FlexRender.tsxpackages/solid-table/src/createTableHelper.tspackages/solid-table/src/createTableHook.tsxpackages/solid-table/src/index.tsx
💤 Files with no reviewable changes (1)
- packages/solid-table/src/createTableHelper.ts
| "dev": "vite", | ||
| "build": "vite build", | ||
| "serve": "vite preview", | ||
| "lint": "eslint ./src" |
Contributor
There was a problem hiding this comment.
ESLint script references missing dependency.
Same issue as in basic-external-store/package.json — the lint script uses eslint but it's not in devDependencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/solid/basic-external-state/package.json` at line 10, The "lint" NPM
script uses the eslint binary but eslint is not declared in devDependencies;
update package.json to add eslint as a devDependency (or replace the script to
invoke npx eslint) so the "lint" script ("lint": "eslint ./src") will work
consistently; e.g., add "eslint" under devDependencies and run the corresponding
install to ensure the CLI is available for that script.
Comment on lines
+1
to
+11
| import { | ||
| createColumnHelper, | ||
| createPaginatedRowModel, | ||
| createSortedRowModel, | ||
| flexRender, | ||
| rowPaginationFeature, | ||
| rowSortingFeature, | ||
| sortFns, | ||
| tableFeatures, | ||
| createTable, | ||
| } from '@tanstack/solid-table' |
Contributor
There was a problem hiding this comment.
Fix the sort-imports lint error.
Line 10 leaves createTable out of alphabetical order in this named import list, so the ESLint check will stay red until it is moved above flexRender.
🧰 Tools
🪛 ESLint
[error] 10-10: Member 'createTable' of the import declaration should be sorted alphabetically.
(sort-imports)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/solid/basic-external-state/src/App.tsx` around lines 1 - 11, The
named imports from '@tanstack/solid-table' are not alphabetized: move
createTable so the import list is sorted alphabetically (i.e., place createTable
before flexRender) to satisfy the sort-imports lint rule; update the import
statement that currently lists createTable after flexRender so all symbols like
createColumnHelper, createPaginatedRowModel, createSortedRowModel, createTable,
flexRender, rowPaginationFeature, rowSortingFeature, sortFns, tableFeatures
appear in proper alphabetical order.
Comment on lines
+88
to
+104
| <div | ||
| class={ | ||
| header.column.getCanSort() | ||
| ? 'cursor-pointer select-none' | ||
| : '' | ||
| } | ||
| onClick={header.column.getToggleSortingHandler()} | ||
| > | ||
| {flexRender( | ||
| header.column.columnDef.header, | ||
| header.getContext(), | ||
| )} | ||
| {{ | ||
| asc: ' 🔼', | ||
| desc: ' 🔽', | ||
| }[header.column.getIsSorted() as string] ?? null} | ||
| </div> |
Contributor
There was a problem hiding this comment.
Use a real button for sortable headers.
Lines 88-104 attach sorting to a plain <div>, so keyboard users cannot focus or toggle it. Please render a <button type="button"> for sortable headers and keep non-sortable headers as plain text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/solid/basic-external-state/src/App.tsx` around lines 88 - 104,
Replace the clickable div used for sortable headers with a real button: where
the code currently checks header.column.getCanSort() and renders a div with
onClick={header.column.getToggleSortingHandler()}, render a <button
type="button"> when header.column.getCanSort() is true and retain a plain
non-interactive element for non-sortable headers; keep using
flexRender(header.column.columnDef.header, header.getContext()) for the label
and preserve the sort indicator computed from header.column.getIsSorted(), and
attach the toggle handler to the button (using
header.column.getToggleSortingHandler()) while keeping existing CSS/classes for
cursor and selection behavior.
Comment on lines
+134
to
+160
| <button | ||
| class="border rounded p-1" | ||
| onClick={() => table.setPageIndex(0)} | ||
| disabled={!table.getCanPreviousPage()} | ||
| > | ||
| {'<<'} | ||
| </button> | ||
| <button | ||
| class="border rounded p-1" | ||
| onClick={() => table.previousPage()} | ||
| disabled={!table.getCanPreviousPage()} | ||
| > | ||
| {'<'} | ||
| </button> | ||
| <button | ||
| class="border rounded p-1" | ||
| onClick={() => table.nextPage()} | ||
| disabled={!table.getCanNextPage()} | ||
| > | ||
| {'>'} | ||
| </button> | ||
| <button | ||
| class="border rounded p-1" | ||
| onClick={() => table.setPageIndex(table.getPageCount() - 1)} | ||
| disabled={!table.getCanNextPage()} | ||
| > | ||
| {'>>'} |
Contributor
There was a problem hiding this comment.
Add accessible names to the pager controls.
Lines 134-160 use symbol-only labels (<<, <, >, >>), which gives assistive tech vague button names. Add aria-labels for first/previous/next/last page.
♿ Suggested update
<button
+ aria-label="First page"
class="border rounded p-1"
onClick={() => table.setPageIndex(0)}
disabled={!table.getCanPreviousPage()}
>
@@
<button
+ aria-label="Previous page"
class="border rounded p-1"
onClick={() => table.previousPage()}
disabled={!table.getCanPreviousPage()}
>
@@
<button
+ aria-label="Next page"
class="border rounded p-1"
onClick={() => table.nextPage()}
disabled={!table.getCanNextPage()}
>
@@
<button
+ aria-label="Last page"
class="border rounded p-1"
onClick={() => table.setPageIndex(table.getPageCount() - 1)}
disabled={!table.getCanNextPage()}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| class="border rounded p-1" | |
| onClick={() => table.setPageIndex(0)} | |
| disabled={!table.getCanPreviousPage()} | |
| > | |
| {'<<'} | |
| </button> | |
| <button | |
| class="border rounded p-1" | |
| onClick={() => table.previousPage()} | |
| disabled={!table.getCanPreviousPage()} | |
| > | |
| {'<'} | |
| </button> | |
| <button | |
| class="border rounded p-1" | |
| onClick={() => table.nextPage()} | |
| disabled={!table.getCanNextPage()} | |
| > | |
| {'>'} | |
| </button> | |
| <button | |
| class="border rounded p-1" | |
| onClick={() => table.setPageIndex(table.getPageCount() - 1)} | |
| disabled={!table.getCanNextPage()} | |
| > | |
| {'>>'} | |
| <button | |
| aria-label="First page" | |
| class="border rounded p-1" | |
| onClick={() => table.setPageIndex(0)} | |
| disabled={!table.getCanPreviousPage()} | |
| > | |
| {'<<'} | |
| </button> | |
| <button | |
| aria-label="Previous page" | |
| class="border rounded p-1" | |
| onClick={() => table.previousPage()} | |
| disabled={!table.getCanPreviousPage()} | |
| > | |
| {'<'} | |
| </button> | |
| <button | |
| aria-label="Next page" | |
| class="border rounded p-1" | |
| onClick={() => table.nextPage()} | |
| disabled={!table.getCanNextPage()} | |
| > | |
| {'>'} | |
| </button> | |
| <button | |
| aria-label="Last page" | |
| class="border rounded p-1" | |
| onClick={() => table.setPageIndex(table.getPageCount() - 1)} | |
| disabled={!table.getCanNextPage()} | |
| > | |
| {'>>'} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/solid/basic-external-state/src/App.tsx` around lines 134 - 160, The
pager buttons currently use only symbol labels which are inaccessible; update
each button element that invokes table.setPageIndex, table.previousPage,
table.nextPage and the last-page setPageIndex call to include descriptive
aria-label attributes (e.g., aria-label="First page", "Previous page", "Next
page", "Last page") so assistive technologies get meaningful names; keep
existing disabled logic and onClick handlers unchanged, just add the aria-labels
to the corresponding button elements.
| "dev": "vite", | ||
| "build": "vite build", | ||
| "serve": "vite preview", | ||
| "lint": "eslint ./src" |
Contributor
There was a problem hiding this comment.
ESLint script references missing dependency.
The lint script uses eslint but it's not declared in devDependencies. Running npm run lint will fail unless ESLint is installed globally or added as a dependency.
🔧 Option 1: Add ESLint dependency
"devDependencies": {
"@faker-js/faker": "^10.2.0",
+ "eslint": "^9.0.0",
"typescript": "5.9.3",🔧 Option 2: Remove unused lint script
"build": "vite build",
"serve": "vite preview",
- "lint": "eslint ./src"
+ "lint": "tsc --noEmit"
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/solid/basic-external-store/package.json` at line 10, The
package.json "lint" npm script references eslint but eslint is not declared in
devDependencies; either add eslint (and any needed plugins/configs) to
devDependencies so the "lint" script can run (e.g., install and list "eslint"
under devDependencies) or remove/replace the "lint" script entry if you don't
intend to provide linting; update the "lint" script name "lint" and the
package.json devDependencies accordingly and ensure any referenced
configs/plugins are also added.
Comment on lines
+58
to
+67
| onChange={(e) => { | ||
| const page = e.target.value ? Number(e.target.value) - 1 : 0 | ||
| table.setPageIndex(page) | ||
| }} | ||
| /> | ||
| </span> | ||
| <select | ||
| value={pagination().pageSize} | ||
| onChange={(e) => { | ||
| table.setPageSize(Number(e.target.value)) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In SolidJS TypeScript event handlers, should andvalues be read fromevent.currentTarget.valuerather thanevent.target.value?
💡 Result:
Yes—in SolidJS with TypeScript, prefer reading form control values from event.currentTarget.value (and checked, etc.) rather than event.target.value. Solid’s TS guidance and examples use currentTarget, because:
currentTarget is the element the handler is attached to (so TS can safely treat it as the <input>/<select> type and expose .value).[1]
target is the element that originated the event (could be a descendant), and in TypeScript it’s often just EventTarget, which doesn’t guarantee a .value property.[2]
So for <input> / <select> handlers in Solid, write e.g. e.currentTarget.value.[1]
Use currentTarget.value in these Solid handlers for type safety.
In SolidJS with TypeScript, event.target is untyped (EventTarget) and doesn't guarantee a .value property. event.currentTarget is properly typed as the element the handler is attached to, giving you safe access to .value and other element-specific properties.
Type-safe handler update
<input
type="number"
min="1"
max={table.getPageCount()}
value={pagination().pageIndex + 1}
onChange={(e) => {
- const page = e.target.value ? Number(e.target.value) - 1 : 0
+ const page = e.currentTarget.value
+ ? Number(e.currentTarget.value) - 1
+ : 0
table.setPageIndex(page)
}}
/>
@@
<select
value={pagination().pageSize}
onChange={(e) => {
- table.setPageSize(Number(e.target.value))
+ table.setPageSize(Number(e.currentTarget.value))
}}
>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/solid/composable-tables/src/components/table-components.tsx` around
lines 58 - 67, Replace uses of e.target.value with e.currentTarget.value in the
two Solid onChange handlers so TypeScript knows the event is from the
input/select element; specifically update the page input handler that computes
page = Number(e.currentTarget.value) - 1 before calling
table.setPageIndex(page), and update the page size select handler to call
table.setPageSize(Number(e.currentTarget.value)); this ensures type-safe access
to .value for the handlers referencing table.setPageIndex, table.setPageSize and
pagination().
Comment on lines
+4
to
+8
| export default defineConfig({ | ||
| plugins: [solidPlugin()], | ||
| build: { | ||
| target: 'esnext', | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether other Solid example Vite configs already carry the allowedHosts safeguard.
rg -n -C2 'allowedHosts|server\s*:' examples/solid --glob 'vite.config.ts'Repository: TanStack/table
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Find all vite.config.ts files in examples/solid directory
fd 'vite.config.ts' examples/solid/
# Also check the structure of examples/solid
echo "=== Solid examples structure ==="
ls -la examples/solid/ 2>/dev/null | head -20Repository: TanStack/table
Length of output: 1640
🏁 Script executed:
#!/bin/bash
# Read the actual file being reviewed
cat -n examples/solid/with-tanstack-query/vite.config.tsRepository: TanStack/table
Length of output: 300
🏁 Script executed:
#!/bin/bash
# Check the filters example (referenced in learning)
cat -n examples/solid/filters/vite.config.ts
echo "=== Checking a few other Solid examples ==="
# Check basic-app-table
echo "--- basic-app-table ---"
cat examples/solid/basic-app-table/vite.config.ts
echo "--- sorting ---"
cat examples/solid/sorting/vite.config.tsRepository: TanStack/table
Length of output: 853
Add server.allowedHosts for CodeSandbox/newer Vite compatibility.
This config needs the server: { allowedHosts: true } block to prevent DNS rebinding security blocks when running in CodeSandbox-like environments on newer Vite versions (5.4.12+ or 6.0.9+). This safeguard is forward-compatible and will be silently ignored by older Vite versions.
Suggested fix
export default defineConfig({
plugins: [solidPlugin()],
+ server: {
+ allowedHosts: true,
+ },
build: {
target: 'esnext',
},
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export default defineConfig({ | |
| plugins: [solidPlugin()], | |
| build: { | |
| target: 'esnext', | |
| }, | |
| export default defineConfig({ | |
| plugins: [solidPlugin()], | |
| server: { | |
| allowedHosts: true, | |
| }, | |
| build: { | |
| target: 'esnext', | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/solid/with-tanstack-query/vite.config.ts` around lines 4 - 8, The
Vite config lacks a server.allowedHosts setting which causes DNS rebinding
blocks in CodeSandbox/newer Vite; update the exported default config (the
defineConfig call where plugins: [solidPlugin()] and build.target are set) to
include a server property with allowedHosts: true so newer Vite versions accept
external hosts (older Vite will ignore it); add server: { allowedHosts: true }
alongside the existing build block in the same object returned by defineConfig.
Comment on lines
+908
to
+913
| const extendedCell = Object.assign(props.cell, { | ||
| FlexRender: CellFlexRender, | ||
| ...cellComponents, | ||
| }) as Cell<TFeatures, TData, TValue> & | ||
| TCellComponents & { FlexRender: () => JSXElement } | ||
|
|
Contributor
There was a problem hiding this comment.
table.FlexRender still bypasses the hook-bound cell/header extensions.
The extra TextCell / SortIndicator-style members are only attached inside AppCell, AppHeader, and AppFooter. Returning the raw FlexRender here means a direct <table.FlexRender cell={cell} /> or header={header} on fresh row-model objects will not have those members in getContext(), so templates like cell: ({ cell }) => <cell.TextCell /> can fail unless callers go through the wrappers first.
Also applies to: 974-979, 1040-1044, 1067-1073
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/solid-table/src/createTableHook.tsx` around lines 908 - 913, The
current assignment returns the raw FlexRender (CellFlexRender) which bypasses
the AppCell/AppHeader/AppFooter wrappers and so any hook-attached members (e.g.,
TextCell, SortIndicator) are missing when callers directly render
table.FlexRender with fresh cell/header/footer objects; update the logic that
sets table.FlexRender (and analogous header/footer FlexRender returns) to wrap
the incoming props by merging the appropriate cellComponents into
props.cell/props.header/props.footer before delegating to CellFlexRender (or
Header/Footer FlexRender), i.e., create a small wrapper function that does
Object.assign(props.cell, cellComponents) (or for header/footer) and then calls
CellFlexRender so templates like cell => <cell.TextCell /> will always see the
extended members.
Comment on lines
+77
to
+85
| <Match when={'header' in props && props.header}> | ||
| {(header) => | ||
| flexRender(header().column.columnDef.header, header().getContext()) | ||
| } | ||
| </Match> | ||
| <Match when={'footer' in props && props.footer}> | ||
| {(footer) => | ||
| flexRender(footer().column.columnDef.footer, footer().getContext()) | ||
| } |
Contributor
There was a problem hiding this comment.
Skip placeholder headers and footers inside FlexRender.
Lines 77-85 always invoke flexRender, but placeholder headers are supposed to render nothing. With grouped columns this will duplicate header/footer content, and the wrappers in packages/solid-table/src/createTableHook.tsx, Lines 791-812 propagate the same bug through header.FlexRender() and footer.FlexRender().
🩹 Suggested fix
<Match when={'header' in props && props.header}>
{(header) =>
- flexRender(header().column.columnDef.header, header().getContext())
+ header().isPlaceholder
+ ? null
+ : flexRender(header().column.columnDef.header, header().getContext())
}
</Match>
<Match when={'footer' in props && props.footer}>
{(footer) =>
- flexRender(footer().column.columnDef.footer, footer().getContext())
+ footer().isPlaceholder
+ ? null
+ : flexRender(footer().column.columnDef.footer, footer().getContext())
}
</Match>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Match when={'header' in props && props.header}> | |
| {(header) => | |
| flexRender(header().column.columnDef.header, header().getContext()) | |
| } | |
| </Match> | |
| <Match when={'footer' in props && props.footer}> | |
| {(footer) => | |
| flexRender(footer().column.columnDef.footer, footer().getContext()) | |
| } | |
| <Match when={'header' in props && props.header}> | |
| {(header) => | |
| header().isPlaceholder | |
| ? null | |
| : flexRender(header().column.columnDef.header, header().getContext()) | |
| } | |
| </Match> | |
| <Match when={'footer' in props && props.footer}> | |
| {(footer) => | |
| footer().isPlaceholder | |
| ? null | |
| : flexRender(footer().column.columnDef.footer, footer().getContext()) | |
| } | |
| </Match> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/solid-table/src/FlexRender.tsx` around lines 77 - 85, FlexRender
currently always calls flexRender for header and footer, causing placeholder
headers/footers to render duplicated content; update the Match arms in
FlexRender so they skip rendering when the header() or footer() is a placeholder
(e.g., check header().isPlaceholder and footer().isPlaceholder) and only call
flexRender when not a placeholder. Apply the same guard logic for the wrappers
that call header.FlexRender() and footer.FlexRender() in createTableHook.tsx
(the header.FlexRender/footer.FlexRender paths) so placeholder rows are not
forwarded or rendered.
| export * from './createTable' | ||
| export * from './FlexRender' | ||
| export * from './createTableHelper' | ||
| export * from './createTableHook' |
Contributor
There was a problem hiding this comment.
Keep the legacy root export or add a compatibility shim.
Line 5 makes createTableHook available from the barrel, but the previous createTableHelper root export is now gone. That turns this into a breaking change for existing import { createTableHelper } from '@tanstack/solid-table' consumers.
Compatibility-friendly barrel change
export * from './createTable'
export * from './FlexRender'
+export * from './createTableHelper'
export * from './createTableHook'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/solid-table/src/index.tsx` at line 5, The barrel export removed the
legacy name createTableHelper and broke consumers; restore compatibility by
re-exporting the implementation under the old name (e.g., add an export that
maps createTableHook to createTableHelper) or add a small compatibility shim
that imports createTableHook and exports it as createTableHelper so both
createTableHook and createTableHelper are available from the barrel.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces the new
createTableHookAPI for solid. Implementation will be very similiar to react with some naming changes (createAppTable instead of useAppTable etc.)Added the following examples:
Summary by CodeRabbit
New Features
createTableHookAPI for building composable tables with pre-bound components and context-aware helpers.FlexRenderwith discriminated union props for improved type safety and clarity.Documentation