Skip to content

Commit 397b6f7

Browse files
committed
chore(react-router): code clean up
1 parent 5a77916 commit 397b6f7

File tree

3 files changed

+7
-118
lines changed

3 files changed

+7
-118
lines changed

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -153,19 +153,16 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
153153
}
154154

155155
const leavingUrl = leavingLocationInfo.pathname + leavingLocationInfo.search;
156-
// Check if the URL has changed.
157156
if (leavingUrl !== location.pathname) {
158-
// An external navigation was triggered.
159157
if (!incomingRouteParams.current) {
160158
// Determine if the destination is a tab route by checking if it matches
161159
// the pattern of tab routes (containing /tabs/ in the path)
162160
const isTabRoute = /\/tabs(\/|$)/.test(location.pathname);
163-
let tabToUse = isTabRoute ? currentTab.current : undefined;
161+
const tabToUse = isTabRoute ? currentTab.current : undefined;
164162

165163
// If we're leaving tabs entirely, clear the current tab
166164
if (!isTabRoute && currentTab.current) {
167165
currentTab.current = undefined;
168-
tabToUse = undefined;
169166
}
170167

171168
/**
@@ -201,7 +198,6 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
201198
};
202199
}
203200
}
204-
// Still found no params, set it to a default state of forward.
205201
if (!incomingRouteParams.current) {
206202
const state = location.state as LocationState | null;
207203
incomingRouteParams.current = {
@@ -247,9 +243,8 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
247243
params: incomingRouteParams.current?.params
248244
? filterUndefinedParams(incomingRouteParams.current.params as RouteParams)
249245
: {},
250-
prevRouteLastPathname: leavingLocationInfo.lastPathname, // The lastPathname of the route we are leaving
246+
prevRouteLastPathname: leavingLocationInfo.lastPathname,
251247
};
252-
// It's a linear navigation.
253248
if (isPushed) {
254249
// Only inherit tab from leaving route if we don't already have one.
255250
// This preserves tab context for same-tab navigation while allowing cross-tab navigation.
@@ -312,7 +307,6 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
312307
setRouteInfo(routeInfo);
313308
}
314309

315-
// Reset for the next navigation.
316310
incomingRouteParams.current = null;
317311
};
318312

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

Lines changed: 3 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,6 @@ const resolveIndexRouteMatch = (
102102
};
103103

104104
export class ReactRouterViewStack extends ViewStacks {
105-
private pendingViewItems: Map<string, ViewItem> = new Map();
106-
private deactivationQueue: Map<string, NodeJS.Timeout> = new Map();
107105
private viewItemCounter = 0;
108106

109107
constructor() {
@@ -182,21 +180,9 @@ export class ReactRouterViewStack extends ViewStacks {
182180
return existingViewItem;
183181
}
184182

185-
// Create a truly unique ID by combining outlet ID with an incrementing counter
186183
this.viewItemCounter++;
187184
const id = `${outletId}-${this.viewItemCounter}`;
188185

189-
// Add infinite loop detection with a more reasonable limit
190-
// In complex navigation flows, we may have many view items across different outlets
191-
if (this.viewItemCounter > 100) {
192-
// Clean up all outlets to prevent memory leaks
193-
this.getStackIds().forEach((stackId) => this.cleanupStaleViewItems(stackId));
194-
// Reset counter to a lower value after cleanup
195-
if (this.viewItemCounter > 100) {
196-
this.viewItemCounter = 50;
197-
}
198-
}
199-
200186
const viewItem: ViewItem = {
201187
id,
202188
outletId,
@@ -219,9 +205,7 @@ export class ReactRouterViewStack extends ViewStacks {
219205
childProps: reactElement.props,
220206
};
221207

222-
// Store in pending until properly added
223-
const key = `${outletId}-${routeInfo.pathname}`;
224-
this.pendingViewItems.set(key, viewItem);
208+
this.add(viewItem);
225209

226210
return viewItem;
227211
};
@@ -254,14 +238,6 @@ export class ReactRouterViewStack extends ViewStacks {
254238
// Flag to indicate this view should not be reused for this different parameterized path
255239
const shouldSkipForDifferentParam = isParameterRoute && match && previousMatch && !isSamePath;
256240

257-
// Cancel any pending deactivation if we have a match
258-
if (match) {
259-
const timeoutId = this.deactivationQueue.get(viewItem.id);
260-
if (timeoutId) {
261-
clearTimeout(timeoutId);
262-
this.deactivationQueue.delete(viewItem.id);
263-
}
264-
}
265241

266242
// Don't deactivate views automatically - let the StackManager handle view lifecycle
267243
// This preserves views in the stack for navigation history like native apps
@@ -342,9 +318,7 @@ export class ReactRouterViewStack extends ViewStacks {
342318

343319
// Check if this view item would match the current route
344320
const vMatch = v.reactElement ? matchComponent(v.reactElement, routeInfo.pathname) : null;
345-
const hasMatch = !!vMatch;
346-
347-
return hasMatch;
321+
return !!vMatch;
348322
});
349323

350324
if (hasSpecificMatch) {
@@ -655,7 +629,6 @@ export class ReactRouterViewStack extends ViewStacks {
655629
return true;
656630
});
657631

658-
// Render all view items using renderViewItem
659632
const renderedItems = renderableViewItems.map((viewItem) => this.renderViewItem(viewItem, routeInfo, parentPath));
660633
return renderedItems;
661634
};
@@ -702,25 +675,6 @@ export class ReactRouterViewStack extends ViewStacks {
702675
let match: PathMatch<string> | null = null;
703676
let viewStack: ViewItem[];
704677

705-
// First check pending items
706-
if (outletId) {
707-
const pendingKey = `${outletId}-${pathname}`;
708-
const pendingItem = this.pendingViewItems.get(pendingKey);
709-
if (pendingItem) {
710-
// Move from pending to active
711-
this.pendingViewItems.delete(pendingKey);
712-
this.add(pendingItem);
713-
return { viewItem: pendingItem, match: pendingItem.routeData.match };
714-
}
715-
716-
// Fallback: If we did not find a pending item for this outlet, look for
717-
// a pending view item created under a different outlet for the same pathname
718-
// and adopt it into the current outlet. This can happen if an outlet remounts
719-
// (e.g., during browser forward navigation) and gets a new generated id.
720-
// Disable cross-outlet adoption for now; it can cause mismatches where
721-
// views are moved between outlets with different routing scopes.
722-
}
723-
724678
// Helper function to sort views by specificity (most specific first)
725679
const sortBySpecificity = (views: ViewItem[]) => {
726680
return [...views].sort((a, b) => {
@@ -885,19 +839,13 @@ export class ReactRouterViewStack extends ViewStacks {
885839

886840
super.add(viewItem);
887841

888-
// Clean up stale view items after adding new ones
889842
this.cleanupStaleViewItems(viewItem.outletId);
890843
};
891844

892845
/**
893-
* Override remove to clear any pending deactivations
846+
* Override remove
894847
*/
895848
remove = (viewItem: ViewItem) => {
896-
const timeoutId = this.deactivationQueue.get(viewItem.id);
897-
if (timeoutId) {
898-
clearTimeout(timeoutId);
899-
this.deactivationQueue.delete(viewItem.id);
900-
}
901849
super.remove(viewItem);
902850
};
903851
}

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

Lines changed: 2 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
113113
this.registerIonPage = this.registerIonPage.bind(this);
114114
this.transitionPage = this.transitionPage.bind(this);
115115
this.handlePageTransition = this.handlePageTransition.bind(this);
116-
// Use provided id prop if available; otherwise generate a unique id.
117116
this.id = props.id || `routerOutlet-${generateId('routerOutlet')}`;
118117
this.prevProps = undefined;
119118
this.skipTransition = false;
@@ -436,13 +435,6 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
436435
return false;
437436
}
438437

439-
// routeOptions.unmount should NOT trigger view removal - it just hides the view
440-
// The view needs to remain in the stack for tab navigation and back navigation
441-
// Don't set mount = false, just let the transition hide the page
442-
// if (routeInfo.routeOptions?.unmount) {
443-
// return true;
444-
// }
445-
446438
if (routeInfo.routeAction === 'replace') {
447439
return true;
448440
}
@@ -799,16 +791,12 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
799791
leavingViewItem.ionPageElement.classList.add('ion-page-hidden');
800792
leavingViewItem.ionPageElement.setAttribute('aria-hidden', 'true');
801793
}
802-
if (shouldUnmountLeavingViewItem && leavingViewItem) {
794+
if (shouldUnmountLeavingViewItem) {
803795
leavingViewItem.mount = false;
804796
}
805-
} else {
806-
// No entering or leaving view - this might be a routing issue
807-
// Don't retry endlessly to avoid infinite loops
808797
}
809798
}
810799

811-
// Force re-render so views update according to their new mount/visible status
812800
this.forceUpdate();
813801
}
814802
}
@@ -1054,11 +1042,8 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
10541042
await runCommit(enteringViewItem.ionPageElement, undefined);
10551043
}
10561044
} else {
1057-
// The leaving view is not the same as the entering view
1058-
// (e.g., `/home` → `/settings` or initial load `/`)
10591045
await runCommit(enteringViewItem.ionPageElement, leavingViewItem?.ionPageElement);
10601046
if (leavingViewItem && leavingViewItem.ionPageElement && !progressAnimation) {
1061-
// An initiial load will not have a leaving view.
10621047
leavingViewItem.ionPageElement.classList.add('ion-page-hidden');
10631048
leavingViewItem.ionPageElement.setAttribute('aria-hidden', 'true');
10641049
}
@@ -1213,7 +1198,7 @@ function findRouteByRouteInfo(node: React.ReactNode, routeInfo: RouteInfo, paren
12131198
if (absolutePathRoutes.length > 0) {
12141199
// Find common prefix of all absolute paths to determine outlet scope
12151200
const absolutePaths = absolutePathRoutes.map((r) => r.props.path as string);
1216-
const commonPrefix = findCommonPrefix(absolutePaths);
1201+
const commonPrefix = computeCommonPrefix(absolutePaths);
12171202

12181203
// If we have a common prefix, check if the current pathname is within that scope
12191204
if (commonPrefix && commonPrefix !== '/') {
@@ -1234,44 +1219,6 @@ function findRouteByRouteInfo(node: React.ReactNode, routeInfo: RouteInfo, paren
12341219
return matchedNode ?? fallbackNode;
12351220
}
12361221

1237-
/**
1238-
* Finds the longest common prefix among an array of paths.
1239-
* Used to determine the scope of an outlet with absolute routes.
1240-
*/
1241-
function findCommonPrefix(paths: string[]): string {
1242-
if (paths.length === 0) return '';
1243-
if (paths.length === 1) {
1244-
// For a single path, extract the directory-like prefix
1245-
// e.g., /dynamic-routes/home -> /dynamic-routes
1246-
const segments = paths[0].split('/').filter(Boolean);
1247-
if (segments.length > 1) {
1248-
return '/' + segments.slice(0, -1).join('/');
1249-
}
1250-
return '/' + segments[0];
1251-
}
1252-
1253-
// Split all paths into segments
1254-
const segmentArrays = paths.map((p) => p.split('/').filter(Boolean));
1255-
const minLength = Math.min(...segmentArrays.map((s) => s.length));
1256-
1257-
const commonSegments: string[] = [];
1258-
for (let i = 0; i < minLength; i++) {
1259-
const segment = segmentArrays[0][i];
1260-
// Skip segments with route parameters or wildcards
1261-
if (segment.includes(':') || segment.includes('*')) {
1262-
break;
1263-
}
1264-
const allMatch = segmentArrays.every((s) => s[i] === segment);
1265-
if (allMatch) {
1266-
commonSegments.push(segment);
1267-
} else {
1268-
break;
1269-
}
1270-
}
1271-
1272-
return commonSegments.length > 0 ? '/' + commonSegments.join('/') : '';
1273-
}
1274-
12751222
function matchComponent(node: React.ReactElement, pathname: string, forceExact?: boolean) {
12761223
const routePath: string | undefined = node?.props?.path;
12771224
const pathnameToMatch = derivePathnameToMatch(pathname, routePath);

0 commit comments

Comments
 (0)