Skip to content

Commit 7fd0659

Browse files
committed
fix(react-router): nested redirect fix
1 parent b02c197 commit 7fd0659

File tree

3 files changed

+71
-3
lines changed

3 files changed

+71
-3
lines changed

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,12 @@ export class ReactRouterViewStack extends ViewStacks {
275275
if (isNavigateComponent) {
276276
// Navigate components should only be mounted when they match
277277
// Once they redirect (no longer match), they should be removed completely
278-
if (!match && viewItem.mount) {
278+
// IMPORTANT: For index routes, we need to check indexMatch too since matchComponent
279+
// may not properly match index routes without explicit parent path context
280+
const indexMatch = viewItem.routeData?.childProps?.index ? resolveIndexRouteMatch(viewItem, routeInfo.pathname, parentPath) : null;
281+
const hasValidMatch = match || indexMatch;
282+
283+
if (!hasValidMatch && viewItem.mount) {
279284
viewItem.mount = false;
280285
// Schedule removal of the Navigate view item after a short delay
281286
// This ensures the redirect completes before removal
@@ -288,7 +293,9 @@ export class ReactRouterViewStack extends ViewStacks {
288293
// Components that don't have IonPage elements and no longer match should be cleaned up
289294
// BUT we need to be careful not to remove them if they're part of browser navigation history
290295
// This handles components that perform immediate actions like programmatic navigation
291-
if (!match && viewItem.mount && !viewItem.ionPageElement) {
296+
// EXCEPTION: Navigate components should ALWAYS remain mounted until they redirect
297+
// since they need to be rendered to trigger the navigation
298+
if (!match && viewItem.mount && !viewItem.ionPageElement && !isNavigateComponent) {
292299
// Check if this view item should be preserved for browser navigation
293300
// We'll keep it if it was recently active (within the last navigation)
294301
const shouldPreserve =

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

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,17 @@ import { findRoutesNode } from './utils/findRoutesNode';
1414
import { derivePathnameToMatch } from './utils/derivePathnameToMatch';
1515
import { matchPath } from './utils/matchPath';
1616

17+
// Debug helper to check if a view item contains a Navigate component
18+
const isNavigateViewItem = (viewItem: ViewItem | undefined): boolean => {
19+
if (!viewItem) return false;
20+
const elementComponent = viewItem.reactElement?.props?.element;
21+
return (
22+
React.isValidElement(elementComponent) &&
23+
(elementComponent.type === Navigate ||
24+
(typeof elementComponent.type === 'function' && elementComponent.type.name === 'Navigate'))
25+
);
26+
};
27+
1728
/**
1829
* Checks if a route matches the remaining path.
1930
* Note: This function is used for checking if ANY route could match, not for determining priority.
@@ -320,11 +331,18 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
320331
* Set a flag to indicate that we should transition the page after
321332
* the component has updated (i.e., in `componentDidUpdate`).
322333
*/
334+
if (process.env.NODE_ENV !== 'production') {
335+
console.log(`[StackManager] Outlet ${this.id} not ready yet, setting pendingPageTransition=true for ${routeInfo.pathname}`);
336+
}
323337
this.pendingPageTransition = true;
324338
} else {
325339
let enteringViewItem = this.context.findViewItemByRouteInfo(routeInfo, this.id);
326340
let leavingViewItem = this.context.findLeavingViewItemByRouteInfo(routeInfo, this.id);
327341

342+
if (process.env.NODE_ENV !== 'production') {
343+
console.log(`[StackManager:handlePageTransition] outlet=${this.id}, pathname=${routeInfo.pathname}, entering=${enteringViewItem?.id}, leaving=${leavingViewItem?.id}, enteringIsNav=${isNavigateViewItem(enteringViewItem)}, leavingIsNav=${isNavigateViewItem(leavingViewItem)}`);
344+
}
345+
328346
/**
329347
* If we don't have a leaving view item, but the route info indicates
330348
* that the user has routed from a previous path, then the leaving view
@@ -425,7 +443,13 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
425443
: [];
426444

427445
// Unmount and remove all views in this outlet immediately to avoid leftover content
446+
if (process.env.NODE_ENV !== 'production') {
447+
console.log(`[StackManager:outOfScope] outlet=${this.id} is out of scope, removing ${allViewsInOutlet.length} views`);
448+
}
428449
allViewsInOutlet.forEach((viewItem) => {
450+
if (process.env.NODE_ENV !== 'production') {
451+
console.log(`[StackManager:outOfScope] Removing view ${viewItem.id} from outlet ${this.id}, isNavigate=${isNavigateViewItem(viewItem)}`);
452+
}
429453
if (viewItem.ionPageElement) {
430454
viewItem.ionPageElement.classList.add('ion-page-hidden');
431455
viewItem.ionPageElement.setAttribute('aria-hidden', 'true');
@@ -466,6 +490,9 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
466490
leavingViewItem.ionPageElement.setAttribute('aria-hidden', 'true');
467491
}
468492
if (leavingViewItem) {
493+
if (isNavigateViewItem(leavingViewItem) && process.env.NODE_ENV !== 'production') {
494+
console.log(`[StackManager:hasRelativeRoutes] Setting mount=false on Navigate ${leavingViewItem.id}, outlet=${this.id}, pathname=${routeInfo.pathname}`);
495+
}
469496
leavingViewItem.mount = false;
470497
}
471498
this.forceUpdate();
@@ -492,13 +519,19 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
492519
// If this is a nested outlet (has an explicit ID) and no route matches,
493520
// it means this outlet shouldn't handle this route
494521
if (this.id !== 'routerOutlet' && !enteringRoute && !enteringViewItem) {
522+
if (process.env.NODE_ENV !== 'production') {
523+
console.log(`[StackManager:noMatchingRoute] outlet=${this.id} has no matching route for ${routeInfo.pathname}, leavingViewItem=${leavingViewItem?.id}`);
524+
}
495525
// Hide any visible views in this outlet since it has no matching route
496526
if (leavingViewItem && leavingViewItem.ionPageElement) {
497527
leavingViewItem.ionPageElement.classList.add('ion-page-hidden');
498528
leavingViewItem.ionPageElement.setAttribute('aria-hidden', 'true');
499529
}
500530
// Unmount the leaving view to prevent components from staying active
501531
if (leavingViewItem) {
532+
if (isNavigateViewItem(leavingViewItem) && process.env.NODE_ENV !== 'production') {
533+
console.log(`[StackManager:noMatchingRoute] Setting mount=false on Navigate ${leavingViewItem.id}, outlet=${this.id}, pathname=${routeInfo.pathname}`);
534+
}
502535
leavingViewItem.mount = false;
503536
}
504537
this.forceUpdate();
@@ -512,9 +545,18 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
512545
if (enteringViewItem && enteringRoute) {
513546
// Update existing view item
514547
enteringViewItem.reactElement = enteringRoute;
548+
if (process.env.NODE_ENV !== 'production') {
549+
console.log(`[StackManager] Updated existing view item ${enteringViewItem.id} for outlet ${this.id}`);
550+
}
515551
} else if (enteringRoute) {
552+
if (process.env.NODE_ENV !== 'production') {
553+
console.log(`[StackManager] Creating new view item for outlet ${this.id}, route path="${enteringRoute.props?.path ?? '(index)'}"`);
554+
}
516555
enteringViewItem = this.context.createViewItem(this.id, enteringRoute, routeInfo);
517556
this.context.addViewItem(enteringViewItem);
557+
if (process.env.NODE_ENV !== 'production') {
558+
console.log(`[StackManager] Added view item ${enteringViewItem.id} to outlet ${this.id}`);
559+
}
518560
}
519561

520562
/**
@@ -674,6 +716,9 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
674716
* and repeatedly hide the leaving view. Treat this as a no-op transition and allow
675717
* the follow-up navigation to proceed.
676718
*/
719+
if (process.env.NODE_ENV !== 'production') {
720+
console.log(`[StackManager:Navigate] outlet=${this.id}, entering=${enteringViewItem?.id}, leaving=${leavingViewItem?.id}, shouldUnmount=${shouldUnmountLeavingViewItem}, pathname=${routeInfo.pathname}`);
721+
}
677722
this.waitingForIonPage = false;
678723
if (this.ionPageWaitTimeout) {
679724
clearTimeout(this.ionPageWaitTimeout);
@@ -686,7 +731,13 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
686731
leavingViewItem.ionPageElement.classList.add('ion-page-hidden');
687732
leavingViewItem.ionPageElement.setAttribute('aria-hidden', 'true');
688733
}
689-
if (shouldUnmountLeavingViewItem && leavingViewItem) {
734+
// IMPORTANT: Don't unmount if entering and leaving are the same view item
735+
// This happens during chained Navigate redirects where the same Navigate view item
736+
// is being processed multiple times before it can render and trigger the redirect
737+
if (shouldUnmountLeavingViewItem && leavingViewItem && enteringViewItem !== leavingViewItem) {
738+
if (isNavigateViewItem(leavingViewItem) && process.env.NODE_ENV !== 'production') {
739+
console.log(`[StackManager:Navigate:unmountLeaving] Setting mount=false on Navigate ${leavingViewItem.id}, outlet=${this.id}`);
740+
}
690741
leavingViewItem.mount = false;
691742
}
692743

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,16 @@ describe('Routing Tests', () => {
345345
cy.get('div.ion-page[data-pageid=home-details-page-1] [data-testid="details-input"]').should('have.value', '1');
346346
});
347347

348+
it('should complete chained Navigate redirects from root to /routing/tabs/home', () => {
349+
// Tests that chained Navigate redirects work correctly:
350+
// / > click Routing link > /routing (Navigate to tabs) > /routing/tabs (Navigate to home) > /routing/tabs/home
351+
// This was a bug where the second Navigate would be unmounted before it could trigger
352+
cy.visit(`http://localhost:${port}/`);
353+
cy.ionNav('ion-item', 'Routing');
354+
cy.ionPageVisible('home-page');
355+
cy.url().should('include', '/routing/tabs/home');
356+
});
357+
348358
/*
349359
Tests to add:
350360
Test that lifecycle events fire

0 commit comments

Comments
 (0)