ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.93k stars 13.51k forks source link

bug: Ionic React conditional routing not updating same way as plain react #22586

Open ar-daniel opened 3 years ago

ar-daniel commented 3 years ago

Bug Report

Ionic version:

[ ] 4.x [x] 5.x v5.5.1

Current behavior:

Conditional routing does not update(responsive), It checks and routes for the first time, but when the value/result for the condition changes the routes are not updated. It works in react though

Expected behavior:

Routes as a result of condition should get updated as when the result of the condition changes.

Steps to reproduce:

I've taken blank Ionic project, added two sets of routes and using a condition rendered resulting route. Sample code lands on login page. Upon login ideally should navigate to home page but doesnt. Same code is used in a plain react project, where it works.

Related code:

Ionic React code https://stackblitz.com/edit/ionicreact-ts-issue-conditional-routing?file=src%2FApp.tsx

Plain React code https://stackblitz.com/edit/react-ts-conditional-route?file=src%2FApp.tsx

Other information:

related question in stackoverflow (with no answer) https://stackoverflow.com/questions/59957805/how-can-i-conditionally-render-routes-and-redirect-on-prop-changes-with-react-io

forum question - however the solution provided might work for the first time the condition is checked, not when the condition result is updated. (also included this alternative in the sample code) https://forum.ionicframework.com/t/how-to-render-routes-conditionally/182404/2 https://github.com/aaronksaunders/react-firebase-upload-hook-capacitor/blob/a374d37a72dcb556444b42799959a96b6983ec81/src/App.tsx

I believe this worked in V5.2.x.

Ionic info:


Ionic:

   Ionic CLI       : 6.12.2 (/usr/lib/node_modules/@ionic/cli)
   Ionic Framework : @ionic/react 5.5.1

Capacitor:

   Capacitor CLI   : 2.4.3
   @capacitor/core : 2.4.3

Utility:

   cordova-res : not installed
   native-run  : not installed

System:

   NodeJS : v14.15.1 (/usr/bin/node)
   npm    : 6.14.8
   OS     : Linux 5.4

Kindly help solve the issue. Thank you.

ar-daniel commented 3 years ago

Any support is greatly appreciated. Please help. Is there a way to solve this issue.?

samuelsunil commented 3 years ago

Can you check if this will suffice - https://stackblitz.com/edit/ionicreact-ts-conditional-routing?file=src/App.tsx

Edited above link

samuelsunil commented 3 years ago

@ar-daniel ^

Njs-Monish commented 3 years ago

@ar-daniel yes i too have faced the same issue

ar-daniel commented 3 years ago

@samuelsunil In the code snippet you've provided - The routes are always redirected to /login if not authenticated and / home if authenticated. Unable to load any other route.

eg, before logging in, try to load route /register (which is a non authenticated route, already present in the sample code), it gets redirected to /login. same would be the case with authenticated routes when other routes than /home is present.

samuelsunil commented 3 years ago

@ar-daniel Please look at the latest changes now - https://stackblitz.com/edit/ionicreact-ts-conditional-routing?file=src%2FApp.tsx

1) Yes. That's where I would differ in the implementation of the Routes in the example you have. You should register all the routes, but for the authenticated routes, create like a "RestrictedRoute" component that internally checked for authentication/Authorization and Redirect internally, rather than on the main component. 2) My changes were just to show the usage of the state check with in the Router; and that seems to be resolved. The rest is the logic you have to move internally and use the "history" object of the react-router to navigate. Let me know if this works. I made changes to the Login and handle_login event

samuelsunil commented 3 years ago

@ar-daniel : Did it work out ?

ar-daniel commented 3 years ago

@samuelsunil I've been working till now for a workaround with use of history object as per your suggestion. hence the delay. Before going to my findings, a quick tip in your code- the history object need not be passed from component, it can be made available in the main app where the authentication/ history object is used. Just we need to move the IonReactRouter to the index file and wrap component with it. As it avoids changing the underlying code, and passing history may not work/ feasible when doing a programmatic navigation ( like in redux use case, logout to be triggered irrespective of the page he is in, etc )

Attempt for workaround & findings

1) Making a history.push() or history.replace() triggers a re-analysis of the route I believe and authroutes get active. Including it a useEffect()

  let useEffectFirstRun_isAuthenticated = useRef<boolean>(true)
...
  useEffect(() => {
    if (useEffectFirstRun_isAuthenticated.current === false) {  // to avoid first run
    ...
      if (isAuthenticated) {
        history.push('/profile')
      }
      else {
        history.push('/login')
      }
    }
    useEffectFirstRun_isAuthenticated.current = false

  }, [isAuthenticated])

This now gets into the authenticated routes, but loads /home instead of /profile or any other auth route that is pushed. Some how the Redirect inside auth routes is always triggered. ( 1st issue )

2) Removing the Redirect from the authroutes now navigates properly to /profile or any other auth page. But incorrect/ unknown routes cannot be redirected ( this is the second issue with workaround)

Next, in actual use-case where user tries to load any of auth route is auto authenticated and to be navigated to the route user first asked for. It seen in the next 2 cases.

3) Trying simulate a server auto authenticate using timeout, (while keeping hardcoded history.push for now. Still with Redirect already disabled

  const [autoLogin, setAutoLogin] = useState<boolean>(true)
...
  useEffect(() => {

    if(autoLogin === true ) {       // to simulate server based authentication
      setTimeout(() => {
        setIsAuthenticated(true)    
      }, 1000);                   // 1 sec so its noticable 
    }

  }, [])

now whatever auth route is called for, auto logs in after a 1 sec, loads the hardcoded route - in this case ('/profile') But does not load when same hardcoded route is tried to be loaded, wherein it loads only a blank page. ( this is 3rd issue ) (I suspect, when the hardcoded route is tried to be loaded, - initially it renders a blank page as its not authenticated, but after authentication, again when the same route is called for, tries to load the same blank render page from cache, This is a guess - I dont know how it works. )

4) The step here is the actual test case for the work around with history. Getting the initial route asked using the useLocation and storign in a ref obejct to be used later for history push when authenticated

  const location = useLocation()
  let firstLocationRef = useRef<string>('/')
....
  useEffect(() => {
    firstLocationRef.current = location.pathname
    console.log('firstLocationRef.current - ')
    console.log(firstLocationRef.current)
...
  }, [])
...
  useEffect(() => {
    if (useEffectFirstRun_isAuthenticated.current === false) {

      console.log('Authentication state - ', isAuthenticated)
      if (isAuthenticated) {

        console.log(`-> pushing ->  history.replace(${firstLocationRef.current})`)
        history.push(firstLocationRef.current)
      }
      else {
        console.log("-> pushing -> history.replace('/login')")
        history.push('/login')
      }
    }
    useEffectFirstRun_isAuthenticated.current = false

  }, [isAuthenticated])

now it does not load any auth route as because whatever route is asked for is the same route that is pushed in the history.push upon authentication. Suspected reason in the previous point.

Worked out sample code All the four vairants are uploaded in the below stackblitz link To make checking easier, 4 different app files are present and can turned on/off in the index js based on the case for testing.

https://stackblitz.com/edit/ionicreact-ts-issue-workaround-conditional-routing?file=src%2Findex.tsx

Pl fork the repo to local computer and test it as I've not ensured it works/ testable online in stackblitz.

Using the history.push or history.replace seemed like a good workaround. but didn't work for the above reasons.

In any case there is basic issue present in the ionic framework which need to be checked - so that the original problem be resolved or even for the workaround with history object to work, as its already working without issues in plain react.

I already tired to work it out for 8 days but no luck. Requesting Ionic team to help in the matter as its a real bottleneck. Thanks

rgoldiez commented 3 years ago

@ar-daniel - we solved this writing a ProtectedRoute component that checks auth status and redirects the user in the render prop of the Route component if the user is not authenticated. ProtectedRoute takes the same props as Route. If the user navigates to a protected route, the render prop of the `Route component will either return the “protected route” if the user is authenticated and if not redirect the user to the login page. I’m not sure if this behavior is what you’re after, but it works quite well.

ar-daniel commented 3 years ago

@rgoldiez . Does it work with v 5.5.x. It had worked for me in v5.2.x, I believe. It doesn't work for me though. I've made 2 attempts anyways.

1) Made all the auth and non-routes as protected routes with check if its isAuthenticated or redirect if its not. Upon clicking login, it should auto navigate to /home, but doesn't.

.... 
      <IonRouterOutlet>

        <Switch>
          <Route exact path="/login" render={() => !isAuthenticated ? <Login handle_login={handle_login} isAuthenticated={isAuthenticated} /> : <Redirect to="/home" />} />
          <Route exact path="/register" render={() => !isAuthenticated ? <Register /> : <Redirect to="/home" />} />
          <Route exact path="/home" render={() => isAuthenticated ? (<Home handle_logout={handle_logout} isAuthenticated={isAuthenticated} />) : <Redirect to="/login" />} />
          <Route exact path="/profile" render={() => isAuthenticated ? (<Profile handle_logout={handle_logout} isAuthenticated={isAuthenticated} />) : <Redirect to="/login" />} />

          <Redirect to={isAuthenticated ? "/home" : "/login" } />
        </Switch>

      </IonRouterOutlet>
....

2) More practical test case having an auto authenticate ( autologin after 2 sec - same logic as per my prev comment)

and when feed with the original route with history push upon auto login

both these snippets are added in the variants of workaround. https://stackblitz.com/edit/ionicreact-ts-issue-workaround-conditional-routing?file=src%2Findex.tsx

now the second test case above is enabled in the index.js file

Please try the samples yourself.

rgoldiez commented 3 years ago

Hi @ar-daniel - we've been using it in a production app with 5.3.x for a couple of months and just moved to 5.5.x without issue. I'm not sure if our use cases are different than yours but we have used "protected routes" quite successfully in ionic/react.

ar-daniel commented 3 years ago

@rgoldiez . I get this authentication status check would be present in most application and generally there wouldn't be an issue related to this. Me getting stuck here is lame.

Just to clarify- The main use case here is the authentication status is set after a second or so after the app component is rendered or set upon manual change. And not before first time the routes are rendered. (Setting the authentication status prior to render of component would not pose a problem. ) In effect, it actually needs re-render upon authentication status change. Is this a similar case in your application as well ?

And by protected routes, is it the same ones as above in my prev comment that you are referring or do you have a different style ?

ar-daniel commented 3 years ago

both these snippets are added in the variants of workaround. https://stackblitz.com/edit/ionicreact-ts-issue-workaround-conditional-routing?file=src%2Findex.tsx

now the second test case above is enabled in the index.js file

I hadnt saved the files earlier in Stackblitz, hence it might be missed in the online repo. Now its available.

rgoldiez commented 3 years ago

@ar-daniel - In our case, we know auth status on the initial render. We do have a few other use cases where we redirect by setting window.location.href to get the behavior we need. Maybe that's a solution for you? Meaning, have a hook that listens for your auth status and redirect via window.location.href if it changes?

elylucas commented 3 years ago

Hey all, I wanted to provide an update on this one.

The crux of the issue is that IonRouterOutlet doesn't currently support swapping out routes here and there well (like using one set of routes for authenticated users and one set of routes for non-authed users). This is something we want to support well and are working on it, but it is a large change and is taking time to go through and test. Hopefully we will have an update for it soon.

wmadden commented 3 years ago

@ar-daniel I believe <IonRouterOutlet> will render all routes whose path will match - unlike <Switch> - which might explain the behavior you're observing.

pdsullivan commented 3 years ago

hey, @elylucas it's been a few months, any updates? we have run into this bug trying to upgrade to the latest version of ionic-react and we seem to be blocked.

captaincole commented 3 years ago

Hey all, I'm also having the same issue here too. I've got a private route component that I'm using with an auth check described below.


const PrivateRoute: React.FC<PrivateRouteProps> = (props) => {
  // Watch Authentication!
  const { userId } = useContext(UserContext)
  return (
    <Route
      path={props.path}
      exact={props.exact}
      render={() => {
        return userId ? <props.component /> : <Redirect to="/login" />
      }} />
  )
}

And when I use this in combination with a history.push('/<new_route') I see two transition animations. Video below.

https://user-images.githubusercontent.com/7246257/126534756-64df35b4-1ae0-43e8-bfe3-91956fdd71d6.mov

I've tried many (if not all) of the suggestions but I cant seem to figure this out. Interestingly, when not using a tranistion (eg. routerDirection="none" there is no double transition.

Here is an example of my routerOutlet JSX

    <IonApp>
      <IonReactRouter>
        {loading && <LoadingPage />}
        {!loading && (
          <IonSplitPane contentId="main">
            {userId && <Menu />}
            <IonRouterOutlet id="main">
              <Route path="/login" exact={true} component={LoginPage} />
              <Route path="/login/email" exact={true} component={EmailLogin} />
              <PrivateRoute
                path="/settings"
                exact={true}
                component={SettingsPage}
                userId={userId}
              />
              ...
</IonApp>
the1979 commented 3 years ago

@captaincole, we ran into the same issue (among many others).

For us, the double animation was solved by ensuring we are passing all props to the child <Route>. I think the crux is an implicit computedMatch property (~ match) that is normally passed from IonRouterOutlet (and <Switch>) to all child Routes. Guessing Ionic is using this to control animations.

So:

const PrivateRoute: React.FC<PrivateRouteProps> = ({ component, ...rest }) => {
  // Watch Authentication!
  const { userId } = useContext(UserContext)
  return (
    <Route
      {...rest}
      render={() => {
        return userId ? <component /> : <Redirect to="/login" />
      }} />
  )
}

That works for us, though routing still breaks after logging out/in more than once.

condinoaljoseph commented 10 months ago

hey, @elylucas it's been a few months, any updates? we have run into this bug trying to upgrade to the latest version of ionic-react and we seem to be blocked.

Hi @pdsullivan, we've also run into this bug, have you guys already have a work around on this one? we're using the latest ionic version v7 by the way.

okletstryit commented 8 months ago

same bug. Can't render the page afeter auth state is changed to Not logged in. It changes an URL but doesn't render a page

nadavhalfon commented 8 months ago

Same here

3oax commented 7 months ago

same problem with latest version. when the protected route redirects to the login page and i log back in, i get this animation. also i noticed, that the ion-router-outlet prepends the page to the outlet. in all other cases the pages get appended.

edit: when then animation to the page is set to "none" (router.push('/<new_route'), 'none') there is no bug