Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add generics for Remix type enhancements #10843

Merged
merged 2 commits into from Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/align-types.md
@@ -0,0 +1,11 @@
---
"react-router-dom": patch
"react-router": patch
"@remix-run/router": patch
---

In order to move towards stricter TypeScript support in the future, we're aiming to replace current usages of `any` with `unknown` on exposed typings for user-provided data. To do this in Remix v2 without introducing breaking changes in React Router v6, we have added generics to a number of shared types. These continue to default to `any` in React Router and are overridden with `unknown` in Remix. In React Router v7 we plan to move these to `unknown` as a breakjing change.

- `Location` now accepts a generic for the `location.state` value
- `ActionFunctionArgs`/`ActionFunction`/`LoaderFunctionArgs`/`LoaderFunction` now accept a generic for the `context` parameter (only used in SSR usages via `createStaticHandler`)
- The return type of `useMatches` (now exported as `UIMatch`) accepts generics for `match.data` and `match.handle` - both of which were already set to `unknown`
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -110,7 +110,7 @@
},
"filesize": {
"packages/router/dist/router.umd.min.js": {
"none": "47.2 kB"
"none": "47.3 kB"
},
"packages/react-router/dist/react-router.production.min.js": {
"none": "13.9 kB"
Expand Down
1 change: 1 addition & 0 deletions packages/react-router-dom-v5-compat/index.ts
Expand Up @@ -104,6 +104,7 @@ export type {
SubmitOptions,
To,
URLSearchParamsInit,
UIMatch,
unstable_Blocker,
unstable_BlockerFunction,
} from "./react-router-dom";
Expand Down
3 changes: 3 additions & 0 deletions packages/react-router-dom/index.tsx
Expand Up @@ -130,6 +130,7 @@ export type {
ShouldRevalidateFunction,
ShouldRevalidateFunctionArgs,
To,
UIMatch,
} from "react-router";
export {
AbortedDeferredError,
Expand Down Expand Up @@ -1218,6 +1219,8 @@ export type FetcherWithComponents<TData> = Fetcher<TData> & {
load: (href: string) => void;
};

// TODO: (v7) Change the useFetcher generic default from `any` to `unknown`

/**
* Interacts with route loaders and actions without causing a navigation. Great
* for any interaction that stays on the same page.
Expand Down
1 change: 1 addition & 0 deletions packages/react-router-native/index.tsx
Expand Up @@ -65,6 +65,7 @@ export type {
ShouldRevalidateFunction,
ShouldRevalidateFunctionArgs,
To,
UIMatch,
} from "react-router";
export {
AbortedDeferredError,
Expand Down
2 changes: 2 additions & 0 deletions packages/react-router/index.ts
Expand Up @@ -26,6 +26,7 @@ import type {
ShouldRevalidateFunction,
ShouldRevalidateFunctionArgs,
To,
UIMatch,
} from "@remix-run/router";
import {
AbortedDeferredError,
Expand Down Expand Up @@ -167,6 +168,7 @@ export type {
ShouldRevalidateFunction,
ShouldRevalidateFunctionArgs,
To,
UIMatch,
Blocker as unstable_Blocker,
BlockerFunction as unstable_BlockerFunction,
};
Expand Down
19 changes: 4 additions & 15 deletions packages/react-router/lib/hooks.tsx
Expand Up @@ -12,10 +12,12 @@ import type {
Router as RemixRouter,
RevalidationState,
To,
UIMatch,
} from "@remix-run/router";
import {
IDLE_BLOCKER,
Action as NavigationType,
UNSAFE_convertRouteMatchToUiMatch as convertRouteMatchToUiMatch,
UNSAFE_getPathContributingMatches as getPathContributingMatches,
UNSAFE_invariant as invariant,
isRouteErrorResponse,
Expand Down Expand Up @@ -834,25 +836,12 @@ export function useRevalidator() {
* Returns the active route matches, useful for accessing loaderData for
* parent/child routes or the route "handle" property
*/
export function useMatches() {
export function useMatches(): UIMatch[] {
let { matches, loaderData } = useDataRouterState(
DataRouterStateHook.UseMatches
);
return React.useMemo(
() =>
matches.map((match) => {
let { pathname, params } = match;
// Note: This structure matches that created by createUseMatchesMatch
// in the @remix-run/router , so if you change this please also change
// that :) Eventually we'll DRY this up
return {
id: match.route.id,
pathname,
params,
data: loaderData[match.route.id] as unknown,
handle: match.route.handle as unknown,
};
}),
() => matches.map((m) => convertRouteMatchToUiMatch(m, loaderData)),
[matches, loaderData]
);
}
Expand Down
7 changes: 5 additions & 2 deletions packages/router/history.ts
Expand Up @@ -49,15 +49,18 @@ export interface Path {
hash: string;
}

// TODO: (v7) Change the Location generic default from `any` to `unknown` and
// remove Remix `useLocation` wrapper.

/**
* An entry in a history stack. A location contains information about the
* URL path, as well as possibly some arbitrary state and a key.
*/
export interface Location extends Path {
export interface Location<S = any> extends Path {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface Location<S = any> extends Path {
export interface Location<State = any> extends Path {

/**
* A value of arbitrary data associated with this location.
*/
state: any;
state: S;

/**
* A unique string associated with this location. May be used to safely store
Expand Down
2 changes: 2 additions & 0 deletions packages/router/index.ts
Expand Up @@ -25,6 +25,7 @@ export type {
ShouldRevalidateFunction,
ShouldRevalidateFunctionArgs,
TrackedPromise,
UIMatch,
V7_FormMethod,
} from "./utils";

Expand Down Expand Up @@ -84,6 +85,7 @@ export {
DeferredData as UNSAFE_DeferredData,
ErrorResponseImpl as UNSAFE_ErrorResponseImpl,
convertRoutesToDataRoutes as UNSAFE_convertRoutesToDataRoutes,
convertRouteMatchToUiMatch as UNSAFE_convertRouteMatchToUiMatch,
getPathContributingMatches as UNSAFE_getPathContributingMatches,
} from "./utils";

Expand Down
31 changes: 4 additions & 27 deletions packages/router/router.ts
Expand Up @@ -11,7 +11,6 @@ import type {
ActionFunction,
AgnosticDataRouteMatch,
AgnosticDataRouteObject,
AgnosticRouteMatch,
AgnosticRouteObject,
DataResult,
DeferredData,
Expand All @@ -31,12 +30,14 @@ import type {
ShouldRevalidateFunctionArgs,
Submission,
SuccessResult,
UIMatch,
V7_FormMethod,
V7_MutationFormMethod,
} from "./utils";
import {
ErrorResponseImpl,
ResultType,
convertRouteMatchToUiMatch,
convertRoutesToDataRoutes,
getPathContributingMatches,
immutableRouteKeys,
Expand Down Expand Up @@ -394,20 +395,12 @@ export interface RouterSubscriber {
(state: RouterState): void;
}

interface UseMatchesMatch {
id: string;
pathname: string;
params: AgnosticRouteMatch["params"];
data: unknown;
handle: unknown;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now exported from utils.ts as UIMatch


/**
* Function signature for determining the key to be used in scroll restoration
* for a given location
*/
export interface GetScrollRestorationKeyFunction {
(location: Location, matches: UseMatchesMatch[]): string | null;
(location: Location, matches: UIMatch[]): string | null;
}

/**
Expand Down Expand Up @@ -2461,7 +2454,7 @@ export function createRouter(init: RouterInit): Router {
if (getScrollRestorationKey) {
let key = getScrollRestorationKey(
location,
matches.map((m) => createUseMatchesMatch(m, state.loaderData))
matches.map((m) => convertRouteMatchToUiMatch(m, state.loaderData))
);
return key || location.key;
}
Expand Down Expand Up @@ -4332,22 +4325,6 @@ function hasNakedIndexQuery(search: string): boolean {
return new URLSearchParams(search).getAll("index").some((v) => v === "");
}

// Note: This should match the format exported by useMatches, so if you change
// this please also change that :) Eventually we'll DRY this up
function createUseMatchesMatch(
match: AgnosticDataRouteMatch,
loaderData: RouteData
): UseMatchesMatch {
let { route, pathname, params } = match;
return {
id: route.id,
pathname,
params,
data: loaderData[route.id] as unknown,
handle: route.handle as unknown,
};
}

function getTargetMatch(
matches: AgnosticDataRouteMatch[],
location: Location | string
Expand Down
41 changes: 33 additions & 8 deletions packages/router/utils.ts
Expand Up @@ -137,21 +137,24 @@ export type Submission =
* Arguments passed to route loader/action functions. Same for now but we keep
* this as a private implementation detail in case they diverge in the future.
*/
interface DataFunctionArgs {
interface DataFunctionArgs<Context> {
request: Request;
params: Params;
context?: any;
context?: Context;
}

// TODO: (v7) Change the defaults from any to unknown in and remove Remix wrappers:
// ActionFunction, ActionFunctionArgs, LoaderFunction, LoaderFunctionArgs

/**
* Arguments passed to loader functions
*/
export interface LoaderFunctionArgs extends DataFunctionArgs {}
export interface LoaderFunctionArgs<C = any> extends DataFunctionArgs<C> {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface LoaderFunctionArgs<C = any> extends DataFunctionArgs<C> {}
export interface LoaderFunctionArgs<Context = any> extends DataFunctionArgs<Context> {}


/**
* Arguments passed to action functions
*/
export interface ActionFunctionArgs extends DataFunctionArgs {}
export interface ActionFunctionArgs<C = any> extends DataFunctionArgs<C> {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface ActionFunctionArgs<C = any> extends DataFunctionArgs<C> {}
export interface ActionFunctionArgs<Context = any> extends DataFunctionArgs<Context> {}


/**
* Loaders and actions can return anything except `undefined` (`null` is a
Expand All @@ -163,15 +166,15 @@ type DataFunctionValue = Response | NonNullable<unknown> | null;
/**
* Route loader function signature
*/
export interface LoaderFunction {
(args: LoaderFunctionArgs): Promise<DataFunctionValue> | DataFunctionValue;
export interface LoaderFunction<C = any> {
(args: LoaderFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
Comment on lines +169 to +170
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface LoaderFunction<C = any> {
(args: LoaderFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
export interface LoaderFunction<Context = any> {
(args: LoaderFunctionArgs<Context>): Promise<DataFunctionValue> | DataFunctionValue;

}

/**
* Route action function signature
*/
export interface ActionFunction {
(args: ActionFunctionArgs): Promise<DataFunctionValue> | DataFunctionValue;
export interface ActionFunction<C = any> {
(args: ActionFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
Comment on lines +176 to +177
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface ActionFunction<C = any> {
(args: ActionFunctionArgs<C>): Promise<DataFunctionValue> | DataFunctionValue;
export interface ActionFunction<Context = any> {
(args: ActionFunctionArgs<Context>): Promise<DataFunctionValue> | DataFunctionValue;

}

/**
Expand Down Expand Up @@ -490,6 +493,28 @@ export function matchRoutes<
return matches;
}

export interface UIMatch<D = unknown, H = unknown> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface UIMatch<D = unknown, H = unknown> {
export interface UIMatch<Data = unknown, Handle = unknown> {

id: string;
pathname: string;
params: AgnosticRouteMatch["params"];
data: D;
handle: H;
Comment on lines +500 to +501
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
data: D;
handle: H;
data: Data;
handle: Handle;

}

export function convertRouteMatchToUiMatch(
match: AgnosticDataRouteMatch,
loaderData: RouteData
): UIMatch {
let { route, pathname, params } = match;
return {
id: route.id,
pathname,
params,
data: loaderData[route.id],
handle: route.handle,
};
}

interface RouteMeta<
RouteObjectType extends AgnosticRouteObject = AgnosticRouteObject
> {
Expand Down