use-query-params icon indicating copy to clipboard operation
use-query-params copied to clipboard

Calling setQuery empties a location hash

Open RomanTokar opened this issue 3 years ago • 5 comments

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);
};

RomanTokar avatar Aug 17 '22 11:08 RomanTokar

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.

pbeshai avatar Aug 17 '22 17:08 pbeshai

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

RomanTokar avatar Aug 17 '22 20:08 RomanTokar

@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.

andymerskin avatar Jan 25 '24 17:01 andymerskin

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.

vincerubinetti avatar Apr 16 '24 18:04 vincerubinetti

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.

m-scimonelli avatar Jul 03 '24 17:07 m-scimonelli