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

fix: support basename in static routers #9591

Merged
merged 6 commits into from Nov 23, 2022
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
6 changes: 6 additions & 0 deletions .changeset/funny-oranges-arrive.md
@@ -0,0 +1,6 @@
---
"react-router-dom": patch
"@remix-run/router": patch
---

Support `basename` in static data routers
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -107,7 +107,7 @@
},
"filesize": {
"packages/router/dist/router.umd.min.js": {
"none": "34.5 kB"
"none": "35 kB"
},
"packages/react-router/dist/react-router.production.min.js": {
"none": "12.5 kB"
Expand Down
66 changes: 63 additions & 3 deletions packages/react-router-dom/__tests__/data-static-router-test.tsx
Expand Up @@ -3,6 +3,7 @@ import * as ReactDOMServer from "react-dom/server";
import type { StaticHandlerContext } from "@remix-run/router";
import { unstable_createStaticHandler as createStaticHandler } from "@remix-run/router";
import {
Link,
Outlet,
useLoaderData,
useLocation,
Expand All @@ -17,7 +18,7 @@ beforeEach(() => {
jest.spyOn(console, "warn").mockImplementation(() => {});
});

describe("A <DataStaticRouter>", () => {
describe("A <StaticRouterProvider>", () => {
it("renders an initialized router", async () => {
let hooksData1: {
location: ReturnType<typeof useLocation>;
Expand Down Expand Up @@ -45,7 +46,12 @@ describe("A <DataStaticRouter>", () => {
loaderData: useLoaderData(),
matches: useMatches(),
};
return <h1>👋</h1>;
return (
<>
<h1>👋</h1>
<Link to="/the/other/path">Other</Link>
</>
);
}

let routes = [
Expand All @@ -71,7 +77,7 @@ describe("A <DataStaticRouter>", () => {
let { query } = createStaticHandler(routes);

let context = (await query(
new Request("http:/localhost/the/path?the=query#the-hash", {
new Request("http://localhost/the/path?the=query#the-hash", {
signal: new AbortController().signal,
})
)) as StaticHandlerContext;
Expand All @@ -85,6 +91,7 @@ describe("A <DataStaticRouter>", () => {
</React.StrictMode>
);
expect(html).toMatch("<h1>👋</h1>");
expect(html).toMatch('<a href="/the/other/path">');

// @ts-expect-error
expect(hooksData1.location).toEqual({
Expand Down Expand Up @@ -155,6 +162,59 @@ describe("A <DataStaticRouter>", () => {
]);
});

it("renders an initialized router with a basename", async () => {
let location: ReturnType<typeof useLocation>;

function GetLocation() {
location = useLocation();
return (
<>
<h1>👋</h1>
<Link to="/the/other/path">Other</Link>
</>
);
}

let routes = [
{
path: "the",
children: [
{
path: "path",
element: <GetLocation />,
},
],
},
];
let { query } = createStaticHandler(routes, { basename: "/base" });

let context = (await query(
new Request("http://localhost/base/the/path?the=query#the-hash", {
signal: new AbortController().signal,
})
)) as StaticHandlerContext;

let html = ReactDOMServer.renderToStaticMarkup(
<React.StrictMode>
<StaticRouterProvider
router={createStaticRouter(routes, context)}
context={context}
/>
</React.StrictMode>
);
expect(html).toMatch("<h1>👋</h1>");
expect(html).toMatch('<a href="/base/the/other/path">');

// @ts-expect-error
expect(location).toEqual({
pathname: "/the/path",
search: "?the=query",
hash: "#the-hash",
state: null,
key: expect.any(String),
});
});

it("renders hydration data by default", async () => {
let routes = [
{
Expand Down
15 changes: 15 additions & 0 deletions packages/react-router-dom/__tests__/static-link-test.tsx
Expand Up @@ -17,6 +17,21 @@ describe("A <Link> in a <StaticRouter>", () => {

expect(renderer.root.findByType("a").props.href).toEqual("/mjackson");
});

it("uses the right href with a basename", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<StaticRouter location="/base" basename="/base">
<Link to="mjackson" />
</StaticRouter>
);
});

expect(renderer.root.findByType("a").props.href).toEqual(
"/base/mjackson"
);
});
});

describe("with an object", () => {
Expand Down
6 changes: 2 additions & 4 deletions packages/react-router-dom/server.tsx
Expand Up @@ -65,7 +65,6 @@ export function StaticRouter({
}

export interface StaticRouterProviderProps {
basename?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To align with <RouterProvider> we no longer accept a basename as a prop, we inherit from the router passed to it. In the SSR case, that further inherits from the static handler.

context: StaticHandlerContext;
router: RemixRouter;
hydrate?: boolean;
Expand All @@ -77,7 +76,6 @@ export interface StaticRouterProviderProps {
* on the server where there is no stateful UI.
*/
export function unstable_StaticRouterProvider({
basename,
context,
router,
hydrate = true,
Expand All @@ -92,7 +90,7 @@ export function unstable_StaticRouterProvider({
router,
navigator: getStatelessNavigator(),
static: true,
basename: basename || "/",
basename: context.basename || "/",
};

let hydrateScript = "";
Expand Down Expand Up @@ -191,7 +189,7 @@ export function unstable_createStaticRouter(

return {
get basename() {
return "/";
return context.basename;
},
get state() {
return {
Expand Down
47 changes: 47 additions & 0 deletions packages/router/__tests__/router-test.ts
Expand Up @@ -10100,6 +10100,21 @@ describe("a router", () => {
});
});

it("should support document load navigations with a basename", async () => {
let { query } = createStaticHandler(SSR_ROUTES, { basename: "/base" });
let context = await query(createRequest("/base/parent/child"));
expect(context).toMatchObject({
actionData: null,
loaderData: {
parent: "PARENT LOADER",
child: "CHILD LOADER",
},
errors: null,
location: { pathname: "/base/parent/child" },
matches: [{ route: { id: "parent" } }, { route: { id: "child" } }],
});
});

it("should support document load navigations returning responses", async () => {
let { query } = createStaticHandler(SSR_ROUTES);
let context = await query(createRequest("/parent/json"));
Expand Down Expand Up @@ -11020,6 +11035,38 @@ describe("a router", () => {
expect(data).toBe("");
});

it("should support singular route load navigations (with a basename)", async () => {
let { queryRoute } = createStaticHandler(SSR_ROUTES, {
basename: "/base",
});
let data;

// Layout route
data = await queryRoute(createRequest("/base/parent"), "parent");
expect(data).toBe("PARENT LOADER");

// Index route
data = await queryRoute(createRequest("/base/parent"), "parentIndex");
expect(data).toBe("PARENT INDEX LOADER");

// Parent in nested route
data = await queryRoute(createRequest("/base/parent/child"), "parent");
expect(data).toBe("PARENT LOADER");

// Child in nested route
data = await queryRoute(createRequest("/base/parent/child"), "child");
expect(data).toBe("CHILD LOADER");

// Non-undefined falsey values should count
let T = setupFlexRouteTest();
data = await T.resolveLoader(null);
expect(data).toBeNull();
data = await T.resolveLoader(false);
expect(data).toBe(false);
data = await T.resolveLoader("");
expect(data).toBe("");
});

it("should support singular route submit navigations (primitives)", async () => {
let { queryRoute } = createStaticHandler(SSR_ROUTES);
let data;
Expand Down
30 changes: 20 additions & 10 deletions packages/router/router.ts
Expand Up @@ -298,6 +298,7 @@ export interface RouterInit {
* State returned from a server-side query() call
*/
export interface StaticHandlerContext {
basename: Router["basename"];
location: RouterState["location"];
matches: RouterState["matches"];
loaderData: RouterState["loaderData"];
Expand Down Expand Up @@ -1858,14 +1859,18 @@ const validActionMethods = new Set(["POST", "PUT", "PATCH", "DELETE"]);
const validRequestMethods = new Set(["GET", "HEAD", ...validActionMethods]);

export function unstable_createStaticHandler(
routes: AgnosticRouteObject[]
routes: AgnosticRouteObject[],
opts?: {
basename?: string;
}
): StaticHandler {
invariant(
routes.length > 0,
"You must provide a non-empty routes array to unstable_createStaticHandler"
);

let dataRoutes = convertRoutesToDataRoutes(routes);
let basename = (opts ? opts.basename : null) || "/";

/**
* The query() method is intended for document requests, in which we want to
Expand All @@ -1891,13 +1896,14 @@ export function unstable_createStaticHandler(
): Promise<StaticHandlerContext | Response> {
let url = new URL(request.url);
let location = createLocation("", createPath(url), null, "default");
let matches = matchRoutes(dataRoutes, location);
let matches = matchRoutes(dataRoutes, location, basename);

if (!validRequestMethods.has(request.method)) {
let error = getInternalRouterError(405, { method: request.method });
let { matches: methodNotAllowedMatches, route } =
getShortCircuitMatches(dataRoutes);
return {
basename,
location,
matches: methodNotAllowedMatches,
loaderData: {},
Expand All @@ -1914,6 +1920,7 @@ export function unstable_createStaticHandler(
let { matches: notFoundMatches, route } =
getShortCircuitMatches(dataRoutes);
return {
basename,
location,
matches: notFoundMatches,
loaderData: {},
Expand All @@ -1935,7 +1942,7 @@ export function unstable_createStaticHandler(
// When returning StaticHandlerContext, we patch back in the location here
// since we need it for React Context. But this helps keep our submit and
// loadRouteData operating on a Request instead of a Location
return { location, ...result };
return { location, basename, ...result };
}

/**
Expand All @@ -1961,7 +1968,7 @@ export function unstable_createStaticHandler(
async function queryRoute(request: Request, routeId?: string): Promise<any> {
let url = new URL(request.url);
let location = createLocation("", createPath(url), null, "default");
let matches = matchRoutes(dataRoutes, location);
let matches = matchRoutes(dataRoutes, location, basename);

if (!validRequestMethods.has(request.method)) {
throw getInternalRouterError(405, { method: request.method });
Expand Down Expand Up @@ -2007,7 +2014,7 @@ export function unstable_createStaticHandler(
location: Location,
matches: AgnosticDataRouteMatch[],
routeMatch?: AgnosticDataRouteMatch
): Promise<Omit<StaticHandlerContext, "location"> | Response> {
): Promise<Omit<StaticHandlerContext, "location" | "basename"> | Response> {
invariant(
request.signal,
"query()/queryRoute() requests must contain an AbortController signal"
Expand Down Expand Up @@ -2056,7 +2063,7 @@ export function unstable_createStaticHandler(
matches: AgnosticDataRouteMatch[],
actionMatch: AgnosticDataRouteMatch,
isRouteRequest: boolean
): Promise<Omit<StaticHandlerContext, "location"> | Response> {
): Promise<Omit<StaticHandlerContext, "location" | "basename"> | Response> {
let result: DataResult;

if (!actionMatch.route.action) {
Expand All @@ -2078,7 +2085,7 @@ export function unstable_createStaticHandler(
request,
actionMatch,
matches,
undefined, // Basename not currently supported in static handlers
basename,
true,
isRouteRequest
);
Expand Down Expand Up @@ -2168,7 +2175,10 @@ export function unstable_createStaticHandler(
routeMatch?: AgnosticDataRouteMatch,
pendingActionError?: RouteData
): Promise<
| Omit<StaticHandlerContext, "location" | "actionData" | "actionHeaders">
| Omit<
StaticHandlerContext,
"location" | "basename" | "actionData" | "actionHeaders"
>
| Response
> {
let isRouteRequest = routeMatch != null;
Expand Down Expand Up @@ -2208,7 +2218,7 @@ export function unstable_createStaticHandler(
request,
match,
matches,
undefined, // Basename not currently supported in static handlers
basename,
true,
isRouteRequest
)
Expand Down Expand Up @@ -2519,7 +2529,7 @@ async function callLoaderOrAction(
request: Request,
match: AgnosticDataRouteMatch,
matches: AgnosticDataRouteMatch[],
basename: string | undefined,
basename = "/",
isStaticRequest: boolean = false,
isRouteRequest: boolean = false
): Promise<DataResult> {
Expand Down