bug: can't control Modal with router params

Prerequisites

[X] I have read the Contributing Guidelines.
[X] I agree to follow the Code of Conduct.
[X] I have searched for existing issues that already report this problem, without success.

Ionic Framework Version

v7.x

Current Behavior

When the router params are changed, a new view instance is created for the parameterized route. This means that we can't re-update content (modal in our case) based on route changes.

We are using modals to show some additional details for items in the list. We also want to change the route path so the user can share the link to the particular item. This is a specific case; however, the issue is not with the modal itself but with the parameterized router.

Expected Behavior

When the router params are changed, the same view instance should be rendered but with new route params.

Steps to Reproduce

In reproduction repo:

Please use the modal-routing-issue branch.
Open the /items/ page.
Click on any item -> the route path changes to /items/:id/, and a modal with details will appear.
Click on the close button -> the modal is not dismissed, although the route path changes.
Click outside of the modal -> the modal will disappear due to internal state.
Click on the same item -> the modal will not open. There's no way to reopen that modal anymore.

to see the expected behaviour please downgrade Ionic to version before 7.5.8:

yarn add @ionic/react@7.5.7 @ionic/react-router@7.5.7

Code Reproduction URL

https://github.com/legendar/ionic-issues/tree/modal-routing-issue

Ionic Info

Ionic:

   Ionic CLI       : 7.2.0
   Ionic Framework : @ionic/react 7.8.2

Capacitor:

   Capacitor CLI      : 5.6.0
   @capacitor/android : not installed
   @capacitor/core    : 5.6.0
   @capacitor/ios     : not installed

Utility:

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

System:

   NodeJS : v18.15.0 (/home/***/.nvm/versions/node/v18.15.0/bin/node)
   npm    : 9.5.0
   OS     : Linux 5.15

Additional Information

This worked well before v7.5.8. The issue was introduced by this commit, which aimed to fix the parameterized routes issue, but also introduced this breaking change.

Video demonstrating the behavior before version 7.5.8:

https://github.com/ionic-team/ionic-framework/assets/139363/663c40bb-9b70-45e4-92ae-229be87a71cd

Video demonstrating the behavior after version 7.5.8:

https://github.com/ionic-team/ionic-framework/assets/139363/914a59a1-8880-46e2-be89-883e3236619b

Another scenario is if we have some filtering controls on the page that also change the URL path (to allow the user to share the URL), every filter change will now (after version 7.5.8) create a new view and reset the entire page state. So, I think the solution introduced in https://github.com/ionic-team/ionic-framework/commit/1705d064cc041e99f432a27207f3aab7fa62c778 is not optimal as it introduces a lot of breaking changes.

Thank you for the issue. The original behavior that was changed in https://github.com/ionic-team/ionic-framework/commit/1705d064cc041e99f432a27207f3aab7fa62c778 was a bug in Ionic Framework. Re-using the same view instance when navigating from one route to another is not a desired behavior; you can read more about the problems that arise in the original issue here: https://github.com/ionic-team/ionic-framework/issues/26524 It sounds like your original solutions were relying on this bug in Ionic, so they will need updating now that the bug is fixed.

I can reproduce the behavior you've described in your linked reproduction, but the issue is due to the code not properly reacting to changes in the route parameters or updating isDetailsModalOpen accordingly. I was able to get everything working with the following changes:

+ import { useEffect, useState } from 'react';

- const {itemId} = useParams<{itemId: string}>();
- const isDetailsModalOpen = Boolean(itemId);
+ const params = useParams<{itemId: string}>();
+ const [isDetailsModalOpen, setDetailsModalOpen] = useState(false);

const closeDetails = () => {
  push("/items/", "none", "pop");
+ setDetailsModalOpen(false);
};

+ useEffect(() => {
+   setDetailsModalOpen(Boolean(params.itemId));
+ }, [params]);

// in the modal render
- <IonTitle>Item {itemId} details</IonTitle>
+ <IonTitle>Item {params.itemId} details</IonTitle>

Can you give this a try and let me know if you have any questions?

Thank you for the reply. Your solution works well in this particular case👍

However, it's not a solution but rather a workaround. It relies on the fact that params is a new object for every component render, and React uses Object.is to compare dependencies in useEffect. So useEffect will always be triggered in this case. But if you try to change the code slightly to make it more prettier, it stops working☹️:

- const params = useParams<{itemId: string}>();
+ const {itemId} = useParams<{itemId: string}>();

- useEffect(() => {
-   setDetailsModalOpen(Boolean(params.itemId));
- }, [params]);
+ useEffect(() => {
+   setDetailsModalOpen(Boolean(params.itemId));
+ }, [itemId]);

The issue is much bigger than just modal showing. It affects the entire component's state and makes the React state useless. For example, if we have a checkbox in a list, there is no simple way to pass this state into the modal without additional setState, which is not the React way. See the example:

const [items, setItems] = useState<number[]>([]);

const toggleItem = (e) => {
    e.stopPropagation();

    const id = e.target.dataset.id;

    if(e.target.checked) {
      setItems(items => [...items, id])
    } else {
      setItems(items => items.filter(i => i !== id))
    }
  }

then render the list of items with checkboxes:

<IonLabel><input type="checkbox" data-id={1} onClick={toggleItem} /> Item 1</IonLabel>

and see what happens:

https://github.com/ionic-team/ionic-framework/assets/139363/f28cc68e-2dbd-42c8-8aa6-da31590a8874

Sure, we can find a workaround for this case as well, but it's not a solution. The actual issue is that Ionic creates a new view instance every time the route is changed. So, after clicking on an item in the list, the modal doesn't react to the parameters because it's another modal in another view instance. You can verify this by changing the push call to just push("/items/") instead of push("/items/", "none", "pop") ("none" added to suppress animation).

Basically, if I specify a parameterized route, I expect the same view to be re-rendered with a new parameter and this is how react works.

<Route path="/items/past/" exact={true}>
  <ItemsList filter="past" /> {/* <-- separate view because in a separate Route component */}
</Route>
<Route path="/items/upcoming/" exact={true}>
  <ItemsList filter="upcoming" /> {/* <-- separate view because in a separate Route component */}
</Route>
<Route path="/items/:itemId?/">
  <ItemsList /> {/* <-- always the same view */}
</Route>

Yes, I understand that in some situations it's good to have a new view item for animations, etc., as shown in #26524 or in any cases where the new route corresponds to a new page. However, in other scenarios like the one I described above, where we have one big page with some internal states or filtering, it's not a good idea. So, there should probably be a mechanism to switch between the two behaviors instead of pushing one that forces us to find a lot of workarounds for other scenarios.

The actual issue is that Ionic creates a new view instance every time the route is changed.

This is the intended behavior, though it differs slightly from how web applications work. Normally in web apps the JS Framework will reuse the component instance, as you noted. For example, if /page/:id maps to Foo, going from /page/1 to /page/2 will re-use the same instance of Foo. For web apps, this is fine. For mobile apps it causes issues due to how stack based navigation works.

As an example, say you are on /page/1 and you enter your email into a text input. You press a button that brings you to /page/2. Typically in a mobile app, the view for /page/2 should be separate from /page/1. Additionally, you should be able to swipe back from /page/2 to /page/1 and still see your email entered on the first view. If we reused the view instance that native mobile paradigm breaks. More specifically, you would not be able to swipe back to /page/1, and the email you entered into the text input would be missing.

I would advise against coupling modals to routing like this. Modals are temporary dialogs that are dependent on the context in which they are presented. By coupling the modal to the global state of your app (the URL), you limit how and when you can show this dialog. A lot of the complexity here could be avoided by pushing a new view with a route instead of trying to couple the modal to the route.

Part of the issue here is how views are being pushed in order to get around the modal/routing behavior. As I noted, I would recommend avoiding this entirely. But if you need to do it this way, here are some adjustments you can make to your current component. I added some documentation to explain some of the changes I made.

import { IonContent, IonHeader, IonPage, IonTitle, IonToolbar, IonList, IonItem, IonLabel, IonModal, useIonRouter, IonButtons, IonButton, createAnimation } from '@ionic/react';
import { useParams } from "react-router";

/**
  Setting routerDirection="none" seems to cause new views to be mounted
  and then never unmounted which is contributing to the problem here. To
  avoid this, we pass in a blank animation to skip the transition effect.
  Seems related to https://github.com/ionic-team/ionic-framework/issues/24074
*/
const animation = () => createAnimation();

export default function ItemsList() {
  const { itemId } = useParams<{itemId: string}>();

  /**
   * Note that since you are navigating backwards without an animation, the view
   * and therefore the modal will be unmounted immediately. As a result, the modal
   * will not have a dismissing animation. This is one of the reasons why it's best
   * to avoid coupling a modal with routing.
   */
  const router = useIonRouter();
  const closeDetails = () => {
    router.goBack();
  };

  return (
    <>
      <IonPage>
        <IonHeader>
          <IonToolbar>
            <IonTitle>Items list. Click on item to open details</IonTitle>
          </IonToolbar>
        </IonHeader>
        <IonContent>
          <IonList>
            <IonItem
              button
              routerLink={`/items/1/`}
              routerAnimation={animation}
              detail={true}
            >
              <IonLabel>Item 1</IonLabel>
            </IonItem>
            <IonItem
              button
              routerLink={`/items/2/`}
              routerAnimation={animation}
              detail={true}
            >
              <IonLabel>Item 2</IonLabel>
            </IonItem>
          </IonList>
        </IonContent>
      </IonPage>
      <IonModal
        isOpen={itemId !== undefined}
        onWillDismiss={closeDetails}
        initialBreakpoint={0.9}
        breakpoints={[0.9]}
      >
        <IonHeader>
          <IonToolbar>
            <IonButtons slot="start">
              <IonButton onClick={closeDetails}>Close</IonButton>
            </IonButtons>
            <IonTitle>Item {itemId} details</IonTitle>
          </IonToolbar>
        </IonHeader>
        <IonContent>
          <p>
            Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum
          </p>
        </IonContent>
      </IonModal>
    </>
  );
}

As I noted previously, the current behavior in Ionic is intended, so I am going to close this.

@liamdebeasi Thanks for the reply.

I understand why this is considered intended behavior. However, I can't agree that it should be the only behavior. The example with the modal is not very illustrative, but as I mentioned earlier, the issue is not specific to modals but affects anything that depends on routing changes.

Here is another example with content filtering. We have a custom filtering element at the top of the page, and underneath that, there's a list that filters based on the choices made in this element. See the screenshot of one of the filtering elements below:

When a user makes a choice, we want to display the same view and just filter the content. However, we also want to change the route so the user can share a link or open the page with a predefined value for this selector. Additionally, we want the swipe action to navigate to the previous view (any other page), but not just change the value in the selector. This is easy to achieve by disabling page animation and using the replace action on the route. We also want to display a bubble with a hint for some choices, and we want the arrow of the bubble to be aligned with the selected choice. The issue here is that we can't use animation for the bubble arrow, as on every route change, a new view with a new bubble element and new arrow will be created.

@liamdebeasi, any suggestions regarding my last note? Or should I create another ticket for this case?

I recently decided to upgrade my app from Ionic 6.x to 8.x. All went pretty smoothly until in testing we realised that pages weren't always showing the right data. E.g. we'd go to post page for post 1, then post 2 and then go back to post 1 and still see post 2's data.

After a lot of hunting through GitHub issues, I found that this new multiple of the same page system had been added to Ionic's React router, which in theory looks like a great feature. Unfortunately our app has been built around the expectation there will only be one of each page (which has been the case since we started the project 4 years ago). All our global state management expects there to only be one post page present, so these new multiple pages were picking up incorrect data from redux, causing the wrong data to be shown.

I appreciate Ionic wants to make the product better and I do believe this new multi-page system is just that, but as far as I can tell this was not documented on the breaking changes document and it means for me to get our app working, it will take days, maybe weeks, of refactoring how our global store is structured and the components interact with it. I agree with @legendar that there should be a way to switch between the two different routing modes, especially as all Ionic apps will have been built with there being only one of each page in mind since before this change was added.

I now have to decide if I want to move the app to v7.5.7, right before this change was added, or commit to all this refactoring to get to the latest version.

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.

ionic-team / ionic-framework