Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

[v6] feature request: allow searchParams to be passed to useNavigate's options to navigate to a URL with a query string #7743

Closed
caseyjhol opened this issue Jan 20, 2021 · 7 comments

Comments

@caseyjhol
Copy link

caseyjhol commented Jan 20, 2021

I'd like to be able to do something like:

const navigate = useNavigate();

// this would navigate to listing?foo=bar
navigate("listing", {
    searchParams: {
        foo: "bar"
    }
});

See https://stackoverflow.com/questions/65800658/react-router-v6-navigate-to-a-url-with-searchparams for more information.

@caseyjhol
Copy link
Author

caseyjhol commented Feb 1, 2021

That's close, but it still requires prepending ? and manually converting the object to a string via createSearchParams(params).toString().

navigate({
    pathname: "listing",
    search: `?${createSearchParams({
        foo: "bar"
    }).toString()}`
})

It looks like history wants to just keep the string: remix-run/history#478. Regarding the ? prefix, remix-run/history#813 and #7496 are related.

I think it'd be trivial for react-router to check if an object was passed through and convert it to a query string automatically, but I guess it's fairly trivial to do it manually too.

@epidemian
Copy link

@caseyjhol, the toString() call there is unnecessary since that's done automatically when concatenating strings, so a trimmed version of that code could be:

navigate({
    pathname: "listing",
    search: '?' + createSearchParams({ foo: "bar" })
})

Which i think is not that bad 🙂

I would also like to have a simple way to pass search params as an object. The immediate problem i see with adding this to navigate is that if they are passed alongside a search value, then navigate should reconcile these two and take care of collisions:

// should this navigate to /listing?foo=bar,  /listing?baz=qux, or /listing?foo=bar&baz=qux ?
navigate({
  pathname: 'listing',
  search: '?foo=bar',
  searchParams: { baz: 'qux' }
});

// should this navigate to /listing?foo=bar or to /listing?foo=42, or even /listing?foo=bar&foo=42 ?
navigate({
  pathname: 'listing',
  search: '?foo=bar',
  searchParams: { foo: 42 }
});

None of those questions is obvious to me. I think react-router made a good decision of staying away from query-string parsing in general (except in providing a hook for a URLSearchParams object, which is a browser builtin).

BTW, i think your solution of implementing a custom useNavigateParams() hook is perfectly reasonable for this use case.

@caseyjhol
Copy link
Author

Oh good catch. That definitely cleans it up a bit. I don't think we need a separate searchParams option (now that I know about search). From a DX perspective, I think being able to set search to a string or an object would be the best (and it could call createSearchParams internally). I'd even be fine with just accepting a string (and using createSearchParams anyway). There's something about having to manually prefix the ? that seems a little wonky.

@BenJenkinson
Copy link

I think being able to set search to an object of parameters is a great feature to add.

// navigate to /listing?foo=bar
navigate({
  pathname: "listing",
  search: "?foo=bar",
});

// also navigate to /listing?foo=bar
navigate({
  pathname: "listing",
  search: {
    foo: "bar",
  },
});

This would be really useful when merging the parameters for the "new" page with any existing parameters.

const currentSearch = useSearch();

const currentPage = currentSearch.page ?? 1;

navigate({
  pathname: "listing",
  search: {
    ...currentSearch,
    page: currentPage + 1
  },
});

It could also handle arrays of parameters, for example:

// navigate to /listing?fruit=apple&fruit=banana
navigate({
  pathname: "listing",
  search: {
    fruit: ["apple", "banana"],
  },
});

@epidemian
Copy link

epidemian commented Mar 1, 2021

From a DX perspective, I think being able to set search to a string or an object would be the best

Oh, i hadn't thought about that; you are completely right! It would be super convenient in some cases (e.g., dynamic values), and also wouldn't introduce any ambiguity problem like the ones i described 👍

@chaance
Copy link
Collaborator

chaance commented Sep 5, 2021

Just a note that you no longer need to prepend ? to the search param. Something like this should totally work.

navigate({
  pathname: "listing",
  search: "foo=bar",
});

Overloads add complexity which add code, and we're always doing what we can to weigh DX against keeping the library as lean as possible. I definitely see the utility here, but I believe it should probably be handled in user code. My suggestion would be creating your own useNavigate hook with the API you prefer:

import { useNavigate as useNavigate_ } from "react-router-dom";
import { parsePath } from "history";

export function useNavigate() {
  let navigate = useNavigate_();
  return React.useCallback((to, { searchParams, ...options } = {}) => {
    let search = typeof searchParams === "object"
      ? new URLSearchParams(searchParams).toString()
      : undefined;
    to = typeof to === "string" ? parsePath(to) : to;
    return navigate({ search, ...to }, options);
  }, [navigate]);
}

@remix-run remix-run locked and limited conversation to collaborators Sep 5, 2021
@chaance chaance closed this as completed Sep 5, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants