-
Notifications
You must be signed in to change notification settings - Fork 5.7k
feat: use nuqs to manange search params #3576
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: master
Are you sure you want to change the base?
Conversation
|
@TinsFox is attempting to deploy a commit to the umami-software Team on Vercel. A member of the Team first needs to authorize it. |
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.
Greptile Summary
This PR introduces URL-based state management using the nuqs library to store query parameters in the URL for improved shareability and user experience. The changes implement a comprehensive solution that allows application state to persist in URLs, making it easy for users to bookmark and share specific views.
The implementation follows a proper architectural pattern:
- Root-level integration: The
NuqsAdapteris added to the root layout (src/app/layout.tsx) wrapped within the existing Providers component, ensuring it has access to necessary context providers while being available to all child components - Dependency management: The
nuqspackage (version ^2.4.3) is added topackage.jsonas a type-safe search params state manager that provides React hooks similar touseStatebut stores state in URL query strings - Practical implementation: The Sessions page (
SessionsPage.tsx) demonstrates the new pattern by replacinguseStatewithuseQueryStatefor tab management, storing the active tab ('activity' vs 'properties') in URL parameters with a default value of 'activity' - Build configuration: The
pnpm-workspace.yamlfile is updated to include 'unrs-resolver' in theonlyBuiltDependencieslist, though this appears to be a typo for what should likely be a nuqs-related dependency
This change transforms the application from using internal component state to URL-persisted state, which is particularly valuable for an analytics dashboard like Umami where users frequently need to share specific filtered views or bookmark particular configurations. The library abstracts away the complexity of URL parameter parsing, serialization, and synchronization with React state while maintaining type safety.
Confidence score: 3/5
- This PR introduces significant architectural changes that require careful testing across different components and URL scenarios
- Score reflects concerns about the potential typo in pnpm-workspace.yaml ('unrs-resolver') and the need to verify nuqs integration doesn't break existing URL handling
- Pay close attention to pnpm-workspace.yaml for the suspicious 'unrs-resolver' dependency name and test URL state management thoroughly
4 files reviewed, 2 comments
|
|
||
| export function SessionsPage({ websiteId }) { | ||
| const [tab, setTab] = useState('activity'); | ||
| const [tab, setTab] = useQueryState('activity', { defaultValue: 'activity' }); |
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.
style: The query parameter key is set to 'activity' but this may be confusing since it represents the tab state, not just the activity tab. Consider using 'tab' as the key for clarity.
| - '@prisma/client' | ||
| - '@prisma/engines' | ||
| - prisma | ||
| - unrs-resolver |
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.
syntax: The package name 'unrs-resolver' appears to be a typo. Based on the PR description mentioning 'nuqs', this should likely be 'nuqs' or verify the correct package name.
| - unrs-resolver | |
| - nuqs |
Store query parameters in the URL for easier sharing and improved user experience.