fix: move onReset to run after goToFirst and autoInit (#404) - #408
fix: move onReset to run after goToFirst and autoInit (#404)#408rickcedwhat-ai wants to merge 2 commits into
Conversation
onReset previously ran before pagination reset, so DOM settle logic inside the hook fired before the table navigated back to page 1. Move it to the end of reset() so it runs after goToFirst, cache clear, and autoInit — matching user expectations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🤖 Bot HQ🔗 Issue Link🔗 Closes #404 🔍 AI Review
This comment is managed by the bot — do not edit directly. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
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 `@src/useTable.ts`:
- Line 459: Update reset() in src/useTable.ts: distinguish the
already-first-page case from failed or unsuccessful goToFirst navigation, and
invoke onReset only after page 1 is verified. In src/types.ts at line 918,
document or enforce the resulting page-1 guarantee consistently; regenerate the
corresponding documentation in src/typeContext.ts at line 918 and
docs/.vitepress/tableresult-signatures.json at line 80 to match.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b92dbe42-ebb6-4950-911a-ba73f92a8e0a
📒 Files selected for processing (6)
docs/.vitepress/tableconfig-signatures.jsondocs/.vitepress/tableresult-signatures.jsonsrc/typeContext.tssrc/types.tssrc/useTable.tstests/edge-cases.spec.ts
| tableMapper.clear(); | ||
| log("Table reset complete. Calling autoInit to restore state."); | ||
| await _autoInit(); | ||
| await config.onReset(createStrategyContext()); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enforce or qualify the page-1 reset guarantee.
reset() can invoke onReset after an attempted reset while the DOM remains on another page. goToFirst is optional, and its result is not used. The implementation and public documentation must agree.
src/useTable.ts#L459-L459: distinguish an already-first-page result from navigation failure, and invokeonResetonly after page 1 is verified.src/types.ts#L918-L918: enforce page-1 restoration or document the optional and unsuccessful-navigation fallback.src/typeContext.ts#L918-L918: regenerate this documentation from the corrected source.docs/.vitepress/tableresult-signatures.json#L80-L80: update the generated signature documentation to match the enforced or qualified behavior.
📍 Affects 4 files
src/useTable.ts#L459-L459(this comment)src/types.ts#L918-L918src/typeContext.ts#L918-L918docs/.vitepress/tableresult-signatures.json#L80-L80
🤖 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 `@src/useTable.ts` at line 459, Update reset() in src/useTable.ts: distinguish
the already-first-page case from failed or unsuccessful goToFirst navigation,
and invoke onReset only after page 1 is verified. In src/types.ts at line 918,
document or enforce the resulting page-1 guarantee consistently; regenerate the
corresponding documentation in src/typeContext.ts at line 918 and
docs/.vitepress/tableresult-signatures.json at line 80 to match.
Summary
onResetto run aftergoToFirst, cache clear, andautoInitinstead of beforeonResetfired before the table navigated back to page 1, making the hook ineffective for post-reset waitsonResetandreset()to clarify the new timingonResetsees page 1 contentCloses #404
Test plan
pnpm exec tsc --noEmitpassesreset()E2E test still passesonReset runs after goToFirst and autoInitE2E test passes — confirms hook sees page 1🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation
Tests