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

[BUG] useHashLocation not syncing with state #417

Closed
martonlederer opened this issue Feb 21, 2024 · 5 comments · Fixed by #444
Closed

[BUG] useHashLocation not syncing with state #417

martonlederer opened this issue Feb 21, 2024 · 5 comments · Fixed by #444
Labels
unclear Probably needs more context, or can't be reproduced

Comments

@martonlederer
Copy link

With the useHashLocation hook, the active route does not sync with the current state/location, once it is updated. The initial location loads the correct route.

For example/reproduction:

  1. Set up a project with wouter and hash routing.
  2. Create a route, let's call it test. Add a link from it to the index (#/) or a navigate() call (using useLocation)
  3. Go to a #/test
  4. Click the link you added. The location will be updated, but the active route will not change.
@martonlederer martonlederer changed the title useHashLocation not syncing with state [BUG] useHashLocation not syncing with state Feb 21, 2024
@molefrog
Copy link
Owner

Hi! I can't reproduce this, would you mind sharing your code?

@martonlederer
Copy link
Author

Hey! Yeah so it is really simple, just a home page and an App component:

export default function App() {
	return (
		<>
			<Nav />
			<main>
				<Router hook={useHashLocation}>
                    <Switch>
                       <Route path="/config" component={Config} />
                       <Route path="/" component={Home} />
                    </Switch>
                  </Router>
			</main>
		</>
	);
}

@molefrog
Copy link
Owner

molefrog commented Feb 26, 2024

Does your link look like <Link href="/" /> or <Link href="#/" />?

@molefrog molefrog added the unclear Probably needs more context, or can't be reproduced label Mar 4, 2024
@neoeno
Copy link
Contributor

neoeno commented May 7, 2024

Hello — I think I stumbled on this issue today and can share a reproduction.

Here's a replit link that has runnable code: https://replit.com/@neoeno/Reproduction-of-wouter-active-links-issue-with-hash-location

An example of how it looks in the browser:

Screen.Recording.2024-05-07.at.17.23.24.mov

And here's the relevant code:

// App.jsx

import "./App.css";
import { Route, Router, Link } from "wouter";
import { useHashLocation } from "wouter/use-hash-location";

export default function App() {
  return (
    <>
      <h1>Default useBrowserLocation activates the links</h1>
      <Router>
        <ul>
          <li>
            <Link
              href="/route-one"
              className={(active) => (active ? "active" : "normal")}
            >
              Link to Route One
            </Link>
          </li>
          <li>
            <Link
              href="/route-two"
              className={(active) => (active ? "active" : "normal")}
            >
              Link to Route Two
            </Link>
          </li>
        </ul>
        <Route path="/route-one">
          <p>Route One</p>
        </Route>
        <Route path="/route-two">
          <p>Route Two</p>
        </Route>
      </Router>

      <h1>useHashLocation does not activate the links</h1>
      <Router hook={useHashLocation}>
        <ul>
          <li>
            <Link
              href="/route-one"
              className={(active) => (active ? "active" : "normal")}
            >
              Link to Route One
            </Link>
          </li>
          <li>
            <Link
              href="/route-two"
              className={(active) => (active ? "active" : "normal")}
            >
              Link to Route Two
            </Link>
          </li>
        </ul>
        <Route path="/route-one">
          <p>Route One</p>
        </Route>
        <Route path="/route-two">
          <p>Route Two</p>
        </Route>
      </Router>
    </>
  );
}
/* App.css */

.active {
  border: 1px solid red;
}

I believe the issue occurs here: https://github.com/molefrog/wouter/blob/v3/packages/wouter/src/index.js#L239

  return asChild && isValidElement(children)
    ? cloneElement(children, { onClick, href })
    : h("a", {
        ...restProps,
        onClick,
        href,
        // `className` can be a function to apply the class if this link is active
        className: cls?.call ? cls(path === href) : cls, // <------
        children,
        ref,
      });

From debugging, href ends up having the #, while path does not. Could be related to #421?

Thanks for all your hard work on wouter! Please do say if you'd appreciate a PR to fix.

@molefrog
Copy link
Owner

molefrog commented May 9, 2024

hi @neoeno, this is indeed a bug. the href gets transformed using the hrefs() function a few lines above. so I guess we need to keep the original href and store the transformed one somewhere else. feel free to file a PR 💁‍♂️

neoeno added a commit to neoeno/wouter that referenced this issue May 9, 2024
Previously when we formatted the `href` attribute (for example,
to prefix a `#` when using useHashLocation) this would make it
not match the current path, which would not have the hash prefix.

This patch preserves the target (href, or to) of the Link component
and then compares this value to the active path. This should then
match.

I have also altered some variable names to improve clarity given
we now have four different various 'paths' in this component.

Fixes molefrog#417
neoeno added a commit to neoeno/wouter that referenced this issue May 10, 2024
Previously when we formatted the `href` attribute (for example,
to prefix a `#` when using useHashLocation) this would make it
not match the current path, which would not have the hash prefix.

This patch preserves the target (href, or to) of the Link component
and then compares this value to the active path. This should then
match.

I have also altered some variable names to improve clarity given
we now have four different various 'paths' in this component.

Fixes molefrog#417
molefrog pushed a commit that referenced this issue May 10, 2024
Previously when we formatted the `href` attribute (for example,
to prefix a `#` when using useHashLocation) this would make it
not match the current path, which would not have the hash prefix.

This patch preserves the target (href, or to) of the Link component
and then compares this value to the active path. This should then
match.

I have also altered some variable names to improve clarity given
we now have four different various 'paths' in this component.

Fixes #417
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unclear Probably needs more context, or can't be reproduced
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants