bug: Ionic React Router is not creating new instances of views with parameterized routes

[ericbf]

Prerequisites

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

Ionic Framework Version

• v6.x

Current Behavior

When navigating, it seems as if ionic router (under react, at least) is using the wrong instance of a re-used view to push on the stack, clobbering the old instance. It’s a little hard to explain in prose.

Let’s say you start at a FormPage at path "/form/0".

You input some data and submit, and it pushes to a LinkPage at "/link".

The stack is now [FormPage (at path "/form/0"), LinkPage].

LinkPage has a link to "/form/1" which uses the same page component (FormPage) using a route param.

The stack should now push a brand new instance of FormPage, but what happens is that the old instance is pushed onto the stack (the form inputs have the values that were submitted at "/form/0", even though the new path is "/form/1").

Let’s say you now navigate back twice (to the first form) using IonBackButtons, you’ll find that the inputs have all been cleared, seemingly resetting that instance of FormPage.

The stack should be [FormPage (at path "/form/0"), LinkPage, new instance of FormPage (at path "/form/1")]

But the stack seems to be [reseted instance of FormPage (at path "/form/0"), LinkPage, first instance of FormPage (at path "/form/1")]

See the repro for more details!

Expected Behavior

I understand from #19531 and all the other linked/related issues that router caching when pushing onto the stack is a "feature", not a bug (sigh). To get around that, I added a route param to the path that should make it unique and thusly get me a new instance when I push onto the stack. I expect each page rendered at a unique path to be its own instance.

Steps to Reproduce

Clone repro repo, install deps, ionic serve

Type something in the input and submit
Click the link
Observe that the input has your value you just typed a minute ago (it should be a brand spanking new instance of the FormPage)
Click back button
Click back button
Observe that your input value is no longer on the page in the history (the page in the stack should have remained intact)

Code Reproduction URL

https://github.com/ericbf/ionic-react-navigation-caching-bug

Ionic Info

Ionic:

Ionic CLI : 6.20.5 (/Users/eric/.npm/_npx/f6fddb685269761d/node_modules/@ionic/cli)
Ionic Framework : @ionic/react 6.4.1

Capacitor:

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

Utility:

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

System:

NodeJS : v19.2.0 (/usr/local/bin/node)
npm : 8.19.3
OS : macOS

Additional Information

I know there’s some other issues relating to caching and navigation (#19531, among others). Those all state that when you use a unique path (like with a route param), it should not use the cached view. That seems to be broken here.

[ionitron-bot]

Thanks for the issue! This issue has been labeled as holiday triage. With the winter holidays quickly approaching, much of the Ionic Team will soon be taking time off. During this time, issue triaging and PR review will be delayed until the team begins to return. After this period, we will work to ensure that all new issues are properly triaged and that new PRs are reviewed. In the meantime, please read our Winter Holiday Triage Guide for information on how to ensure that your issue is triaged correctly. Thank you!

[sean-perkins]

@ericbf thank you for the reproduction and the write-up on the issue. I am able to reproduce and you are correct, when pushing a new route with a unique path parameter, it should be constructing a new instance of the view/component.

This problem is also reproducible when going from /form/0 → /form/1.

This does not occur if you move from parameterized routes to static routes, e.g.:

<Route path="/form/1" component={FormPage} />
<Route path="/form/2" component={FormPage} />
<Route path="/form/3" component={FormPage} />

This means somewhere in our comparison logic for parameterized routes, is either matching /form/:index against /form/:index (not taking into account the path value) or a similar comparison is incorrect.

I'll categorize this as a bug so that we can digest it into our backlog.

Thanks!

[arifeld]

Hi, is there any estimated ETA on when this issue may be fixed? I am currently working on a project in Ionic React and this bug has unfortunately made it impossible to continue working on it.

Apologies for being brash, just trying to determine whether I should bite the bullet and switch to Ionic Angular given the deadline of my project :'(

[sean-perkins]

@arifeld we do not communicate ETAs on bugs/features, as to not over promise and under deliver. This bug is not currently in our active sprint.

I did further exploration of this problem and greatly narrowed the scope. If you want to experiment with the current state of my exploration you can use this dev-build:

npm install @ionic/react@6.5.1-dev.11674100664.17f2685c @ionic/react-router@6.5.1-dev.11674100664.17f2685c

Note: I have only validated the workflow from the original reproduction (parameterized route, static route, parameterized route). I have not tested any other routing scenario or even ran our e2e tests against my changes.

As I previously speculated, our routing integration is matching the previous route using matchPath and that is returning the previous parameterized route as the entering view instead of creating a new instance since the url is different.

[aeharding]

Hi there! I just wanted to toss out a quick comment that this is the #⁠1 issue wefwef has with Ionic today. I would really appreciate if this was fixed!

Just curious, is this planning to be resolved with the routing refactoring?

[sean-perkins]

@aeharding have you tested with the dev-build here: #26524 (comment)? If so, does it resolve the challenge you are experiencing? If not, would you be willing to test and let me know your results?

This problematic behavior could be resolved much sooner than the routing refactor. Here is the estimated changed as it stands today: 7f2685c

If I can get additional feedback on if that proposed change works as expected, without any other regressed routing behaviors, I can open it up for review with the larger team.

[aeharding]

Hi @sean-perkins!

Thanks so much for the quick reply. I just tried out the build. It seems to be working to an extent! Route with different parameters are not being reused!

However I have observed three problems in my testing, the first being a blocker.

If it would help, I can create minimal code sample reproductions. Just let me know! I apologize if my explanations are unclear.

Assumed app routes

Assume app with two routes in the router:

1. /foo/:user
2. /baz

Problem one

Summary: Navigating through parameterized routes and then navigating back can leave the page blank

Repro:

1. Navigate /foo/alex
2. Navigate /baz
3. Navigate /foo/sean
4. press browser back button to go to /baz
5. press browser back button again to go to /foo/alex. Observe blank tab content - ion-page in the DOM is still set to hidden (it's the right page if you remove the ion-page-hidden class, just not visible)

Problem two

Background: This problem is not a blocker like issue 1 is, but would be nice to be fixed (and also potentially related to issue 1?)

Summary: If you navigate to the same parameterized route separated by an unrelated route, the router appears to reuse the first instance. Then when you navigate back, a new instance is shown.

Repro:

1. Navigate /foo/alex. First instance of user page created.
2. Navigate /baz
3. Navigate /foo/alex. Observe the router is reusing the first instance of user page. (This isn’t necessarily a problem, but it would be ideal to be a new instance.)
4. Press back twice.
5. Observe instead of showing the first instance of user page, it creates a second instance. This is a problem, because the user might have state related this particular instance of the route that they were expecting to go back to

Problem 3 (MOVED TO #27900)

Background: This problem is also in the latest production Ionic build. But I thought I would mention it in case it is related. Please let me know if you would like a separate issue for this.

Summary: Swipe gestures stop working after navigating through parameterized routes

1. Navigate /foo/alex
2. Navigate /baz
3. Navigate /foo/sean
4. Swipe back once
5. Try to swipe back again, see it is not possible (you need to hit the back button of the browser or in the header)

[aeharding]

Adding another test case that I think is related to problem 3 above! Let me know if you'd like this in a separate issue.

Problem 4 (MOVED TO #27900)

Background: This problem is regarding replacing a parameterized route using useIonRouter. There's also a related cosmetic issue here: #24260 but the report in the comment I'm writing is a functional issue.

Assumed app routes

/:name
/:name/details

Repro

• Go to /one
• Replace route with /two
• Go to /two/details
• See swipe back is broken

Note that this bug doesn't happen without parameterized routes (for example, if I explicitly define /one, /two, /one/details and /two/details it works great)

End user reported here: https://lemmy.world/post/2368724

[sean-perkins]

@aeharding thank you so much for the additional issue reports! Let's track the swipe to go back behavior separately.

Problem one is definitely a side effect of my dev-build. I'm still discovering why re-creating the entering view would cause the page to not transition in correctly when navigating back.

Problem two sounds related to the primary issue here, so I can track that together until I have more debugging information to say otherwise and then can split the ticket.

[aeharding]

@sean-perkins No problem, just created #27900 to track gesture issues!

[sean-perkins]

👋 hello everyone! This has been a fun issue to debug and proposed a resolution for!

Can anyone that is effected by this, try out the dev-build and let me know if you run into any regressions or odd behavior?

npm install @ionic/react@7.5.8-dev.11701375195.1e63388c @ionic/react-router@7.5.8-dev.11701375195.1e63388c

Here is an example of the behavior I've tested locally:

Kapture.2023-11-30.at.15.05.04.mp4

[aeharding]

@sean-perkins Preliminary tests look great!!

The only thing I am seeing, (which is existing behavior in ionic's react router), is that pushing the exact same route a second time, for example /foo -> /bar -> /foo still has swipe back broken when you try to swipe back to the initial instance (and doesn't create a new instance of /foo). Is that considered a separate issue?

[sean-perkins]

@aeharding thanks for the quick testing! I also have an updated dev-build after running through all our test cases for routing:

npm install @ionic/react@7.5.8-dev.11701383555.17254408 @ionic/react-router@7.5.8-dev.11701383555.17254408

Yes! I would consider the swipe to go back a separate issue/fix. The target area for that change will be here:

ionic-framework/packages/react-router/src/ReactRouter/StackManager.tsx

Lines 213 to 311 in a3cd204

	const canStart = () => {
	const config = getConfig();
	const swipeEnabled = config && config.get('swipeBackEnabled', routerOutlet.mode === 'ios');
	if (!swipeEnabled) {
	return false;
	}
	const { routeInfo } = this.props;
	const propsToUse =
	this.prevProps && this.prevProps.routeInfo.pathname === routeInfo.pushedByRoute
	? this.prevProps.routeInfo
	: ({ pathname: routeInfo.pushedByRoute || '' } as any);
	const enteringViewItem = this.context.findViewItemByRouteInfo(propsToUse, this.id, false);
	return (
	!!enteringViewItem &&
	/**
	* The root url '/' is treated as
	* the first view item (but is never mounted),
	* so we do not want to swipe back to the
	* root url.
	*/
	enteringViewItem.mount &&
	/**
	* When on the first page (whatever view
	* you land on after the root url) it
	* is possible for findViewItemByRouteInfo to
	* return the exact same view you are currently on.
	* Make sure that we are not swiping back to the same
	* instances of a view.
	*/
	enteringViewItem.routeData.match.path !== routeInfo.pathname
	);
	};
	const onStart = async () => {
	const { routeInfo } = this.props;
	const propsToUse =
	this.prevProps && this.prevProps.routeInfo.pathname === routeInfo.pushedByRoute
	? this.prevProps.routeInfo
	: ({ pathname: routeInfo.pushedByRoute || '' } as any);
	const enteringViewItem = this.context.findViewItemByRouteInfo(propsToUse, this.id, false);
	const leavingViewItem = this.context.findViewItemByRouteInfo(routeInfo, this.id, false);
	/**
	* When the gesture starts, kick off
	* a transition that is controlled
	* via a swipe gesture.
	*/
	if (enteringViewItem && leavingViewItem) {
	await this.transitionPage(routeInfo, enteringViewItem, leavingViewItem, 'back', true);
	}
	return Promise.resolve();
	};
	const onEnd = (shouldContinue: boolean) => {
	if (shouldContinue) {
	this.skipTransition = true;
	this.context.goBack();
	} else {
	/**
	* In the event that the swipe
	* gesture was aborted, we should
	* re-hide the page that was going to enter.
	*/
	const { routeInfo } = this.props;
	const propsToUse =
	this.prevProps && this.prevProps.routeInfo.pathname === routeInfo.pushedByRoute
	? this.prevProps.routeInfo
	: ({ pathname: routeInfo.pushedByRoute || '' } as any);
	const enteringViewItem = this.context.findViewItemByRouteInfo(propsToUse, this.id, false);
	const leavingViewItem = this.context.findViewItemByRouteInfo(routeInfo, this.id, false);
	/**
	* Ionic React has a design defect where it
	* a) Unmounts the leaving view item when using parameterized routes
	* b) Considers the current view to be the entering view when using
	* parameterized routes
	*
	* As a result, we should not hide the view item here
	* as it will cause the current view to be hidden.
	*/
	if (enteringViewItem !== leavingViewItem && enteringViewItem?.ionPageElement !== undefined) {
	const { ionPageElement } = enteringViewItem;
	ionPageElement.setAttribute('aria-hidden', 'true');
	ionPageElement.classList.add('ion-page-hidden');
	}
	}
	};
	routerOutlet.swipeHandler = {
	canStart,
	onStart,
	onEnd,
	};

[ionitron-bot]

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.