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(router): Skip search params with undefined and null values passed to named routes #11635

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

antonmoiseev
Copy link
Contributor

@antonmoiseev antonmoiseev commented Sep 24, 2024

Automatically ignoring null and undefined values for search params helps avoiding repetitive and verbose checks when calling navigate() function.
Examples:

  1. Passing redirectTo parameter in case of multi-step (multi-page) sign in / sign up process:
    navigate(routes.webAuthn({ redirectTo }))
  2. Filter params on a search page:
    navigate(routes.clothes({ size, category, color }))

In both examples parameters are optional and may be missing. Currently we have to manually filter out undefined and null values before passing them to generated route functions.

@Josh-Walker-GM Josh-Walker-GM added the release:fix This PR is a fix label Sep 24, 2024
@Josh-Walker-GM Josh-Walker-GM added this to the next-release milestone Sep 24, 2024
@dthyresson dthyresson added the changesets-ok Override the changesets check label Sep 30, 2024
@antonmoiseev
Copy link
Contributor Author

antonmoiseev commented Oct 9, 2024

I noticed it wasn't included in the 8.4.0 release, does it need anything else?

@antonmoiseev
Copy link
Contributor Author

@Josh-Walker-GM sorry for tagging you, just want to make sure we have a chance to resolve any issues before next release

@Josh-Walker-GM
Copy link
Collaborator

Josh-Walker-GM commented Oct 16, 2024

Hey @antonmoiseev! No worries about the pinging. Thanks for making sure this doesn't slip out of our memory! I think some others on the team wanted to just double/sanity check this but we've all been bogged down with other work so this has slipped. Sorry about that, we really do appreciate the contributions and the team has mentioned a number of times that we're grateful for your involvement in-particular.

I'll see if I can get someone to look at this pr this week/next.

--

Would you be able to update the description to provide a bit more context around the change, what it is, why it was needed etc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changesets-ok Override the changesets check release:fix This PR is a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants