useHashLocation must trim query string, useSearch must parse query string from location.hash

[GravityTwoG]

While using the useHashLocation hook, the current location is retrieved from location.hash, including the query string (everything after "?"). This breaks route matching.

Additionally, when useHashLocation is used, the useSearch hook works incorrectly. The location.search property is always empty because everything is stored inside location.hash. I added another useSearch hook in use-hash-location.js that retrieves query string from location.hash.

Here is a reproduction of the problem: https://codesandbox.io/p/sandbox/wouter-hash-location-t6gy4n

You can uncomment the line "{/ \ /}" to see how the new solution works.

[stackblitz[bot]]

Run & review this pull request in StackBlitz Codeflow.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (e36651d) to head (2a92830).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## v3 #465 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 1 1 Lines 1 1 ========================================= Hits 1 1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[molefrog]

Hi, thank you for raising this PR.

As I understand, /?abc#/ and /#/?abc are two different cases. The first one is the native browser search accessible via location.search and the second one is just a hash string that contains a ? character. So I was wondering if we want to support this use case, or rather follow the native way of getting the search string.

So I was thinking, maybe we can leave the hash parsing as is and instead update the navigate method to support strings with ? in it (for example, calling navigate("/abc?def") will do two things: set the current hash and also update the search string)?

[GravityTwoG]

Yep, your approach introduces fewer changes and follows the URI spec, unlike react-router's approach.

export const navigate = (to, { state = null } = {}) => {
  // calling `replaceState` allows us to set the history
  // state without creating an extra entry

  let hash = to.replace(/^#?\/?/, "");
  let search = location.search;

  const searchIdx = hash.indexOf("?");
  if (searchIdx !== -1) {
    search = hash.slice(searchIdx, hash.length);
    hash = hash.slice(0, searchIdx);
  }

  history.replaceState(
    state,
    "",
    // keep the current pathname, but replace query string and hash
    location.pathname +
      search +
      // update location hash, this will cause `hashchange` event to fire
      // normalise the value before updating, so it's always preceeded with "#/"
      (location.hash = `#/${hash}`)
  );
};

But it will change behavior that is tested in this case:

it("does not change anything besides the hash", () => {
  history.replaceState(null, "", "/foo?bar#/app");

  const { result } = renderHook(() => useHashLocation());
  const [, navigate] = result.current;

  navigate("/settings/general");
  expect(location.pathname).toBe("/foo");
  expect(location.search).toBe("?bar");
});

This test case actually passes, but on a navigate("/abc?def") call, it will lead to a change of location.search. Is this okay?

[molefrog]

I think that is completely ok. Perhaps we can rename the test case to something like "... when doesn't contain ? symbol"

[molefrog]

Released the new version. Thank you @GravityTwoG

[molefrog]

Decided to make a patch release instead, since this was a behaviour that was more or less expected.