Skip to content

Commit fbed1e0

Browse files
committed
chore(react-router): fixing types, removing usued variable
1 parent 6575a92 commit fbed1e0

File tree

42 files changed

+166
-218
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+166
-218
lines changed

docs/react-router/react-router-6-status.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ This aligns with React Router 6's nested routing semantics where child routes ar
103103

104104
### Before Alpha Release
105105

106-
1. **Run a TypeScript strict check** on the `@ionic/react-router` package to catch any type regressions
106+
1. ~~**Run a TypeScript strict check** on the `@ionic/react-router` package~~ Complete - all type errors resolved
107107
2. **Manual testing pass** through the test app to verify animations and gestures feel correct
108108

109109
### Documentation

packages/react-router/package-lock.json

Lines changed: 38 additions & 58 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/react-router/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@
5252
"@types/node": "^14.0.14",
5353
"@types/react": "^17.0.79",
5454
"@types/react-dom": "^17.0.25",
55-
"@types/react-router": "^5.0.3",
56-
"@types/react-router-dom": "^5.1.5",
55+
"history": "^5.3.0",
5756
"@typescript-eslint/eslint-plugin": "^5.48.2",
5857
"@typescript-eslint/parser": "^5.48.2",
5958
"eslint": "^7.32.0",

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

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@
88

99
import type { AnimationBuilder, RouteAction, RouteInfo, RouteManagerContextState, RouterDirection } from '@ionic/react';
1010
import { LocationHistory, NavManager, RouteManagerContext, generateId, getConfig } from '@ionic/react';
11-
import type { Action as HistoryAction, Location as HistoryLocation } from 'history';
11+
import type { Action as HistoryAction, Location } from 'history';
12+
13+
// Use Location directly - state is typed as `unknown` in history v5
14+
type HistoryLocation = Location;
1215
import type { PropsWithChildren } from 'react';
1316
import React, { useEffect, useRef, useState } from 'react';
1417
import { useLocation, useNavigate } from 'react-router-dom';
@@ -23,10 +26,22 @@ export interface LocationState {
2326
}
2427

2528
interface IonRouterProps {
26-
registerHistoryListener: (cb: (location: HistoryLocation<any>, action: HistoryAction) => void) => void;
29+
registerHistoryListener: (cb: (location: HistoryLocation, action: HistoryAction) => void) => void;
2730
}
2831

2932
type RouteParams = Record<string, string | string[] | undefined>;
33+
type SafeRouteParams = Record<string, string | string[]>;
34+
35+
const filterUndefinedParams = (params: RouteParams): SafeRouteParams => {
36+
const result: SafeRouteParams = {};
37+
for (const key of Object.keys(params)) {
38+
const value = params[key];
39+
if (value !== undefined) {
40+
result[key] = value;
41+
}
42+
}
43+
return result;
44+
};
3045

3146
const areParamsEqual = (a?: RouteParams, b?: RouteParams) => {
3247
const paramsA = a || {};
@@ -61,7 +76,7 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
6176
const viewStack = useRef(new ReactRouterViewStack());
6277
const incomingRouteParams = useRef<Partial<RouteInfo> | null>(null);
6378

64-
const [routeInfo, setRouteInfo] = useState({
79+
const [routeInfo, setRouteInfo] = useState<RouteInfo>({
6580
id: generateId('routeInfo'),
6681
pathname: location.pathname,
6782
search: location.search,
@@ -87,7 +102,7 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
87102
const matchedParams = activeView?.routeData.match?.params as RouteParams | undefined;
88103

89104
if (matchedParams) {
90-
const paramsCopy = { ...matchedParams };
105+
const paramsCopy = filterUndefinedParams({ ...matchedParams });
91106
if (areParamsEqual(routeInfo.params as RouteParams | undefined, paramsCopy)) {
92107
return;
93108
}
@@ -110,7 +125,7 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
110125
* @param location The current location object from the history.
111126
* @param action The action that triggered the history change.
112127
*/
113-
const handleHistoryChange = (location: HistoryLocation<LocationState>, action: HistoryAction) => {
128+
const handleHistoryChange = (location: HistoryLocation, action: HistoryAction) => {
114129
let leavingLocationInfo: RouteInfo;
115130
/**
116131
* A programmatic navigation was triggered.
@@ -188,10 +203,11 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
188203
}
189204
// Still found no params, set it to a default state of forward.
190205
if (!incomingRouteParams.current) {
206+
const state = location.state as LocationState | null;
191207
incomingRouteParams.current = {
192208
routeAction: 'push',
193-
routeDirection: location.state?.direction || 'forward',
194-
routeOptions: location.state?.routerOptions,
209+
routeDirection: state?.direction || 'forward',
210+
routeOptions: state?.routerOptions,
195211
tab: tabToUse,
196212
};
197213
}
@@ -228,7 +244,9 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
228244
lastPathname: leavingLocationInfo.pathname, // The URL we just came from
229245
pathname: location.pathname, // The current (destination) URL
230246
search: location.search,
231-
params: (incomingRouteParams.current?.params as RouteParams | undefined) ?? {},
247+
params: incomingRouteParams.current?.params
248+
? filterUndefinedParams(incomingRouteParams.current.params as RouteParams)
249+
: {},
232250
prevRouteLastPathname: leavingLocationInfo.lastPathname, // The lastPathname of the route we are leaving
233251
};
234252
// It's a linear navigation.

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

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -14,44 +14,6 @@ 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-
28-
/**
29-
* Checks if a route matches the remaining path.
30-
* Note: This function is used for checking if ANY route could match, not for determining priority.
31-
* Wildcard routes are handled specially - they're always considered potential matches but should
32-
* be used as fallbacks when no specific route matches.
33-
*/
34-
const doesRouteMatchRemainingPath = (route: React.ReactElement, remainingPath: string) => {
35-
const routePath = route.props.path;
36-
const isWildcardOnly = routePath === '*' || routePath === '/*';
37-
const isIndex = route.props.index;
38-
39-
// Index routes only match when remaining path is empty
40-
if (isIndex) {
41-
return remainingPath === '' || remainingPath === '/';
42-
}
43-
44-
// Wildcard routes can match any path (used as fallback)
45-
if (isWildcardOnly) {
46-
return true; // Wildcards can always potentially match
47-
}
48-
49-
return !!matchPath({
50-
pathname: remainingPath,
51-
componentProps: route.props,
52-
});
53-
};
54-
5517
/**
5618
* Checks if a route is a specific match (not wildcard or index).
5719
*/
@@ -650,9 +612,12 @@ export class StackManager extends React.PureComponent<StackManagerProps, StackMa
650612
// Skip removal only for container-to-container transitions (nested outlet redirects)
651613
// Remove the leaving view if it's a specific route being replaced
652614
if (!(isEnteringContainerRoute && !isLeavingSpecificRoute)) {
615+
// Capture leavingViewItem for the closure since TypeScript can't
616+
// track the outer if-block's null check through the setTimeout
617+
const viewToUnmount = leavingViewItem;
653618
setTimeout(() => {
654619
// Use a timeout to ensure the transition completes before removal
655-
this.context.unMountViewItem(leavingViewItem);
620+
this.context.unMountViewItem(viewToUnmount);
656621
}, 250);
657622
}
658623
}

packages/react-router/src/ReactRouter/utils/matchRoutesFromChildren.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import React from 'react';
2-
import type { RouteObject, AgnosticRouteMatch } from 'react-router';
2+
import type { RouteObject } from 'react-router';
33
import { matchRoutes } from 'react-router-dom';
44
import { Route } from 'react-router-dom';
55

6+
// Type for the result of matchRoutes - inferred from the function return type
7+
type RouteMatch = NonNullable<ReturnType<typeof matchRoutes>>[number];
8+
69
/**
710
* Sorts routes by specificity. React Router's matchRoutes respects route order,
811
* so we need to ensure more specific routes come before wildcards.
@@ -98,7 +101,7 @@ export function createRouteObjectsFromChildren(children: React.ReactNode): Route
98101
export function findMatchingRoutes(
99102
children: React.ReactNode,
100103
pathname: string
101-
): AgnosticRouteMatch<string, RouteObject>[] | null {
104+
): RouteMatch[] | null {
102105
const routes = createRouteObjectsFromChildren(children);
103106
return matchRoutes(routes, pathname);
104107
}

packages/react-router/test/apps/reactrouter6/package.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22
"name": "react-router-new",
33
"version": "0.0.1",
44
"private": true,
5+
"overrides": {
6+
"@ionic/react-router": {
7+
"react-router": "$react-router",
8+
"react-router-dom": "$react-router-dom"
9+
}
10+
},
511
"dependencies": {
612
"@ionic/react": "^8.6.1",
713
"@ionic/react-router": "^8.6.1",

packages/react-router/test/base/src/App.test.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import React from 'react';
21
import { render } from '@testing-library/react';
2+
import React from 'react';
3+
34
import App from './App';
45

56
test('renders without crashing', () => {

0 commit comments

Comments
 (0)