feat: remove svelte-table selectors to be inline with other signal-ba… - #6464
Merged
Conversation
|
View your CI Pipeline Execution ↗ for commit d36f37e
☁️ Nx Cloud last updated this comment at |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughChangesThe Svelte adapter removes selector-based table state and Svelte state API — State consumption — Examples and documentation use Validation — Tests cover rune projections, unmount behavior, removed exports, pagination synchronization, isolated table instances, and row-selection state. Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/framework/svelte/guide/migrating.md`:
- Around line 5-6: Update the migration note’s wording to use the hyphenated
compound adjective “beta-breaking change,” preserving the surrounding
explanation and formatting.
In `@docs/framework/svelte/guide/row-selection.md`:
- Around line 55-59: Update the selectedCount example derived from rowSelection
to count only entries whose values are truthy, rather than counting all keys.
Keep the rowSelection atom read unchanged and preserve the documented
boolean-map behavior.
In `@docs/framework/svelte/guide/sorting.md`:
- Around line 79-81: Move the table.atoms.sorting.get() snapshot read from
top-level component initialization into a click handler or callback example,
while keeping the $derived sorting declaration unchanged and demonstrating that
the snapshot is read during event handling.
In `@docs/framework/svelte/reference/functions/createTable.md`:
- Around line 47-50: Update the createTable call in the Svelte example to
provide data through a getter that returns the current data value, matching the
reactive pattern in the quick-start example; leave the features and columns
arguments unchanged.
In `@examples/svelte/basic-app-table/src/App.svelte`:
- Line 146: Fix the markup in the table-state `<pre>` block by keeping the
closing element after the complete JSON.stringify expression, including its
closing braces, so Svelte parses the expression and element correctly.
In `@examples/svelte/column-visibility/src/App.svelte`:
- Line 175: Complete the `table-state` `<pre>` markup in `App.svelte` by closing
the `JSON.stringify` expression and adding the closing `</pre>` tag, ensuring
the Svelte example compiles.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 04cb976c-723b-4c2e-9160-0f0da9aeca38
📒 Files selected for processing (85)
docs/config.jsondocs/framework/svelte/guide/cell-selection.mddocs/framework/svelte/guide/column-filtering.mddocs/framework/svelte/guide/column-resizing.mddocs/framework/svelte/guide/composable-tables.mddocs/framework/svelte/guide/custom-features.mddocs/framework/svelte/guide/global-filtering.mddocs/framework/svelte/guide/migrating.mddocs/framework/svelte/guide/pagination.mddocs/framework/svelte/guide/row-selection.mddocs/framework/svelte/guide/sorting.mddocs/framework/svelte/guide/table-state.mddocs/framework/svelte/quick-start.mddocs/framework/svelte/reference/functions/createTable.mddocs/framework/svelte/reference/functions/createTableHook.mddocs/framework/svelte/reference/functions/subscribeTable.mddocs/framework/svelte/reference/index.mddocs/framework/svelte/reference/interfaces/CreateTableHookResult.mddocs/framework/svelte/reference/type-aliases/AppCellContext.mddocs/framework/svelte/reference/type-aliases/AppColumnDefBase.mddocs/framework/svelte/reference/type-aliases/AppColumnDefTemplate.mddocs/framework/svelte/reference/type-aliases/AppColumnHelper.mddocs/framework/svelte/reference/type-aliases/AppDisplayColumnDef.mddocs/framework/svelte/reference/type-aliases/AppGroupColumnDef.mddocs/framework/svelte/reference/type-aliases/AppHeaderContext.mddocs/framework/svelte/reference/type-aliases/AppSvelteTable.mddocs/framework/svelte/reference/type-aliases/ComponentType.mddocs/framework/svelte/reference/type-aliases/CreateTableHookOptions.mddocs/framework/svelte/reference/type-aliases/SubscribeSource.mddocs/framework/svelte/reference/type-aliases/SvelteTable.mdexamples/svelte/aggregation/src/App.svelteexamples/svelte/basic-app-table/src/App.svelteexamples/svelte/basic-dynamic-columns/src/App.svelteexamples/svelte/basic-external-atoms/src/App.svelteexamples/svelte/basic-external-state/src/App.svelteexamples/svelte/cell-selection/src/App.svelteexamples/svelte/column-ordering/src/App.svelteexamples/svelte/column-pinning-split/src/App.svelteexamples/svelte/column-pinning-sticky/src/App.svelteexamples/svelte/column-pinning/src/App.svelteexamples/svelte/column-resizing/src/App.svelteexamples/svelte/column-sizing/src/App.svelteexamples/svelte/column-visibility/src/App.svelteexamples/svelte/column-visibility/tests/e2e/smoke.spec.tsexamples/svelte/composable-tables/src/components/PaginationControls.svelteexamples/svelte/composable-tables/src/components/ProductsTable.svelteexamples/svelte/composable-tables/src/components/UsersTable.svelteexamples/svelte/composable-tables/tests/e2e/smoke.spec.tsexamples/svelte/expanding/src/App.svelteexamples/svelte/filtering/src/App.svelteexamples/svelte/filters-faceted-bucketed/src/App.svelteexamples/svelte/filters-faceted/src/App.svelteexamples/svelte/filters-fuzzy/src/App.svelteexamples/svelte/grouped-aggregation/src/App.svelteexamples/svelte/grouping/src/App.svelteexamples/svelte/kitchen-sink/src/App.svelteexamples/svelte/pagination/src/App.svelteexamples/svelte/pagination/tests/e2e/smoke.spec.tsexamples/svelte/row-pinning/src/App.svelteexamples/svelte/row-selection/src/App.svelteexamples/svelte/row-selection/tests/e2e/smoke.spec.tsexamples/svelte/sorting/src/App.svelteexamples/svelte/sub-components/src/App.svelteexamples/svelte/with-tanstack-form/src/App.svelteexamples/svelte/with-tanstack-form/src/PaginationControls.svelteexamples/svelte/with-tanstack-query/src/App.svelteexamples/vue/pagination/src/App.vueknip.jsonpackages/svelte-table/skills/migrate-v8-to-v9/SKILL.mdpackages/svelte-table/skills/table-state/SKILL.mdpackages/svelte-table/src/createTable.svelte.tspackages/svelte-table/src/createTableHook.svelte.tspackages/svelte-table/src/index.tspackages/svelte-table/src/reactivity.svelte.tspackages/svelte-table/src/subscribe.tspackages/svelte-table/tests/adapter-lifecycle.test.tspackages/svelte-table/tests/fixtures/HookHarness.sveltepackages/svelte-table/tests/fixtures/ReactivityHarness.sveltepackages/svelte-table/tests/fixtures/RuneStateHarness.sveltepackages/svelte-table/tests/fixtures/SelectorHarness.sveltepackages/svelte-table/tests/fixtures/SsrHarness.sveltepackages/svelte-table/tests/public-api.types.tspackages/svelte-table/tests/rendering.test.tsperf-done.mdperf-todo.md
💤 Files with no reviewable changes (10)
- docs/framework/svelte/reference/functions/subscribeTable.md
- packages/svelte-table/src/subscribe.ts
- examples/vue/pagination/src/App.vue
- docs/framework/svelte/reference/type-aliases/SubscribeSource.md
- docs/config.json
- examples/svelte/basic-dynamic-columns/src/App.svelte
- packages/svelte-table/tests/fixtures/SelectorHarness.svelte
- docs/framework/svelte/reference/index.md
- packages/svelte-table/src/index.ts
- examples/svelte/sub-components/src/App.svelte
Comment on lines
+55
to
+59
| Use the same atom read in markup or a native derived value: | ||
|
|
||
| ```ts | ||
| const rowSelection = $derived(table.atoms.rowSelection.get()) | ||
| const selectedCount = $derived(Object.keys(rowSelection).length) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Count only truthy row-selection entries.
Because this guide documents rowSelection as a boolean map that may contain false values, Object.keys(rowSelection).length is not a reliable selected-row count.
-const selectedCount = $derived(Object.keys(rowSelection).length)
+const selectedCount = $derived(
+ Object.values(rowSelection).filter(Boolean).length,
+)📝 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
| Use the same atom read in markup or a native derived value: | |
| ```ts | |
| const rowSelection = $derived(table.atoms.rowSelection.get()) | |
| const selectedCount = $derived(Object.keys(rowSelection).length) | |
| Use the same atom read in markup or a native derived value: | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/framework/svelte/guide/row-selection.md` around lines 55 - 59, Update
the selectedCount example derived from rowSelection to count only entries whose
values are truthy, rather than counting all keys. Keep the rowSelection atom
read unchanged and preserve the documented boolean-map behavior.
| </table> | ||
| <div class="spacer-md"></div> | ||
| <pre data-testid="table-state">{JSON.stringify(table.state, null, 2) | ||
| <pre data-testid="table-state">{JSON.stringify(table.store.get(), null, 2) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '142,148p' examples/svelte/basic-app-table/src/App.svelteRepository: TanStack/table
Length of output: 316
Close the <pre> expression.
The <pre data-testid="table-state"> block closes the element before the }</pre> expression, breaking Svelte parsing.
Proposed fix
- <pre data-testid="table-state">{JSON.stringify(table.store.get(), null, 2)
+ <pre data-testid="table-state">{JSON.stringify(table.store.get(), null, 2)}</pre>
- }
+</pre>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/svelte/basic-app-table/src/App.svelte` at line 146, Fix the markup
in the table-state `<pre>` block by keeping the closing element after the
complete JSON.stringify expression, including its closing braces, so Svelte
parses the expression and element correctly.
| </table> | ||
| <div class="spacer-md"></div> | ||
| <pre data-testid="table-state">{JSON.stringify(table.state, null, 2) | ||
| <pre data-testid="table-state">{JSON.stringify(table.store.get(), null, 2) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '170,180p' examples/svelte/column-visibility/src/App.svelte
rg -nF '<pre data-testid="table-state">{JSON.stringify(table.store.get(), null, 2)}</pre>' \
examples/svelte/column-visibility/src/App.svelteRepository: TanStack/table
Length of output: 330
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Svelte parser tooling:\n'
node -e "try { const p = require('svelte/compiler'); console.log('svelte/compiler available:', p.VERSION); process.exit(0); } catch(err) { console.log(err.message); process.exit(1); }" || true
node -e "try { const p = require('`@tanstack/reactive-utils`'); console.log('available'); } catch(err) { console.log(err.message); }" || true
printf '\nRelevant source lines with explicit null-byte boundaries removed from sed output:\n'
sed -n '173,178p' examples/svelte/column-visibility/src/App.svelte
printf '\nHex dump of pre expression boundary:\n'
sed -n '176p' examples/svelte/column-visibility/src/App.svelte | od -An -tx1 -cRepository: TanStack/table
Length of output: 643
Restore the closing Svelte markup.
The <pre> expression is left open and the element never closes, so this example will fail to compile.
Proposed fix
- <pre data-testid="table-state">{JSON.stringify(table.store.get(), null, 2)
+ <pre data-testid="table-state">{JSON.stringify(table.store.get(), null, 2)}</pre>📝 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
| <pre data-testid="table-state">{JSON.stringify(table.store.get(), null, 2) | |
| <pre data-testid="table-state">{JSON.stringify(table.store.get(), null, 2)}</pre> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/svelte/column-visibility/src/App.svelte` at line 175, Complete the
`table-state` `<pre>` markup in `App.svelte` by closing the `JSON.stringify`
expression and adding the closing `</pre>` tag, ensuring the Svelte example
compiles.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/svelte-table/skills/table-state/SKILL.md`:
- Around line 98-104: Clarify the pagination example around the tableFeatures
configuration: either add createPaginatedRowModel to demonstrate client-side
pagination, or explicitly configure manualPagination with the total row/page
count for state-only or server-side pagination. Keep rowPaginationFeature and
the existing createTableState setup consistent with the selected mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ba193968-9085-40b6-a90c-95661fa7b6a3
📒 Files selected for processing (6)
_artifacts/domain_map.yaml_artifacts/skill_spec.md_artifacts/skill_tree.yamlpackages/svelte-table/skills/migrate-v8-to-v9/SKILL.mdpackages/svelte-table/skills/table-state/SKILL.mdpackages/svelte-table/skills/with-tanstack-query/SKILL.md
Comment on lines
+98
to
+104
| createTableState, | ||
| rowPaginationFeature, | ||
| tableFeatures, | ||
| type PaginationState, | ||
| } from '@tanstack/svelte-table' | ||
|
|
||
| const features = tableFeatures({ rowPaginationFeature }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n "paginatedRowModel|createPaginatedRowModel|manualPagination" \
docs/framework/svelte \
packages/svelte-table/skills/table-state/SKILL.mdRepository: TanStack/table
Length of output: 3936
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Target snippet"
sed -n '70,120p' packages/svelte-table/skills/table-state/SKILL.md
echo
echo "## Nearby examples/usages in target file"
rg -n "rowPaginationFeature|paginatedRowModel|manualPagination|getPageCount|getRowModel|createTableState" packages/svelte-table/skills/table-state/SKILL.md
echo
echo "## Official Svelte pagination guide relevant ranges"
sed -n '1,130p' docs/framework/svelte/guide/pagination.md
sed -n '40,90p' docs/framework/svelte/guide/table-state.mdRepository: TanStack/table
Length of output: 10604
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Full 'Reduce boilerplate with createTableState' section"
sed -n '90,165p' packages/svelte-table/skills/table-state/SKILL.md
echo
echo "## Check for rowPaginationFeature without paginatedRowModel elsewhere in skill docs"
python3 - <<'PY'
from pathlib import Path
import re
path = Path('packages/svelte-table/skills/table-state/SKILL.md')
text = path.read_text()
blocks = re.findall(r'```ts\n(?P<body>.*?)\n```', text, re.S)
for i, body in enumerate(blocks, 1):
if 'rowPaginationFeature' in body and 'createPaginatedRowModel' not in body:
idx = text.index(body)
# Estimate start line by counting preceding newlines
line_no = text[:idx].count('\n') + 1
print(f'block {i} starting approximately line {line_no}')
print(body.strip()[:800])
print('---')
PYRepository: TanStack/table
Length of output: 3575
Scope the pagination example to client- or server-side pagination.
This snippet registers rowPaginationFeature but does not set up pagination row-model behavior. If it is meant to demonstrate client-side pagination, add paginatedRowModel: createPaginatedRowModel() from @tanstack/svelte-table. If it is state/manual-pagination-only, make that explicit by showing manualPagination: true and the total row/page count.
🧰 Tools
🪛 SkillSpector (2.4.4)
[error] 132: [MP3] Memory Manipulation: Skill manipulates agent memory, state, or stored context. Memory corruption can alter personality, override safety rules, or cause unpredictable behavior.
Remediation: Protect agent memory and state from modification by untrusted content. Use read-only memory for critical instructions and validate all state changes.
(Memory Poisoning (MP3))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/svelte-table/skills/table-state/SKILL.md` around lines 98 - 104,
Clarify the pagination example around the tableFeatures configuration: either
add createPaginatedRowModel to demonstrate client-side pagination, or explicitly
configure manualPagination with the total row/page count for state-only or
server-side pagination. Keep rowPaginationFeature and the existing
createTableState setup consistent with the selected mode.
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.
…sed adapters
🎯 Changes
✅ Checklist
pnpm test:pr.Summary by CodeRabbit
createTable/createAppTablenow accept options only.subscribeTableandSubscribeSource, and deprecated the legacytable.statereading approach in favor of atoms/stores.renderComponentandrenderSnippet.