Skip to content

Commit 1705d06

Browse files
authored
fix(react): router creates new view instances of parameterized routes (#28616)
Issue number: Resolves #26524 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## Definitions **Parameterized routes**: A route that includes one or more variables in the path segments, such as `/form/:index`. ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> When an application routes from a parameterized route, to an intermediary route, to the same parameterized route, but with a different value/url, Ionic's routing logic is incorrectly reusing the view item from the first instance of the parameterized route instead of calculating that the matched path is different. This results in the wrong view item being recycled and rendered. Another way of representing it: - User navigates to `/form/0` which resolves `FormPage` - User enters `0` into the form and submits the form - User navigates to `/link`, which resolves `LinkPage` - User navigates to `/form/1`, which resolves `FormPage` - However, instead of creating a new instance of `FormPage` it is reusing the instance of `FormPage` from `/form/0` which includes the form having `0` in the input. - The user now sees a "new view", but with cached data in the form. This is not expected or desired. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Ionic's routing logic will validate if the entering view item matches the match route data before reusing it. This results in new instances of the view item being constructed when using parameterized routes. https://github.com/ionic-team/ionic-framework/assets/13732623/e7e3d03f-2848-4429-9f60-9074d0761e45 ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Dev-build: `7.5.8-dev.11701383555.17254408`
1 parent 60303aa commit 1705d06

File tree

5 files changed

+131
-50
lines changed

5 files changed

+131
-50
lines changed

packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import type { RouteInfo, ViewItem } from '@ionic/react';
22
import { IonRoute, ViewLifeCycleManager, ViewStacks, generateId } from '@ionic/react';
33
import React from 'react';
4-
import { matchPath } from 'react-router';
4+
5+
import { matchPath } from './utils/matchPath';
56

67
export class ReactRouterViewStack extends ViewStacks {
78
constructor() {
@@ -23,21 +24,16 @@ export class ReactRouterViewStack extends ViewStacks {
2324
ionRoute: false,
2425
};
2526

26-
const matchProps = {
27-
exact: reactElement.props.exact,
28-
path: reactElement.props.path || reactElement.props.from,
29-
component: reactElement.props.component,
30-
};
31-
32-
const match = matchPath(routeInfo.pathname, matchProps);
33-
3427
if (reactElement.type === IonRoute) {
3528
viewItem.ionRoute = true;
3629
viewItem.disableIonPageManagement = reactElement.props.disableIonPageManagement;
3730
}
3831

3932
viewItem.routeData = {
40-
match,
33+
match: matchPath({
34+
pathname: routeInfo.pathname,
35+
componentProps: reactElement.props,
36+
}),
4137
childProps: reactElement.props,
4238
};
4339

@@ -106,7 +102,7 @@ export class ReactRouterViewStack extends ViewStacks {
106102
}
107103

108104
findLeavingViewItemByRouteInfo(routeInfo: RouteInfo, outletId?: string, mustBeIonRoute = true) {
109-
const { viewItem } = this.findViewItemByPath(routeInfo.lastPathname!, outletId, false, mustBeIonRoute);
105+
const { viewItem } = this.findViewItemByPath(routeInfo.lastPathname!, outletId, mustBeIonRoute);
110106
return viewItem;
111107
}
112108

@@ -115,7 +111,10 @@ export class ReactRouterViewStack extends ViewStacks {
115111
return viewItem;
116112
}
117113

118-
private findViewItemByPath(pathname: string, outletId?: string, forceExact?: boolean, mustBeIonRoute?: boolean) {
114+
/**
115+
* Returns the matching view item and the match result for a given pathname.
116+
*/
117+
private findViewItemByPath(pathname: string, outletId?: string, mustBeIonRoute?: boolean) {
119118
let viewItem: ViewItem | undefined;
120119
let match: ReturnType<typeof matchPath> | undefined;
121120
let viewStack: ViewItem[];
@@ -140,16 +139,24 @@ export class ReactRouterViewStack extends ViewStacks {
140139
if (mustBeIonRoute && !v.ionRoute) {
141140
return false;
142141
}
143-
const matchProps = {
144-
exact: forceExact ? true : v.routeData.childProps.exact,
145-
path: v.routeData.childProps.path || v.routeData.childProps.from,
146-
component: v.routeData.childProps.component,
147-
};
148-
const myMatch = matchPath(pathname, matchProps);
149-
if (myMatch) {
150-
viewItem = v;
151-
match = myMatch;
152-
return true;
142+
143+
match = matchPath({
144+
pathname,
145+
componentProps: v.routeData.childProps,
146+
});
147+
148+
if (match) {
149+
/**
150+
* Even though we have a match from react-router, we do not know if the match
151+
* is for this specific view item.
152+
*
153+
* To validate this, we need to check if the path and url match the view item's route data.
154+
*/
155+
const hasParameter = match.path.includes(':');
156+
if (!hasParameter || (hasParameter && match.url === v.routeData?.match?.url)) {
157+
viewItem = v;
158+
return true;
159+
}
153160
}
154161
return false;
155162
}
@@ -171,13 +178,9 @@ export class ReactRouterViewStack extends ViewStacks {
171178
}
172179
}
173180

174-
function matchComponent(node: React.ReactElement, pathname: string, forceExact?: boolean) {
175-
const matchProps = {
176-
exact: forceExact ? true : node.props.exact,
177-
path: node.props.path || node.props.from,
178-
component: node.props.component,
179-
};
180-
const match = matchPath(pathname, matchProps);
181-
182-
return match;
181+
function matchComponent(node: React.ReactElement, pathname: string) {
182+
return matchPath({
183+
pathname,
184+
componentProps: node.props,
185+
});
183186
}

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import type { RouteInfo, StackContextState, ViewItem } from '@ionic/react';
22
import { RouteManagerContext, StackContext, generateId, getConfig } from '@ionic/react';
33
import React from 'react';
4-
import { matchPath } from 'react-router-dom';
54

65
import { clonePageElement } from './clonePageElement';
6+
import { matchPath } from './utils/matchPath';
77

88
// TODO(FW-2959): types
99

@@ -433,12 +433,10 @@ export default StackManager;
433433
function matchRoute(node: React.ReactNode, routeInfo: RouteInfo) {
434434
let matchedNode: React.ReactNode;
435435
React.Children.forEach(node as React.ReactElement, (child: React.ReactElement) => {
436-
const matchProps = {
437-
exact: child.props.exact,
438-
path: child.props.path || child.props.from,
439-
component: child.props.component,
440-
};
441-
const match = matchPath(routeInfo.pathname, matchProps);
436+
const match = matchPath({
437+
pathname: routeInfo.pathname,
438+
componentProps: child.props,
439+
});
442440
if (match) {
443441
matchedNode = child;
444442
}
@@ -459,12 +457,11 @@ function matchRoute(node: React.ReactNode, routeInfo: RouteInfo) {
459457
}
460458

461459
function matchComponent(node: React.ReactElement, pathname: string, forceExact?: boolean) {
462-
const matchProps = {
463-
exact: forceExact ? true : node.props.exact,
464-
path: node.props.path || node.props.from,
465-
component: node.props.component,
466-
};
467-
const match = matchPath(pathname, matchProps);
468-
469-
return match;
460+
return matchPath({
461+
pathname,
462+
componentProps: {
463+
...node.props,
464+
exact: forceExact,
465+
},
466+
});
470467
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { matchPath as reactRouterMatchPath } from 'react-router';
2+
3+
interface MatchPathOptions {
4+
/**
5+
* The pathname to match against.
6+
*/
7+
pathname: string;
8+
/**
9+
* The props to match against, they are identical to the matching props `Route` accepts.
10+
*/
11+
componentProps: {
12+
path?: string;
13+
from?: string;
14+
component?: any;
15+
exact?: boolean;
16+
};
17+
}
18+
19+
/**
20+
* @see https://v5.reactrouter.com/web/api/matchPath
21+
*/
22+
export const matchPath = ({
23+
pathname,
24+
componentProps,
25+
}: MatchPathOptions): false | ReturnType<typeof reactRouterMatchPath> => {
26+
const { exact, component } = componentProps;
27+
28+
const path = componentProps.path || componentProps.from;
29+
/***
30+
* The props to match against, they are identical
31+
* to the matching props `Route` accepts. It could also be a string
32+
* or an array of strings as shortcut for `{ path }`.
33+
*/
34+
const matchProps = {
35+
exact,
36+
path,
37+
component,
38+
};
39+
40+
const match = reactRouterMatchPath(pathname, matchProps);
41+
42+
if (!match) {
43+
return false;
44+
}
45+
46+
return match;
47+
};

packages/react-router/test/base/src/pages/routing/Details.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,6 @@ const Details: React.FC<DetailsProps> = () => {
2424
return () => console.log('Home Details unmount');
2525
}, []);
2626

27-
// useIonViewWillEnter(() => {
28-
// console.log('IVWE Details')
29-
// })
30-
3127
const nextId = parseInt(id, 10) + 1;
3228

3329
return (
@@ -58,6 +54,9 @@ const Details: React.FC<DetailsProps> = () => {
5854
<IonButton routerLink={`/routing/tabs/settings/details/1`}>
5955
<IonLabel>Go to Settings Details 1</IonLabel>
6056
</IonButton>
57+
<br />
58+
<br />
59+
<input data-testid="details-input" />
6160
</IonContent>
6261
</IonPage>
6362
);

packages/react-router/test/base/tests/e2e/specs/routing.cy.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,41 @@ describe('Routing Tests', () => {
309309
cy.ionPageDoesNotExist('home-details-page-1');
310310
cy.ionPageVisible('home-page');
311311
});
312+
313+
it('should mount new view item instances of parameterized routes', () => {
314+
cy.visit(`http://localhost:${port}/routing/tabs/home/details/1`);
315+
316+
cy.get('div.ion-page[data-pageid=home-details-page-1]')
317+
.get('[data-testid="details-input"]')
318+
.should('have.value', '');
319+
320+
cy.get('div.ion-page[data-pageid=home-details-page-1] [data-testid="details-input"]').type('1');
321+
322+
cy.ionNav('ion-button', 'Go to Details 2');
323+
cy.ionPageVisible('home-details-page-2');
324+
325+
cy.get('div.ion-page[data-pageid=home-details-page-2] [data-testid="details-input"]').should('have.value', '');
326+
327+
cy.get('div.ion-page[data-pageid=home-details-page-2] [data-testid="details-input"]').type('2');
328+
329+
cy.ionNav('ion-button', 'Go to Details 3');
330+
cy.ionPageVisible('home-details-page-3');
331+
332+
cy.get('div.ion-page[data-pageid=home-details-page-3] [data-testid="details-input"]').should('have.value', '');
333+
334+
cy.get('div.ion-page[data-pageid=home-details-page-3] [data-testid="details-input"]').type('3');
335+
336+
cy.ionBackClick('home-details-page-3');
337+
cy.ionPageVisible('home-details-page-2');
338+
339+
cy.get('div.ion-page[data-pageid=home-details-page-2] [data-testid="details-input"]').should('have.value', '2');
340+
341+
cy.ionBackClick('home-details-page-2');
342+
cy.ionPageVisible('home-details-page-1');
343+
344+
cy.get('div.ion-page[data-pageid=home-details-page-1] [data-testid="details-input"]').should('have.value', '1');
345+
});
346+
312347
/*
313348
Tests to add:
314349
Test that lifecycle events fire

0 commit comments

Comments
 (0)