Repository navigation
Conversation
…ange The options store was a deep `$state`, so every options sync wrapped the caller's `data` array in a new proxy. The core row model memoizes on `table.options.data`, saw new data, and ran its auto-resets, undoing the controlled `expanded` or `pagination` change that caused the sync. `set` always replaces the whole options object, so `$state.raw` gives the store all the reactivity it needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 15141a8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
NickSpag
changed the title
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (3 skipped: 3 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title names the Svelte adapter (svelte-table) and the bug it fixes: controlled state resetting after each change. This matches the main change, which switches the options store to $state.raw so op… Description check The description follows the template well. It has a Changes section that states the bug cause and the fix (switching the options store to $state.raw, with the side effect on row.original). It describe… Linked Issues check Directly linked issue #6640requires that controlled expanded and pagination changes persist when table data is a plain array, while genuine data changes still trigger resets. The reviewed head change…Out of Scope Changes check All changed files relate to the #6640objective. The source change is the one-line$stateto$state.rawswitch in the options-store atom (packages/svelte-table/src/reactivity.svelte.ts). The `.cha…Full details: Title check
-
Explanation
The title names the Svelte adapter (svelte-table) and the bug it fixes: controlled state resetting after each change. This matches the main change, which switches the options store to $state.raw so option syncs stop replacing data identity and triggering row-model auto-resets that undo controlled expanded and pagination changes. The title follows the conventional commit format and is concise and specific.
Full details: Description check
-
Explanation
The description follows the template well. It has a Changes section that states the bug cause and the fix (switching the options store to $state.raw, with the side effect on row.original). It describes the tests added and their before/after results, and the local verification commands. The checklist items are marked, and the release impact section is filled in with a changeset. It links the issue (
#6640) and explains why the existing tests missed the bug. The Changes section does not use the template's own comment prompt, but it does describe the change and its motivation, so the check passes.
Full details: Linked Issues check
-
Explanation
Directly linked issue
#6640requires that controlled expanded and pagination changes persist when table data is a plain array, while genuine data changes still trigger resets. The reviewed head changescreateRuneWritableAtomin packages/svelte-table/src/reactivity.svelte.ts from$state(initialValue)to$state.raw(initialValue). The diff confirms this one-line change.createRuneWritableAtomruns only for the options store (debugName === 'table/optionsStore'). Other table atoms still usecreateAtomfrom@tanstack/svelte-store. The options store is replaced wholesale byseton eachsetOptionscall, so a raw signal holds the new options object. The caller'sdataarray keeps its identity across unchanged syncs. The core row model memoization therefore does not recompute and does not run the auto-resets. A genuine new data array still changes identity and still triggers the resets, so that requirement remains. Automated tests were added:PaginationHarnessgets a 'Next page' button, and the newExpandingHarnessfixture controlsexpandedwithcreateTableState. The lifecycle test file asserts the displayed row IDs after these actions. I did not execute the tests. The test assertions are taken from the PR summary and description, which report fail-before and pass-after results. Those results are unverified by this review.
Full details: Out of Scope Changes check
-
Explanation
All changed files relate to the
#6640objective. The source change is the one-line$stateto$state.rawswitch in the options-store atom (packages/svelte-table/src/reactivity.svelte.ts). The.changeset/raw-svelte-options-store.mdpatch changeset describes that change for the published package. The test changes (adapter-lifecycle.test.ts,ExpandingHarness.svelte, and the 'Next page' button inPaginationHarness.svelte) support the regression coverage for controlled expanded and pagination state. The side effect thatrow.originalis now the caller's object is the behavior the issue asks to remove. No unrelated code, refactoring, or dependency change appears in the reviewed summary. Test contents are based on the PR summary, not on a full diff read.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
This branch has not been deployed
1 participant
🎯 Changes
Fixes #6640.
Bug cause reported in full in the accompanying #6640 issue. TLDR:
The Svelte adapter keeps the table's options in a deep $state. Every time the options are applied, Svelte wraps the plain data array in a new proxy, so table.options.data is a different object each time even though the data hasn't changed. The core row model treats that as new data and runs its auto-resets. Changing controlled expanded or pagination applies the options again, so each change is immediately reset back. When data is already in $state, Svelte reuses the existing proxy and the bug doesn't show.
Fix:
$state.raw.setalways replaces the whole options object, so a top-level signal is all the store needs. As a side effect,row.originalis the caller's object again rather than a proxy of it.Tests:
PaginationHarnessgains a "Next page" button, and a newExpandingHarnesscontrolsexpandedwithcreateTableState. The existing pagination test only callssetPageSize(3), which ends on page 0 whether or not the reset fires, so it didn't catch this. Both new tests fail before the change and pass after it.The examples don't show the bug because they all keep
datain$state. Svelte leaves an existing proxy as it is, sotable.options.datastays stable there. The bug needs a plain array (a constant, a$derivedvalue, or a TanStack Query result), which is why the new tests use the fixtures' plain arrays. The TanStack Query example avoids it for a different reason: it setsmanualPagination: true, which turns off the page reset.✅ Checklist
pnpm testandpnpm test:e2e, or these tests do not apply to this pull request.Locally: the
@tanstack/svelte-tabletargets frompnpm test(test:lib,test:types,test:eslint,build,test:build) all pass.test:e2epasses for all 41 Svelte examples (164 tests). The two new unit tests fail without the change and pass with it.🚀 Release Impact
Summary by CodeRabbit