-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat(useUrlSearchParams): Add a stringify option for users to provide stringify logic #4773
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
…qual sign for empty URL parameters.
Co-authored-by: Robin <robin.kehl@singular-it.de>
@@ -78,7 +86,7 @@ export function useUrlSearchParams<T extends Record<string, any> = UrlParams>( | |||
} | |||
|
|||
function constructQuery(params: URLSearchParams) { | |||
const stringified = params.toString() | |||
const stringified = stripEqualSign ? params.toString().replace(/=(&|$)/g, '$1') : params.toString() |
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.
I think a simple replace with a regex can be dangerous.
I'd suggest exposing a hook for users to provide stringify logic themselves if needed
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.
It is a good suggestion that I add an option called stringify function.
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.
looks good overall.
Maybe we can add one example to the docs 👍🏽
@@ -78,8 +87,7 @@ export function useUrlSearchParams<T extends Record<string, any> = UrlParams>( | |||
} | |||
|
|||
function constructQuery(params: URLSearchParams) { | |||
const stringified = params.toString() | |||
|
|||
const stringified = stringify ? stringify(params) : params.toString() |
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.
tiny change but if we
const stringified = stringify ? stringify(params) : params.toString() | |
const stringified = stringify(params) |
and
const {
initialValue = {},
removeNullishValues = true,
removeFalsyValues = false,
write: enableWrite = true,
writeMode = 'replace',
window = defaultWindow!,
stringify = (params) => params.toString(),
} = options
The default is much more obvious.
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.
Yes, it looks better this way, I will modify them and add doc.
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.
@OrbisK I've made the changes based on your suggestions and added doc. I'm happy to make further adjustments if needed. Thank you!
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.
Thank you 💚
…
Before submitting the PR, please make sure you do the following
fixes #123
).Description
resolves #4772
feat(useUrlSearchParams): Optimize URL empty parameter display format
Add stripEqualSign option to useUrlSearchParams. When this option is set to true, empty parameters in the URL will be displayed as
?foo&bar
instead of?foo=&bar=
test cases
Additional context