enhancement: reimplement createInfiniteScroll#969
Conversation
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults 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:
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.changeset/pagination-solid2-migration.md:
- Line 15: The changeset note references the wrong issue number for this
pagination rework. Update the release note text in the changeset entry so the
“resolves” reference matches the intended issue, replacing the current issue
identifier with `#797`. Keep the rest of the `createInfiniteScroll` summary
unchanged.
In `@packages/pagination/README.md`:
- Around line 221-223: Update the `reset()` documentation in the pagination
README to make it clear that it does not abort the underlying network request.
Reword the description around `reset()` so it says it disposes the reactive page
state and ignores any late result via the stale-request check in the pagination
flow, rather than implying the in-flight fetch is canceled. Keep the
clarification aligned with the `reset()` behavior and the `thisRequest !==
request` logic used by the page fetcher.
In `@packages/pagination/src/index.ts`:
- Around line 456-473: The request handling in the pagination fetch flow can
still update shared state after a page scope has been disposed, and synchronous
fetcher throws are not normalized. In the startRequest logic in the pagination
module, wrap fetcher(index) so sync failures become promise rejections, and add
a disposal check before any then/catch state updates so stale requests cannot
call setEnd, setFetching, setError, or reobserve after cleanup.
🪄 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: 469df13e-baf5-47ff-9c4a-c76f746f9e01
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/pagination-solid2-migration.mdpackage.jsonpackages/pagination/README.mdpackages/pagination/src/index.tspackages/pagination/stories/pagination.stories.tsxpackages/pagination/test/index.test.tspackages/pagination/test/server.test.tspnpm-workspace.yaml
# Conflicts: # pnpm-lock.yaml
I've gone ahead and reimplemented
createInfiniteScrollmostly to account for better error handling as referenced in issue #797. I also noticed we could do a better job of supporting Solid2 capabilities. This PR is meant to explore deeper use and exploration for infinite load pagination with the new API and capabilities.Summary by CodeRabbit
New Features
Bug Fixes
Documentation