undefined | Better HN

0 pointsjohnfn7y ago0 comments

> The solution to the external links issue is a one-liner.

It is most definitely not a one-liner.

> Making the page scroll to the top when navigating to a new page is trivial.

So do it for me. There’s no reason a routing library should break default behavior.

> That's exactly what it's supposed to do. It's a client side routing solution. (I'm also pretty sure that it doesn't remount)

Then it’s “supposed” to have inconsistent behavior. Navigating anywhere else in my site will call constructors to all my components. Navigating to the same page won’t.

> From the issues that you've had with the library, it seems like client side routing is not actually what you're looking for.

My site was a music site. I wanted clientside routing so I could keep music playing while you navigated between links. Seemed like a slam dunk for clientside routing to me.

> may I ask what the alternative would be?

Quite honestly I would go onto GitHub and look for any routing solution that didn’t have multiple closed unfixed issues with hundreds of thumbs up.

0 comments

7 comments · 2 top-level

lopatin7y ago· 3 in thread

> It is most definitely not a one-liner.

  const LinkWrapper = ({to, ...rest}) =>
    to.startsWith(window.location.origin) ? <Link to={to} {...rest} /> : <a href={to} />;

> So do it for me. There’s no reason a routing library should break default behavior.

It's not default behavior for a client side router. Making it automatically scroll to the top would save you a few lines of code while creating a monumental headache for those who need it to retain the scroll position.

johnfnOP7y ago

This has a number of bugs. Links don’t have to start with window.location.origin, they can just start with /. You didn’t pass in ...rest into the a tag, but oh did you know that the props of a are not the same type as the props to Link? They’re not - so even if you did that it wouldn’t have worked. Oh, and don’t forget to handle “javascript:” prefixed links - those are valid too. Oh and good luck getting this to type check with Typescript. You might be able to pull off something fancy with subtraction types, but who knows if it’ll continue to work when react router upgrades next year.

You might think I’m being overly pedantic. Perhaps I am - however in the course of my site I ran into each and every one of these issues after starting out with a clever one liner much like the one you provided. And every time I fixed one I wondered why react router couldn’t handle this for me.

> Making it automatically scroll to the top would save you a few lines of code while creating a monumental headache for those who need it to retain the scroll position.

Maybe I’m just not being charitable enough, but it beats me how a boolean flag is a “monumental headache” but all the above work with absolute links is an obvious thing anyone can do.

tracker17y ago

I understand the frustration.. that said, if you use a `javascript:` link in a modern web application, you deserve what you get (garbage).

bryanrasmussen7y ago

I guess you might need a polyfill for window.location.origin depending on target browsers, and since they discussed relative urls earlier I guess they want to handle relative urls as well and probably want to also handle starts with //.

I think generally what happens, based on what I often end up doing and what I've seen in bigger projects that depend on React Router, is you make a linker component to handle issues, and in fairness you end up not having a 1 liner then.

charliepark7y ago· 2 in thread

> So do it for me.

I think this will work:

Put this wherever you put your reusable functions:

`const scrollToTop = () => document.getElementById('root').scrollIntoView();`

And put this in the components / functions you want the scrollToTop effect to work on:

`useEffect(() => { scrollToTop() }, []);`

And put this in your CSS:

`html { scroll-behavior: smooth }`

(Edit to add: Not saying React Router is something you should / shouldn’t use. Just wanted to share that code in case it helps unblock anyone.)

jamesknelson7y ago

This is a nice and simple solution! Just keep in mind that it'll interact badly with the browser's scroll-to-hash functionality.

A full solution is a little more tricky, and probably involves code on multiple components:

- You'll need something to scroll to the top or the current hash on load - You'll also need code on <Link> components to check if they link to the current URL/hash, and if they're pointing to the current hash, you'll need to scroll to it on click.

Here's some relevant scroll management code from Navi's components:

- https://github.com/frontarm/navi/blob/master/packages/react-...

westurner7y ago

Accidentally downvoted on mobile (and upvoted two others). Thanks for this.

"Scroll Restoration" https://reacttraining.com/react-router/web/guides/scroll-res...

j / k navigate · click thread line to collapse