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

Narrow types for hash, pathname and search #11282

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions .changeset/selfish-walls-relate.md
@@ -0,0 +1,5 @@
---
"react-router": patch
---

Narrow the types of `pathname`, `search` and `hash` on `Location`
1 change: 1 addition & 0 deletions contributors.yml
Expand Up @@ -136,6 +136,7 @@
- kylegirard
- landisdesign
- latin-1
- lensbart
- lequangdongg
- liuhanqu
- lkwr
Expand Down
14 changes: 7 additions & 7 deletions packages/react-router-dom-v5-compat/lib/components.tsx
@@ -1,6 +1,6 @@
import * as React from "react";
import type { Location, To } from "history";
import { Action, createPath, parsePath } from "history";
import type { To } from "history";
import { Action, createPath, parsePath, Location } from "history";

// Get useHistory from react-router-dom v5 (peer dep).
// @ts-expect-error
Expand Down Expand Up @@ -82,13 +82,13 @@ export function StaticRouter({
}

let action = Action.Pop;
let location: Location = {
pathname: locationProp.pathname || "/",
search: locationProp.search || "",
hash: locationProp.hash || "",
let location = {
pathname: (locationProp.pathname || "/") as "" | `/${string}`,
search: (locationProp.search || "") as "" | `?${string}`,
hash: (locationProp.hash || "") as "" | `#${string}`,
state: locationProp.state || null,
key: locationProp.key || "default",
};
} satisfies Location;

let staticNavigator = {
createHref(to: To) {
Expand Down
17 changes: 10 additions & 7 deletions packages/react-router-dom/index.tsx
Expand Up @@ -1063,16 +1063,20 @@ export const NavLink = React.forwardRef<HTMLAnchorElement, NavLinkProps>(
: null;

if (!caseSensitive) {
locationPathname = locationPathname.toLowerCase();
locationPathname = locationPathname.toLowerCase() as Lowercase<
typeof locationPathname
>;
nextLocationPathname = nextLocationPathname
? nextLocationPathname.toLowerCase()
? (nextLocationPathname.toLowerCase() as Lowercase<
typeof nextLocationPathname
>)
: null;
toPathname = toPathname.toLowerCase();
}

if (nextLocationPathname && basename) {
nextLocationPathname =
stripBasename(nextLocationPathname, basename) || nextLocationPathname;
nextLocationPathname = (stripBasename(nextLocationPathname, basename) ||
nextLocationPathname) as Location["pathname"];
}

// If the `to` has a trailing slash, look at that exact spot. Otherwise,
Expand Down Expand Up @@ -1822,9 +1826,8 @@ function useScrollRestoration({
// Strip the basename to match useLocation()
{
...location,
pathname:
stripBasename(location.pathname, basename) ||
location.pathname,
pathname: (stripBasename(location.pathname, basename) ||
location.pathname) as Location["pathname"],
},
matches
)
Expand Down
2 changes: 1 addition & 1 deletion packages/react-router-dom/server.tsx
Expand Up @@ -58,7 +58,7 @@ export function StaticRouter({
future,
}: StaticRouterProps) {
if (typeof locationProp === "string") {
locationProp = parsePath(locationProp);
locationProp = parsePath(locationProp) as Partial<Location>;
}

let action = Action.Pop;
Expand Down
161 changes: 161 additions & 0 deletions packages/react-router/__tests__/context-test.tsx
@@ -0,0 +1,161 @@
/* eslint-disable @typescript-eslint/no-unused-vars -- type tests */
/* eslint-disable jest/expect-expect -- type tests */
import * as React from "react";
import {
UNSAFE_LocationContext as LocationContext,
NavigationType,
} from "react-router";

const location = {
pathname: "",
search: "",
hash: "",
state: null,
key: "default",
} as const;

describe("LocationContext", () => {
it("accepts an object with the correct `pathname`", () => {
const validCases = [
<LocationContext.Provider
value={{
location: { ...location, pathname: "/" },
navigationType: NavigationType.Pop,
}}
/>,
<LocationContext.Provider
value={{
location: { ...location, pathname: "/something" },
navigationType: NavigationType.Pop,
}}
/>,
<LocationContext.Provider
value={{
location: { ...location, pathname: "" },
navigationType: NavigationType.Pop,
}}
/>,
];
});

it("accepts an object with the correct `hash`", () => {
const validCases = [
<LocationContext.Provider
value={{
location: { ...location, hash: "#" },
navigationType: NavigationType.Pop,
}}
/>,
<LocationContext.Provider
value={{
location: { ...location, hash: "#something" },
navigationType: NavigationType.Pop,
}}
/>,
<LocationContext.Provider
value={{
location: { ...location, hash: "" },
navigationType: NavigationType.Pop,
}}
/>,
];
});

it("accepts an object with the correct `search`", () => {
const validCases = [
<LocationContext.Provider
value={{
location: { ...location, search: "?" },
navigationType: NavigationType.Pop,
}}
/>,
<LocationContext.Provider
value={{
location: { ...location, search: "?something" },
navigationType: NavigationType.Pop,
}}
/>,
<LocationContext.Provider
value={{
location: { ...location, search: "" },
navigationType: NavigationType.Pop,
}}
/>,
];
});

it("rejects an object with the wrong `pathname`", () => {
const invalidCases = [
<LocationContext.Provider
value={{
location: {
...location,
// @ts-expect-error
pathname: "something",
},
navigationType: NavigationType.Pop,
}}
/>,
<LocationContext.Provider
value={{
location: {
...location,
// @ts-expect-error
pathname: "some/thing",
},
navigationType: NavigationType.Pop,
}}
/>,
];
});

it("rejects an object with the wrong `hash`", () => {
const invalidCases = [
<LocationContext.Provider
value={{
location: {
...location,
// @ts-expect-error
hash: "something",
},
navigationType: NavigationType.Pop,
}}
/>,
<LocationContext.Provider
value={{
location: {
...location,
// @ts-expect-error
hash: "some#thing",
},
navigationType: NavigationType.Pop,
}}
/>,
];
});

it("rejects an object with the wrong `search`", () => {
const invalidCases = [
<LocationContext.Provider
value={{
location: {
...location,
// @ts-expect-error
search: "something",
},
navigationType: NavigationType.Pop,
}}
/>,
<LocationContext.Provider
value={{
location: {
...location,
// @ts-expect-error
search: "some?thing",
},
navigationType: NavigationType.Pop,
}}
/>,
];
});
});
18 changes: 18 additions & 0 deletions packages/react-router/__tests__/useLocation-test.tsx
@@ -1,6 +1,7 @@
import * as React from "react";
import * as TestRenderer from "react-test-renderer";
import { MemoryRouter, Routes, Route, useLocation } from "react-router";
import type { Equal, Expect } from "@remix-run/router/__tests__/utils/utils";

function ShowLocation() {
let location = useLocation();
Expand Down Expand Up @@ -84,4 +85,21 @@ describe("useLocation", () => {
</pre>
`);
});

// eslint-disable-next-line jest/expect-expect -- type tests
it("returns an object with the correct type", () => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars -- used for type tests
function TestUseLocationReturnType() {
let location = useLocation();

// eslint-disable-next-line @typescript-eslint/no-unused-vars -- type test
type Test1 = Expect<Equal<typeof location.hash, "" | `#${string}`>>;
// eslint-disable-next-line @typescript-eslint/no-unused-vars -- type test
type Test2 = Expect<Equal<typeof location.pathname, "" | `/${string}`>>;
// eslint-disable-next-line @typescript-eslint/no-unused-vars -- type test
type Test3 = Expect<Equal<typeof location.search, "" | `?${string}`>>;

return null;
}
});
timdorr marked this conversation as resolved.
Show resolved Hide resolved
});
4 changes: 2 additions & 2 deletions packages/react-router/lib/components.tsx
Expand Up @@ -441,7 +441,7 @@ export function Router({
);

if (typeof locationProp === "string") {
locationProp = parsePath(locationProp);
locationProp = parsePath(locationProp) as Partial<Location>;
}

let {
Expand All @@ -461,7 +461,7 @@ export function Router({

return {
location: {
pathname: trailingPathname,
pathname: trailingPathname as Location["pathname"],
search,
hash,
state,
Expand Down
17 changes: 8 additions & 9 deletions packages/react-router/lib/hooks.tsx
Expand Up @@ -421,7 +421,7 @@ export function useRoutesImpl(
location = locationFromContext;
}

let pathname = location.pathname || "/";
let pathname = (location.pathname || "/") as `/${string}`;

let remainingPathname = pathname;
if (parentPathnameBase !== "/") {
Expand All @@ -441,7 +441,8 @@ export function useRoutesImpl(
// And the direct substring removal approach won't work :/
let parentSegments = parentPathnameBase.replace(/^\//, "").split("/");
let segments = pathname.replace(/^\//, "").split("/");
remainingPathname = "/" + segments.slice(parentSegments.length).join("/");
remainingPathname = ("/" +
segments.slice(parentSegments.length).join("/")) as `/${string}`;
}

let matches = matchRoutes(routes, { pathname: remainingPathname });
Expand Down Expand Up @@ -506,7 +507,7 @@ export function useRoutesImpl(
state: null,
key: "default",
...location,
},
} as Location,
navigationType: NavigationType.Pop,
}}
>
Expand Down Expand Up @@ -1020,15 +1021,13 @@ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker {
return shouldBlock({
currentLocation: {
...currentLocation,
pathname:
stripBasename(currentLocation.pathname, basename) ||
currentLocation.pathname,
pathname: (stripBasename(currentLocation.pathname, basename) ||
currentLocation.pathname) as Location["pathname"],
},
nextLocation: {
...nextLocation,
pathname:
stripBasename(nextLocation.pathname, basename) ||
nextLocation.pathname,
pathname: (stripBasename(nextLocation.pathname, basename) ||
nextLocation.pathname) as Location["pathname"],
},
historyAction,
});
Expand Down
8 changes: 8 additions & 0 deletions packages/router/__tests__/utils/utils.ts
Expand Up @@ -91,3 +91,11 @@ export function createSubmitRequest(path: string, opts?: RequestInit) {
...opts,
});
}

// See https://www.totaltypescript.com/how-to-test-your-types
export type Expect<T extends true> = T;
export type Equal<X, Y> = (<T>() => T extends X ? 1 : 2) extends <
T
>() => T extends Y ? 1 : 2
? true
: false;
19 changes: 17 additions & 2 deletions packages/router/history.ts
Expand Up @@ -57,6 +57,21 @@ export interface Path {
* URL path, as well as possibly some arbitrary state and a key.
*/
export interface Location<State = any> extends Path {
/**
* A URL pathname, beginning with a /.
*/
pathname: "" | `/${string}`;

/**
* A URL search string, beginning with a ?.
*/
search: "" | `?${string}`;

/**
* A URL fragment identifier, beginning with a #.
*/
hash: "" | `#${string}`;

/**
* A value of arbitrary data associated with this location.
*/
Expand Down Expand Up @@ -536,7 +551,7 @@ export function createLocation(
state: any = null,
key?: string
): Readonly<Location> {
let location: Readonly<Location> = {
let location = {
pathname: typeof current === "string" ? current : current.pathname,
search: "",
hash: "",
Expand All @@ -547,7 +562,7 @@ export function createLocation(
// But that's a pretty big refactor to the current test suite so going to
// keep as is for the time being and just let any incoming keys take precedence
key: (to && (to as Location).key) || key || createKey(),
};
} as Readonly<Location>;
return location;
}

Expand Down