-
Notifications
You must be signed in to change notification settings - Fork 16
feat: support search parameters #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
89fa256 to
19d9517
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enables support for search parameters within the application by updating navigation logic and adding a dedicated component to track URL changes. Key changes include:
- Integrating the useSearchParams hook and managing combined path/search state in TransitionRouter.
- Introducing the SearchParamsHandler component wrapped in Suspense.
- Updating demo page and back button components to reflect the new navigation behavior.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/context.tsx | Added support for search parameters and updated navigation logic. |
| example/components/back.tsx | Updated button text for improved clarity. |
| example/app/demo/page.tsx | Modified page component to use async searchParams and enhance UI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the transition router to be aware of URL search parameters and prevents unnecessary transitions when only query strings change, and updates the demo page to showcase pagination via searchParams.
- Track full path including search parameters using a new
SearchParamsHandler. - Skip transitions if only the query string matches the current URL.
- Update example/demo page to accept
searchParams, add pagination links, and use the newBackcomponent.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/context.tsx | Added SearchParamsHandler, state for full path, and adjusted navigation logic to consider search params. |
| example/components/back.tsx | Updated back button label to include an arrow. |
| example/app/demo/page.tsx | Made DemoPage async with searchParams, added pagination links, and used the Back component. |
Comments suppressed due to low confidence (3)
src/context.tsx:10
- The
SearchParamsHandlerusesuseEffectbutuseEffectis not imported from React. AdduseEffectto the import list.
import { useMemo, useRef, useState, Suspense, } from "react";
src/context.tsx:162
- [nitpick] This new
SearchParamsHandlerlogic for tracking query strings isn’t covered by existing tests. Add unit or integration tests to verify it updatespathWithSearchcorrectly when search params change.
function SearchParamsHandler({
src/context.tsx:150
- [nitpick] The
SearchParamsHandlercomponent doesn’t suspend, so wrapping it inSuspenseisn’t necessary. You can render it directly to simplify the tree.
<Suspense fallback={null}>
| } | ||
| }; | ||
| }, [stage, pathname]); | ||
| }, [stage, pathWithSearch]); |
Copilot
AI
May 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The navigate callback lists pathWithSearch as a dependency but never uses it, and omits pathname. This may cause stale closures when the path changes. Adjust the dependency array to reflect the values used inside the callback.
| }, [stage, pathWithSearch]); | |
| }, [stage, pathname]); |
|
Hi @ismamz why this PR isn't merged yet? can I help you on something? |
No description provided.