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

Issue: Support NotFound Routes in IonRouterOutlet, old title: bug: Routing fall-through is not working on latest React build #20105

Closed
ebk46 opened this issue Dec 18, 2019 · 7 comments
Labels
package: react @ionic/react package

Comments

@ebk46
Copy link

ebk46 commented Dec 18, 2019

Bug Report

Ionic version:
[x] 4.11.7

Current behavior:
When setting up sequential routes, all routes are rendered in layers rather than just the most specific.

Expected behavior:
Only the most specific route should be matched and rendered.

Steps to reproduce:

  1. Set up multiple routes with different specificity. Standard routing indicates that the first matching route (or most specific route) should be matched and rendered. The Stackblitz included below has "/page1" followed by a catchall "/" for any routes that don't match.

  2. Route to the first/most specific route ("/page1" in the case of the stackblitz)

  3. Note that the second route is rendered "over" the first, making it seem like the first isn't rendered at all.

Related code:

https://stackblitz.com/edit/ionic-v4-11-7-route-not-found

In the stackblitz, in addition to seeing the NotFound page rendered when the path is "/page1", you can look at the console to see that both routes are actually being rendered. Further, if you swap the order of the routes, you can see that the page appears to render correctly but both routes are still being rendered (NotFound is just underneath Page1).

Other information:

Ionic info:

Ionic:

   Ionic CLI       : 5.4.13 (/Users/evan/.config/yarn/global/node_modules/ionic)
   Ionic Framework : @ionic/react 4.11.7

Capacitor:

   Capacitor CLI   : 1.4.0
   @capacitor/core : 1.4.0

Utility:

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

System:

   NodeJS : v13.1.0 (/usr/local/Cellar/node/13.1.0/bin/node)
   npm    : 6.12.1
   OS     : macOS Catalina```
@ionitron-bot ionitron-bot bot added the triage label Dec 18, 2019
@elylucas
Copy link
Contributor

Hi @ebk46,

This is by design and functions similar to how BrowserRouter works. By default, any route that matches will be rendered. To fine grain control which routes you want to render in an IonRouterOutlet, use exact props on them. So to solve your above stackblitz sample above, put an exact attribute on the '/' route and that route will only be rendered on the root path.

However, I do realize that this does not solve the NonFound use case you are going for here. With BrowserRouter, you would put your routes into a Switch statement and have the last route not have a path attribute, so if no routes match it would it the route like so:

<BrowserRouter>
  <Switch>
    <Route exact path="/page1" component={Page1} />
    <Route exact path="/page1/details" component={Page1Details} />
    <Route component={NotFound} />
  </Switch>
</BrowserRouter>

However, switches are not supported in IonRouterOutlets, because we need to control and keep pages around in the dom even if a user navigates away from them for page transition and state purposes.

So doing a NotFound route is not possible inside of an IonRouterOutlet. You would have to do it outside of the IonRouterOutlet and that would require you to setup routing in a way that might not be ideal for your app.

I'll keep this issue open and repurpose it to investigate a way to do NotFound routes inside of IonRouterOutlet.

@elylucas elylucas changed the title bug: Routing fall-through is not working on latest React build Issue: Support NotFound Routes in IonRouterOutlet, old title: bug: Routing fall-through is not working on latest React build Dec 18, 2019
@devinshoemaker
Copy link
Contributor

So essentially, you would need to start with a BrowserRouter that has a Switch with all of your accepted routes, and then a fallback route. From there, all of the accepted routes would then route to a component that has IonRouterOutlet to finish the routing. This would result in your accepted routes being listed twice, but I suppose it would work. Is this what you mean, or am I totally off-base?

@elylucas
Copy link
Contributor

You could stick with IonReactRouter, but you would have to define those initial routes outside of IonRouterOutlet like so:

<IonReactRouter>
          <Switch>
            <Route path='/app' render={() => (
              <IonRouterOutlet>
                <Route exact path='/app/page1' component={Page1} />
                <Route path='/app/page1/details' component={Page1Details} />
                <Redirect from="/app" to="/app/page1" exact />
              </IonRouterOutlet>
            )} />
            <Route component={NotFound} />
          </Switch>
        </IonReactRouter>

Downside here is you have to have un otherwise unneeded sub path ( /app here) that your IonRouterOutlet would listen to, and any unknown routes to /app (like /app/blah) wouldn't be covered by the NotFound route.

full example:
https://stackblitz.com/edit/ionic-v4-11-7-route-not-found-nvsctw

@devinshoemaker
Copy link
Contributor

I see. This is essentially what I thought you had meant, just simplified with the /app route predicate. I tested your solution and it seems to work on my project, thanks!

@elylucas elylucas added the package: react @ionic/react package label Dec 19, 2019
@ionitron-bot ionitron-bot bot removed the triage label Dec 19, 2019
@devinshoemaker
Copy link
Contributor

devinshoemaker commented Jan 18, 2020

I thought of another solution to this problem. Basically I check and see if the user is currently using Capacitor or not, and if so, then I render the Ionic router. Otherwise, I use the standard React router with a switch component to enable fallback on browsers, while retaining Ionic specific functionality for native apps.

const AppRoutes: React.FC = () => {
  return (
    <>
      <Route path="/login" component={Login} exact={true} />
      <Route exact path="/" component={() => <Redirect to="/login" />} />
    </>
  );
};

const CapacitorRoutes: React.FC = () => {
  return (
    <IonReactRouter>
      <IonReactRouter>
        <AppRoutes />
      </IonReactRouter>
    </IonReactRouter>
  );
};

const WebRoutes: React.FC = () => {
  return (
    <Router>
      <Switch>
        <AppRoutes />
      </Switch>
    </Router>
  );
};

export const App: React.FC = () => {
  const isCapacitor = isPlatform('capacitor');
  return <IonApp>{isCapacitor ? <CapacitorRoutes /> : <WebRoutes />}</IonApp>;
};

export default App;

EDIT
I found some issues with the above implementation and made some updates. Here's what I have now, which fixes a bug that I found with some routes, and takes isCapacitor as a prop which will help make unit testing easier.

const CapacitorRoutes: React.FC = () => {
  return (
    <IonReactRouter>
      <IonReactRouter>
        <Route exact path="/login" component={Login} />
        <Route exact path="/test" component={Login} />
        <Route exact path="/" component={() => <Redirect to="/login" />} />
      </IonReactRouter>
    </IonReactRouter>
  );
};

const WebRoutes: React.FC = () => {
  return (
    <Router>
      <Switch>
        <Route exact path="/login" component={Login} />
        <Route exact path="/test" component={Login} />
        <Route exact path="/" component={() => <Redirect to="/login" />} />
        <Route render={() => <h2>Page not found</h2>} />          
      </Switch>
    </Router>
  );
};

export interface UnauthenticatedAppProps {
  isCapacitor: boolean;
}

export const UnauthenticatedApp: React.FC<UnauthenticatedAppProps> = (
  props: UnauthenticatedAppProps
) => {
  return <IonApp>{props.isCapacitor ? <CapacitorRoutes /> : <WebRoutes />}</IonApp>;
};

@elylucas
Copy link
Contributor

Not round routes are now available in 4.11.9.

To use one, setup one of the routes below inside of the <IonRouterOutlet>

<Route render={() => <Redirect to="/tab1" />} />

or

<Redirect to="/tab1" />

@ionitron-bot
Copy link

ionitron-bot bot commented Feb 22, 2020

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Feb 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: react @ionic/react package
Projects
None yet
Development

No branches or pull requests

3 participants