reach / router

https://reach.tech/router
MIT License
6.9k stars 326 forks source link

Redirect has flashing behavior - possible to "synchronously" redirect? #292

Open samselikoff opened 5 years ago

samselikoff commented 5 years ago

This is a question ā€“ I searched around the issues & Spectrum but couldn't find an answer.

I'm trying to make a redirect that's "synchronous", in that it doesn't cause a double render. The reason is because I want my top-level nav to <Link to='/docs' />, but the first real page of the docs is actually /docs/getting-started/introduction. I tried adding a <Redirect /> and also using navigateTo, but in both cases I get a flash as /docs renders, then redirect, then /docs/getting-started-introduction renders.

Also, once I'm on /docs/getting-started/introduction, clicking the link to /docs flashes again.

You can see the behavior if you click Documentation here: https://5d52b7a44d225b0009a37a20--miragejs.netlify.com/

My question is, is there a way to sort of statically/synchronously redirect, without any flashing? I want that URL to always go to /docs/getting-started/introduction.

(The reason I don't want to use <Link to='/docs/getting-started/introduction' /> is because I want the Documentation to be partiallyActive when any route under /docs is active.)

Thanks for this great libary šŸ™‚

samselikoff commented 5 years ago

Ended up writing this component in my app

function NavLink({ activeFor, ...props }) {
  activeFor = activeFor || props.to;

  return (
    <Match path={activeFor}>
      {({ match }) => {
        return (
          <Link
            {...props}
            className={`${match ? "text-gray-900" : "text-grey-500"}`}
          />
        );
      }}
    </Match>
  );
}

and using it like this

<NavLink
  to="/docs/getting-started/introduction"
  activeFor="/docs/*"
>
  Documentation
</NavLink>

but, still curious if this idea has a place in this library!

ryanflorence commented 5 years ago

redirectTo and <Redirect/> both throw a redirect which is caught with componentDidCatch and should prevent a double render, navigate however doesn't throw so it will (probably?) cause double renders.

Can you make a test case on codesandbox?

ryanflorence commented 5 years ago

Found your sourcemaps:

in app.js:

<Redirect from="/docs" to="/docs/getting-started/introduction" noThrow />

You opted into the flashing with noThrow.

In development without noThrow its annoying (because react doesn't actually catch the errors, then tools like create-react-app throw up a big error screen).

But in production you won't get that big error screen, but instead get a single render (and no flashing).


// change this
<Redirect from="/docs" to="/docs/getting-started/introduction" noThrow />
// to this
<Redirect from="/docs" to="/docs/getting-started/introduction" />
ryanflorence commented 5 years ago

Oops, this needs better docs so people know hte point of noThrow and why it throws in the first place, and to not worry about the error page that a lot of tools throw up in development.

aryzing commented 5 years ago

Removing the noThrow prop does not fix the issue in prod mode. Getting flashing and errors in console too:

image

Using Redirect like so:

    <Router>
      <Redirect from="/" to="/customers" />
      <Redirect
        from="/customers/:customer"
        to="/customers/:customer/product-access"
      />
    ...

If I'm not mistaken, the flashing indicates that the components are re-mounted and loose their state, which causes issues with the UI I'm building.

ryanflorence commented 5 years ago

You'll still get the error, I don't believe you'll get flashing, but I haven't tested it lately. Can you make a test case in code sandbox?

Bakkhos commented 5 years ago

I tried to reproduce this in this fiddle. The console shows 2 errors whenever a redirect happens, one of which talks about the component tree having been recreated from scratch (a bit unsettling).

Other than that it seems to work. I didn't observe flashing.

samselikoff commented 5 years ago

@ryanflorence Thanks for the response :)

These are the errors I see for one redirect without noThrow:

image image

These are all expected? If so I'll probably just stick with my wrapper component. I'm new to React and find all these errors pretty confusing, also I'm used to writing test suites that fail whenever there's an unhandled exception.

Feel free to close if that's the case!

The-Code-Monkey commented 5 years ago

In terms of how a component should work, in my opinion a component shouldn't change how it acts and behaves from dev to prod, we should be making components stable and not need flags.

jamesdefant commented 4 years ago

I get this same behavior (404 page flashes, then redirects) trying to redirect home if authentication fails. I've added authorization from Auth0 to my simple website as per this tutorial: https://auth0.com/blog/securing-gatsby-with-auth0/

...and in my protected page I have a check to see if we're authorized, if not, redirect back home: ` if( !isAuthenticated() ) { return (

)

}`

I can supply code, but there's not much to see. Here's my trace:

redirect_1 redirect_2

inf3rnus commented 2 years ago

Fyi this is still a problem, I spent quite a bit of time trying to discover the source of the flashing in my team's app and it actually occurs as a side effect of loading the Redirect component from the @reach/router module... So, not even using the component, but loading its reference in via the module will produce the issue.

Hoping we can get this thing fixed, I'd try to make a PR if I had time, but unfortunately I don't :/

Tags for others, hopefully this will bubble up this post in Gooogle's result w/ their crawler:

Reach router screen flashes redirect. Reach router screen flickers redirect. Reach router screen flashes when changing paths. Reach router screen flickers when changing paths. Reach router screen flashes when changing routes. Reach router screen flickers when changing routes.