Skip to content
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

fix: Dynamic keys for useQueryStates #858

Merged
merged 3 commits into from
Jan 28, 2025
Merged

fix: Dynamic keys for useQueryStates #858

merged 3 commits into from
Jan 28, 2025

Conversation

franky47
Copy link
Member

@franky47 franky47 commented Jan 13, 2025

Tasks

  • Add a unit test (probably doesn't need e2e)

Closes #799.

Copy link

vercel bot commented Jan 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nuqs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 28, 2025 8:52am

Copy link

pkg-pr-new bot commented Jan 13, 2025

Open in Stackblitz

npm i https://pkg.pr.new/nuqs@858

commit: 32eebb8

@franky47 franky47 marked this pull request as ready for review January 14, 2025 04:29
@franky47 franky47 requested a review from Copilot January 14, 2025 04:29

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

packages/nuqs/src/useQueryStates.test.ts:296

  • The expectation for result.current[0].d should be consistent with other checks. It should be expect(result.current[0].d).toBeUndefined().
expect(result.current[0].d).toBeNull()

packages/nuqs/src/useQueryStates.ts:116

  • Comparing the joined strings of keys and values is unconventional and error-prone. Consider comparing the lengths of the arrays instead.
Object.keys(queryRef.current).join('&') !== Object.values(resolvedUrlKeys).join('&')
@yarinsa
Copy link

yarinsa commented Jan 28, 2025

Anything I can do to help get this merged :) ?

@franky47 franky47 merged commit 23c57cb into next Jan 28, 2025
4 of 5 checks passed
@franky47 franky47 deleted the fix/799-dynamic-keys branch January 28, 2025 08:50
@yarinsa
Copy link

yarinsa commented Jan 28, 2025

Awesomeness how do we get it to npm?

Copy link

🎉 This PR is included in version 2.3.2-beta.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@franky47
Copy link
Member Author

It's available in beta, I have a few other things to test/merge and it will land in latest later this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic keys in useQueryStates
2 participants