pbeshai / use-query-params

React Hook for managing state in URL query parameters with easy serialization.
https://pbeshai.github.io/use-query-params
ISC License
2.13k stars 95 forks source link

Calling setQuery empties a location hash #229

Open RomanTokar opened 1 year ago

RomanTokar commented 1 year ago

When we call setQuery our existing location hash will be emptied. https://codesandbox.io/s/use-query-params-issue-2eh8n7?file=/src/App.js I tried to change, for instance, ReactRouter5Adapter to this and it likely solved this problem.

const ReactRouter5Adapter = ({ children }) => {
  const history = useHistory();

  const adapter = {
    replace(location) {
      history.replace({ ...history.location, search: location.search }, location.state);
    },
    push(location) {
      history.push({ ...history.location, search: location.search }, location.state);
    },
    get location() {
      return history.location;
    },
  };

  return children(adapter);
};
pbeshai commented 1 year ago

Yeah that's very true. I made this change based on the way react-router works with search params (see this comment for their explanation on removing the hash). I don't necessarily agree with their reasoning, but I don't really use hashes myself very much to have a strong opinion. I'm unsure whether or not the provided adapter should do as you show above or not.

RomanTokar commented 1 year ago

@pbeshai I guess I also don't agree. Here is an codesandbox with applied above adapter.

andymerskin commented 5 months ago

@roman

Great fix, thanks for sharing! Totally agree that the hash shouldn't get nuked when search param changes happen. A lot of applications have tab components or other interactive UI elements that use URL hashes to target them, like going directly to a route with a specific tab selected when the page loads.

For others needing to use this fix in their project locally:

  1. Modify ./node_modules/use-query-params/adapters/react-router-5/index.js in your project with the above changes
  2. Use patch-package [link]() to store a diff of the changes. This tells your package manager to install your local patch whenever you install dependencies for your project, and helps document the changes you've made. Useful for bugfixes like these when your favorite package has lost maintainers or isn't moving on fixes.
  3. Import the JS file directly, rather than the built index.cjs.js file by default:

    import { ReactRouter5Adapter } from "use-query-params/adapters/react-router-5/index";

    Your project may need allowJs in your TypeScript config. Change your configuration accordingly. The import statement may vary depending on your bundling and build setup, but you just need to import the unbuilt JS file from source that you've modified — or jump through the hoops of building this package locally and using it.

vincerubinetti commented 2 months ago

Having the same issue with ReactRouter6Adapter. Surprised this isn't a more common complaint and urgent fix. I feel like this library should try to be as non-destructive to the URL as possible.

m-scimonelli commented 1 week ago

Using the OP as inspiration, I made a ReactRouter6Adapter alternative:

const ReactRouter6Adapter: QueryParamAdapterComponent = ({ children }) => {
  const navigate = useNavigate();
  const location = useLocation();

  const adapter: QueryParamAdapter = {
    replace(location) {
      navigate(location, { replace: true, state: location.state });
    },
    push(location) {
      navigate(location, { state: location.state );
    },
    get location() {
      return location;
    },
  };

  return children(adapter);
};

You can then pass this to the adapter prop of QueryParamProvider. Didn't have the chance to do a complete round of testing, but it seems to be working as intended.