From 09e81a8e4c8dd6ad303b88037e02b0209d848f5e Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 1 May 2024 14:55:10 -0400 Subject: [PATCH] Resource Route Single Fetch Updates (#9349) --- .changeset/rich-spoons-draw.md | 5 + .changeset/slow-peaches-matter.md | 5 + docs/guides/single-fetch.md | 70 ++++++++ integration/package.json | 2 +- integration/single-fetch-test.ts | 167 ++++++++++++++++++ packages/remix-dev/package.json | 2 +- packages/remix-react/package.json | 6 +- packages/remix-server-runtime/deprecations.ts | 13 ++ packages/remix-server-runtime/package.json | 2 +- packages/remix-server-runtime/server.ts | 43 +++++ packages/remix-server-runtime/single-fetch.ts | 31 +++- packages/remix-testing/package.json | 4 +- pnpm-lock.yaml | 50 +++--- 13 files changed, 365 insertions(+), 35 deletions(-) create mode 100644 .changeset/rich-spoons-draw.md create mode 100644 .changeset/slow-peaches-matter.md create mode 100644 packages/remix-server-runtime/deprecations.ts diff --git a/.changeset/rich-spoons-draw.md b/.changeset/rich-spoons-draw.md new file mode 100644 index 00000000000..dc44bcf0d71 --- /dev/null +++ b/.changeset/rich-spoons-draw.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": patch +--- + +Automatically wrap resource route naked object returns in `json()` for back-compat in v2 (and log deprecation warning) diff --git a/.changeset/slow-peaches-matter.md b/.changeset/slow-peaches-matter.md new file mode 100644 index 00000000000..4319d7d95e1 --- /dev/null +++ b/.changeset/slow-peaches-matter.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": patch +--- + +Pass `response` stub to resource route handlerså when single fetch is enabled diff --git a/docs/guides/single-fetch.md b/docs/guides/single-fetch.md index 1c0f5048555..810ec3c7f74 100644 --- a/docs/guides/single-fetch.md +++ b/docs/guides/single-fetch.md @@ -270,6 +270,74 @@ And then when `c` calls `serverLoader`, it'll make it's own call for just the `c GET /a/b/c.data?_routes=routes/c ``` +### Resource Routes + +Because of the new streaming format used by Single Fetch, raw JavaScript objects returned from `loader` and `action` functions are no longer automatically converted to `Response` instances via the `json()` utility. Instead, in normal navigational data loads they're combined with the other loader data and streamed down in a `turbo-stream` response. Resource routes are unique because they're intended to be hit individually -- and not always via Remix client side code. They can also be accessed via any other HTTP client (`fetch`, `cURL`, etc.). + +With Single Fetch enabled, raw Javascript objects returned from resource routes will be handled as follows: + +When accessing from a Remix API such as `useFetcher`, raw Javascript objects will be returned as turbo-stream responses, just like normal loaders and actions (this is because `useFetcher` will append the `.data` suffix to the request). + +When accessing from an external tool such as `fetch` or `cURL`, we will continue this automatic conversion to `json()` or backwards-compatibility in v2: + +- When we detect a raw object for an external request in v2, we will will log a deprecation warning and wrap the value in `json()` for easier opt-into the Single Fetch feature +- At your convenience, you can add the `json()` call to your resource route handlers to stop returning raw objects when you want a JSON response for external consumption +- Once you've addressed all of the deprecation warnings in your application's resource routes, you will be better prepared for the eventual Remix v3 upgrade + +```tsx filename=app/routes/resource.tsx bad +export function loader() { + return { + message: "My externally-accessed resource route", + }; +} +``` + +```tsx filename=app/routes/resource.tsx good +import { json } from "@remix-run/react"; + +export function loader() { + return json({ + message: "My externally-accessed resource route", + }); +} +``` + +#### Response Stub and Resource Routes + +Ad discussed above, the `headers` export is deprecated in favor of a new [`response` stub][responsestub] passed to your `loader` and `action` functions. This is somewhat confusing in resource routes, though, because you get to return the _actual_ `Response` - there's no real need for a "stub" concept because there's no merging results from multiple loaders into a single Response: + +```tsx filename=routes/resource.tsx +// Using your own Response is the most straightforward approach +export async function loader() { + const data = await getData(); + return json(data, { + status: 200, + headers: { + "X-Custom": "whatever", + }, + }); +} +``` + +To keep things consistent, resource route `loader`/`action` functions will still receive a `response` stub and you can use it if you need to (maybe to share code amongst non-resource-route handlers): + +```tsx filename=routes/resource.tsx +// But you can still set values on the response stubstraightforward approach +export async function loader({ + response, +}: LoaderFunctionArgs) { + const data = await getData(); + response.status = 200; + response.headers.set("X-Custom", "whatever"); + return json(data); +} +``` + +It's best to try to avoid using the `response` stub _and also_ returning a `Response` with custom status/headers, but if you do, the following logic will apply": + +- The `Response` instance status will take priority over any `response` stub status +- Headers operations on the `response` stub `headers` will be re-played on the returned `Response` headers instance + [future-flags]: ../file-conventions/remix-config#future [should-revalidate]: ../route/should-revalidate [entry-server]: ../file-conventions/entry.server @@ -284,3 +352,5 @@ GET /a/b/c.data?_routes=routes/c [starttransition]: https://react.dev/reference/react/startTransition [headers]: ../route/headers [mdn-headers]: https://developer.mozilla.org/en-US/docs/Web/API/Headers +[resource-routes]: ../guides/resource-routes +[responsestub]: #headers diff --git a/integration/package.json b/integration/package.json index 3e59031d9f9..f3b75892825 100644 --- a/integration/package.json +++ b/integration/package.json @@ -14,7 +14,7 @@ "@remix-run/dev": "workspace:*", "@remix-run/express": "workspace:*", "@remix-run/node": "workspace:*", - "@remix-run/router": "1.16.0", + "@remix-run/router": "1.16.1-pre.0", "@remix-run/server-runtime": "workspace:*", "@types/express": "^4.17.9", "@vanilla-extract/css": "^1.10.0", diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts index c571d63f46e..7b1abd76f95 100644 --- a/integration/single-fetch-test.ts +++ b/integration/single-fetch-test.ts @@ -1500,6 +1500,173 @@ test.describe("single-fetch", () => { expect(await app.getHtml("#target")).toContain("Target"); }); + test("wraps resource route naked object returns in json with a deprecation warning", async () => { + let oldConsoleWarn = console.warn; + let warnLogs: unknown[] = []; + console.warn = (...args) => warnLogs.push(...args); + + let fixture = await createFixture( + { + config: { + future: { + unstable_singleFetch: true, + }, + }, + files: { + ...files, + "app/routes/resource.tsx": js` + export function loader() { + return { message: "RESOURCE" }; + } + `, + }, + }, + ServerMode.Development + ); + let res = await fixture.requestResource("/resource"); + expect(await res.json()).toEqual({ + message: "RESOURCE", + }); + + expect(warnLogs).toEqual([ + "⚠️ REMIX FUTURE CHANGE: Resource routes will no longer be able to return " + + "raw JavaScript objects in v3 when Single Fetch becomes the default. You " + + "can prepare for this change at your convenience by wrapping the data " + + "returned from your `loader` function in the `routes/resource` route with " + + "`json()`. For instructions on making this change see " + + "https://remix.run/docs/en/v2.9.2/guides/single-fetch#resource-routes", + ]); + console.warn = oldConsoleWarn; + }); + + test("processes response stub onto resource routes returning raw data", async () => { + let fixture = await createFixture( + { + config: { + future: { + unstable_singleFetch: true, + }, + }, + files: { + ...files, + "app/routes/resource.tsx": js` + import { json } from '@remix-run/node'; + + export function loader({ response }) { + // When raw json is returned, the stub status/headers will just be used directly + response.status = 201; + response.headers.set('X-Stub', 'yes') + return { message: "RESOURCE" }; + } + `, + }, + }, + ServerMode.Development + ); + let res = await fixture.requestResource("/resource"); + expect(res.status).toBe(201); + expect(res.headers.get("X-Stub")).toBe("yes"); + expect(await res.json()).toEqual({ + message: "RESOURCE", + }); + }); + + test("processes response stub onto resource routes returning responses", async () => { + let fixture = await createFixture( + { + config: { + future: { + unstable_singleFetch: true, + }, + }, + files: { + ...files, + "app/routes/resource.tsx": js` + import { json } from '@remix-run/node'; + + export function loader({ response }) { + // This will be ignored in favor of the returned Response status + response.status = 200; + response.headers.set('X-Stub', 'yes') + // This will overwrite the returned Response header + response.headers.set('X-Set', '2') + // This will append to the returned Response header + response.headers.append('X-Append', '2') + return json({ message: "RESOURCE" }, { + // This one takes precedence + status: 201, + headers: { + 'X-Response': 'yes', + 'X-Set': '1', + 'X-Append': '1', + }, + }); + } + `, + }, + }, + ServerMode.Development + ); + let res = await fixture.requestResource("/resource"); + expect(res.status).toBe(201); + expect(res.headers.get("X-Response")).toBe("yes"); + expect(res.headers.get("X-Stub")).toBe("yes"); + expect(res.headers.get("X-Set")).toBe("2"); + expect(res.headers.get("X-Append")).toBe("1, 2"); + expect(await res.json()).toEqual({ + message: "RESOURCE", + }); + }); + + test("allows fetcher to hit resource route and return via turbo stream", async ({ + page, + }) => { + let fixture = await createFixture({ + config: { + future: { + unstable_singleFetch: true, + }, + }, + files: { + ...files, + "app/routes/_index.tsx": js` + import { useFetcher } from "@remix-run/react"; + + export default function Component() { + let fetcher = useFetcher(); + return ( +
+ + {fetcher.data ? 
{fetcher.data.message} {fetcher.data.date.toISOString()}
: null} +
+ ); + } + `, + "app/routes/resource.tsx": js` + export function loader() { + // Fetcher calls to resource routes will append ".data" and we'll go through + // the turbo-stream flow. If a user were to curl this endpoint they'd go + // through "handleResourceRoute" and it would be returned as "json()" + return { + message: "RESOURCE", + date: new Date("${ISO_DATE}"), + }; + } + `, + }, + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + await app.clickElement("#load"); + await page.waitForSelector("#fetcher-data"); + expect(await app.getHtml("#fetcher-data")).toContain( + "RESOURCE 2024-03-12T12:00:00.000Z" + ); + }); + test.describe("client loaders", () => { test("when no routes have client loaders", async ({ page }) => { let fixture = await createFixture( diff --git a/packages/remix-dev/package.json b/packages/remix-dev/package.json index 66769100e6c..2b990a5a524 100644 --- a/packages/remix-dev/package.json +++ b/packages/remix-dev/package.json @@ -32,7 +32,7 @@ "@mdx-js/mdx": "^2.3.0", "@npmcli/package-json": "^4.0.1", "@remix-run/node": "workspace:*", - "@remix-run/router": "1.16.0", + "@remix-run/router": "1.16.1-pre.0", "@remix-run/server-runtime": "workspace:*", "@types/mdx": "^2.0.5", "@vanilla-extract/integration": "^6.2.0", diff --git a/packages/remix-react/package.json b/packages/remix-react/package.json index 08f6e8179f7..29675765c11 100644 --- a/packages/remix-react/package.json +++ b/packages/remix-react/package.json @@ -19,10 +19,10 @@ "tsc": "tsc" }, "dependencies": { - "@remix-run/router": "1.16.0", + "@remix-run/router": "1.16.1-pre.0", "@remix-run/server-runtime": "workspace:*", - "react-router": "6.23.0", - "react-router-dom": "6.23.0", + "react-router": "6.23.1-pre.0", + "react-router-dom": "6.23.1-pre.0", "turbo-stream": "^2.0.0" }, "devDependencies": { diff --git a/packages/remix-server-runtime/deprecations.ts b/packages/remix-server-runtime/deprecations.ts new file mode 100644 index 00000000000..d6c25769cf8 --- /dev/null +++ b/packages/remix-server-runtime/deprecations.ts @@ -0,0 +1,13 @@ +export function resourceRouteJsonWarning( + type: "loader" | "action", + routeId: string +) { + return ( + "⚠️ REMIX FUTURE CHANGE: Resource routes will no longer be able to " + + "return raw JavaScript objects in v3 when Single Fetch becomes the default. " + + "You can prepare for this change at your convenience by wrapping the data " + + `returned from your \`${type}\` function in the \`${routeId}\` route with ` + + "`json()`. For instructions on making this change see " + + "https://remix.run/docs/en/v2.9.2/guides/single-fetch#resource-routes" + ); +} diff --git a/packages/remix-server-runtime/package.json b/packages/remix-server-runtime/package.json index 55ec3e4c4f2..d2afcdd090c 100644 --- a/packages/remix-server-runtime/package.json +++ b/packages/remix-server-runtime/package.json @@ -19,7 +19,7 @@ "tsc": "tsc" }, "dependencies": { - "@remix-run/router": "1.16.0", + "@remix-run/router": "1.16.1-pre.0", "@types/cookie": "^0.6.0", "@web3-storage/multipart-parser": "^1.0.0", "cookie": "^0.6.0", diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 269f56551cb..11305551c9c 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -29,6 +29,7 @@ import { isRedirectResponse, isRedirectStatusCode, isResponse, + json, } from "./responses"; import { createServerHandoffString } from "./serverHandoff"; import { getDevServerHooks } from "./dev"; @@ -38,11 +39,14 @@ import { getResponseStubs, getSingleFetchDataStrategy, getSingleFetchRedirect, + getSingleFetchResourceRouteDataStrategy, mergeResponseStubs, singleFetchAction, singleFetchLoaders, SingleFetchRedirectSymbol, } from "./single-fetch"; +import { resourceRouteJsonWarning } from "./deprecations"; +import { ResponseStubOperationsSymbol } from "./routeModules"; export type RequestHandler = ( request: Request, @@ -226,6 +230,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( ) { response = await handleResourceRequest( serverMode, + _build, staticHandler, matches.slice(-1)[0].route.id, request, @@ -559,6 +564,7 @@ async function handleDocumentRequest( async function handleResourceRequest( serverMode: ServerMode, + build: ServerBuild, staticHandler: StaticHandler, routeId: string, request: Request, @@ -566,13 +572,24 @@ async function handleResourceRequest( handleError: (err: unknown) => void ) { try { + let responseStubs = build.future.unstable_singleFetch + ? getResponseStubs() + : {}; // Note we keep the routeId here to align with the Remix handling of // resource routes which doesn't take ?index into account and just takes // the leaf match let response = await staticHandler.queryRoute(request, { routeId, requestContext: loadContext, + ...(build.future.unstable_singleFetch + ? { + unstable_dataStrategy: getSingleFetchResourceRouteDataStrategy({ + responseStubs, + }), + } + : null), }); + if (typeof response === "object") { invariant( !(DEFERRED_SYMBOL in response), @@ -580,6 +597,32 @@ async function handleResourceRequest( `forget to export a default UI component from the "${routeId}" route?` ); } + + if (build.future.unstable_singleFetch) { + let stub = responseStubs[routeId]; + if (isResponse(response)) { + // If a response was returned, we use it's status and we merge our + // response stub headers onto it + let ops = stub[ResponseStubOperationsSymbol]; + for (let [op, ...args] of ops) { + // @ts-expect-error + response.headers[op](...args); + } + } else { + console.warn( + resourceRouteJsonWarning( + request.method === "GET" ? "loader" : "action", + routeId + ) + ); + // Otherwise we create a json Response using the stub + response = json(response, { + status: stub.status, + headers: stub.headers, + }); + } + } + // callRouteLoader/callRouteAction always return responses (w/o single fetch). // With single fetch, users should always be Responses from resource routes invariant( diff --git a/packages/remix-server-runtime/single-fetch.ts b/packages/remix-server-runtime/single-fetch.ts index 8d9d358241a..bdfe745f7fc 100644 --- a/packages/remix-server-runtime/single-fetch.ts +++ b/packages/remix-server-runtime/single-fetch.ts @@ -1,6 +1,7 @@ import type { StaticHandler, unstable_DataStrategyFunctionArgs as DataStrategyFunctionArgs, + unstable_DataStrategyFunction as DataStrategyFunction, StaticHandlerContext, } from "@remix-run/router"; import { @@ -49,7 +50,7 @@ export function getSingleFetchDataStrategy( isActionDataRequest, loadRouteIds, }: { isActionDataRequest?: boolean; loadRouteIds?: string[] } = {} -) { +): DataStrategyFunction { return async ({ request, matches }: DataStrategyFunctionArgs) => { // Don't call loaders on action data requests if (isActionDataRequest && request.method === "GET") { @@ -102,6 +103,32 @@ export function getSingleFetchDataStrategy( }; } +export function getSingleFetchResourceRouteDataStrategy({ + responseStubs, +}: { + responseStubs: ReturnType; +}): DataStrategyFunction { + return async ({ matches }: DataStrategyFunctionArgs) => { + let results = await Promise.all( + matches.map(async (match) => { + let responseStub = match.shouldLoad + ? responseStubs[match.route.id] + : null; + let result = await match.resolve(async (handler) => { + // Cast `ResponseStubImpl -> ResponseStub` to hide the symbol in userland + let ctx: DataStrategyCtx = { + response: responseStub as ResponseStub, + }; + let data = await handler(ctx); + return { type: "data", result: data }; + }); + return result; + }) + ); + return results; + }; +} + export async function singleFetchAction( serverMode: ServerMode, staticHandler: StaticHandler, @@ -128,7 +155,7 @@ export async function singleFetchAction( }), }); - // Unlike `handleDataRequest`, when singleFetch is enabled, queryRoute does + // Unlike `handleDataRequest`, when singleFetch is enabled, query does // let non-Response return values through if (isResponse(result)) { return { diff --git a/packages/remix-testing/package.json b/packages/remix-testing/package.json index 73a3f2c537f..ae5003135e5 100644 --- a/packages/remix-testing/package.json +++ b/packages/remix-testing/package.json @@ -21,8 +21,8 @@ "dependencies": { "@remix-run/node": "workspace:*", "@remix-run/react": "workspace:*", - "@remix-run/router": "1.16.0", - "react-router-dom": "6.23.0" + "@remix-run/router": "1.16.1-pre.0", + "react-router-dom": "6.23.1-pre.0" }, "devDependencies": { "@remix-run/server-runtime": "workspace:*", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 42cf379d7d2..34bdab5023c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -323,8 +323,8 @@ importers: specifier: workspace:* version: link:../packages/remix-node '@remix-run/router': - specifier: 1.16.0 - version: 1.16.0 + specifier: 1.16.1-pre.0 + version: 1.16.1-pre.0 '@remix-run/server-runtime': specifier: workspace:* version: link:../packages/remix-server-runtime @@ -871,8 +871,8 @@ importers: specifier: ^2.9.1 version: link:../remix-react '@remix-run/router': - specifier: 1.16.0 - version: 1.16.0 + specifier: 1.16.1-pre.0 + version: 1.16.1-pre.0 '@remix-run/server-runtime': specifier: workspace:* version: link:../remix-server-runtime @@ -1217,17 +1217,17 @@ importers: packages/remix-react: dependencies: '@remix-run/router': - specifier: 1.16.0 - version: 1.16.0 + specifier: 1.16.1-pre.0 + version: 1.16.1-pre.0 '@remix-run/server-runtime': specifier: workspace:* version: link:../remix-server-runtime react-router: - specifier: 6.23.0 - version: 6.23.0(react@18.2.0) + specifier: 6.23.1-pre.0 + version: 6.23.1-pre.0(react@18.2.0) react-router-dom: - specifier: 6.23.0 - version: 6.23.0(react-dom@18.2.0)(react@18.2.0) + specifier: 6.23.1-pre.0 + version: 6.23.1-pre.0(react-dom@18.2.0)(react@18.2.0) turbo-stream: specifier: ^2.0.0 version: 2.0.0 @@ -1303,8 +1303,8 @@ importers: packages/remix-server-runtime: dependencies: '@remix-run/router': - specifier: 1.16.0 - version: 1.16.0 + specifier: 1.16.1-pre.0 + version: 1.16.1-pre.0 '@types/cookie': specifier: ^0.6.0 version: 0.6.0 @@ -1340,11 +1340,11 @@ importers: specifier: workspace:* version: link:../remix-react '@remix-run/router': - specifier: 1.16.0 - version: 1.16.0 + specifier: 1.16.1-pre.0 + version: 1.16.1-pre.0 react-router-dom: - specifier: 6.23.0 - version: 6.23.0(react-dom@18.2.0)(react@18.2.0) + specifier: 6.23.1-pre.0 + version: 6.23.1-pre.0(react-dom@18.2.0)(react@18.2.0) devDependencies: '@remix-run/server-runtime': specifier: workspace:* @@ -4199,8 +4199,8 @@ packages: - encoding dev: false - /@remix-run/router@1.16.0: - resolution: {integrity: sha512-Quz1KOffeEf/zwkCBM3kBtH4ZoZ+pT3xIXBG4PPW/XFtDP7EGhtTiC2+gpL9GnR7+Qdet5Oa6cYSvwKYg6kN9Q==} + /@remix-run/router@1.16.1-pre.0: + resolution: {integrity: sha512-RnLJQp2JRAQrMEu8a9SNKfzpOCdGjV/cHe6StKFb9iESRIlTdaaZdgqQo0TGXU1kjy2O4q42Cg2zVc+CnOM39g==} engines: {node: '>=14.0.0'} dev: false @@ -12816,26 +12816,26 @@ packages: engines: {node: '>=0.10.0'} dev: false - /react-router-dom@6.23.0(react-dom@18.2.0)(react@18.2.0): - resolution: {integrity: sha512-Q9YaSYvubwgbal2c9DJKfx6hTNoBp3iJDsl+Duva/DwxoJH+OTXkxGpql4iUK2sla/8z4RpjAm6EWx1qUDuopQ==} + /react-router-dom@6.23.1-pre.0(react-dom@18.2.0)(react@18.2.0): + resolution: {integrity: sha512-gXwu2McNyyuD+d2kC8TRqBGmfoYZO9sc7Egv8f/KQQc/wpPDdKOaq4BALX5oHwJZv142uSpfuE+bf8YsdGYFRg==} engines: {node: '>=14.0.0'} peerDependencies: react: '>=16.8' react-dom: '>=16.8' dependencies: - '@remix-run/router': 1.16.0 + '@remix-run/router': 1.16.1-pre.0 react: 18.2.0 react-dom: 18.2.0(react@18.2.0) - react-router: 6.23.0(react@18.2.0) + react-router: 6.23.1-pre.0(react@18.2.0) dev: false - /react-router@6.23.0(react@18.2.0): - resolution: {integrity: sha512-wPMZ8S2TuPadH0sF5irFGjkNLIcRvOSaEe7v+JER8508dyJumm6XZB1u5kztlX0RVq6AzRVndzqcUh6sFIauzA==} + /react-router@6.23.1-pre.0(react@18.2.0): + resolution: {integrity: sha512-bks0gSImPeGXVcfUVA/Xz210cuXci+3le+wCgIqIUy0EQaHrgy0desL7K8AvWom0yBkmBimIJ0kin36lkoI5RA==} engines: {node: '>=14.0.0'} peerDependencies: react: '>=16.8' dependencies: - '@remix-run/router': 1.16.0 + '@remix-run/router': 1.16.1-pre.0 react: 18.2.0 dev: false