From 3e093e513b0d49d7538b212fc977428f0f020c18 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 19 May 2023 14:18:35 -0400 Subject: [PATCH 01/29] Surface thrown response headers to ancestor boundaries (#6425) --- .changeset/expose-error-headers.md | 5 + integration/headers-test.ts | 219 +++++++++++++++++- packages/remix-server-runtime/headers.ts | 44 +++- packages/remix-server-runtime/routeModules.ts | 1 + 4 files changed, 259 insertions(+), 10 deletions(-) create mode 100644 .changeset/expose-error-headers.md diff --git a/.changeset/expose-error-headers.md b/.changeset/expose-error-headers.md new file mode 100644 index 00000000000..2637ed429eb --- /dev/null +++ b/.changeset/expose-error-headers.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": minor +--- + +Add `errorHeaders` parameter to the leaf `headers()` function to expose headers from thrown responses that bubble up to ancestor route boundaries. If the throwing route contains the boundary, then `errorHeaders` will be the same object as `loaderHeaders`/`actionHeaders` for that route. diff --git a/integration/headers-test.ts b/integration/headers-test.ts index 34d7ab6ba44..7521cd6a4a8 100644 --- a/integration/headers-test.ts +++ b/integration/headers-test.ts @@ -13,7 +13,7 @@ test.describe("headers export", () => { test.beforeAll(async () => { appFixture = await createFixture({ - future: { v2_routeConvention: true }, + future: { v2_routeConvention: true, v2_errorBoundary: true }, files: { "app/root.jsx": js` import { json } from "@remix-run/node"; @@ -78,6 +78,99 @@ test.describe("headers export", () => { export default function Action() { return
} `, + + "app/routes/parent.jsx": js` + export function headers({ actionHeaders, errorHeaders, loaderHeaders, parentHeaders }) { + return new Headers([ + ...(parentHeaders ? Array.from(parentHeaders.entries()) : []), + ...(actionHeaders ? Array.from(actionHeaders.entries()) : []), + ...(loaderHeaders ? Array.from(loaderHeaders.entries()) : []), + ...(errorHeaders ? Array.from(errorHeaders.entries()) : []), + ]); + } + + export function loader({ request }) { + if (new URL(request.url).searchParams.get('throw') === "parent") { + throw new Response(null, { + status: 400, + headers: { 'X-Parent-Loader': 'error' }, + }) + } + return new Response(null, { + headers: { 'X-Parent-Loader': 'success' }, + }) + } + + export async function action({ request }) { + let fd = await request.formData(); + if (fd.get('throw') === "parent") { + throw new Response(null, { + status: 400, + headers: { 'X-Parent-Action': 'error' }, + }) + } + return new Response(null, { + headers: { 'X-Parent-Action': 'success' }, + }) + } + + export default function Component() { return
} + + export function ErrorBoundary() { + return

Error!

+ } + `, + + "app/routes/parent.child.jsx": js` + export function headers({ actionHeaders, errorHeaders, loaderHeaders, parentHeaders }) { + return new Headers([ + ...(parentHeaders ? Array.from(parentHeaders.entries()) : []), + ...(actionHeaders ? Array.from(actionHeaders.entries()) : []), + ...(loaderHeaders ? Array.from(loaderHeaders.entries()) : []), + ...(errorHeaders ? Array.from(errorHeaders.entries()) : []), + ]); + } + + export function loader({ request }) { + if (new URL(request.url).searchParams.get('throw') === "child") { + throw new Response(null, { + status: 400, + headers: { 'X-Child-Loader': 'error' }, + }) + } + return new Response(null, { + headers: { 'X-Child-Loader': 'success' }, + }) + } + + export async function action({ request }) { + let fd = await request.formData(); + if (fd.get('throw') === "child") { + console.log('throwing from child action') + throw new Response(null, { + status: 400, + headers: { 'X-Child-Action': 'error' }, + }) + } + console.log('returning from child action') + return new Response(null, { + headers: { 'X-Child-Action': 'success' }, + }) + } + + export default function Component() { return
} + `, + + "app/routes/parent.child.grandchild.jsx": js` + export function loader({ request }) { + throw new Response(null, { + status: 400, + headers: { 'X-Child-Grandchild': 'error' }, + }) + } + + export default function Component() { return
} + `, }, }); }); @@ -147,4 +240,128 @@ test.describe("headers export", () => { let response = await fixture.requestDocument("/"); expect(response.headers.get(HEADER_KEY)).toBe(HEADER_VALUE); }); + + test("returns headers from successful /parent GET requests", async () => { + let response = await appFixture.requestDocument("/parent"); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["x-parent-loader", "success"], + ]) + ); + }); + + test("returns headers from successful /parent/child GET requests", async () => { + let response = await appFixture.requestDocument("/parent/child"); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["x-child-loader", "success"], + ["x-parent-loader", "success"], + ]) + ); + }); + + test("returns headers from successful /parent POST requests", async () => { + let response = await appFixture.postDocument( + "/parent", + new URLSearchParams() + ); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["x-parent-action", "success"], + ["x-parent-loader", "success"], + ]) + ); + }); + + test("returns headers from successful /parent/child POST requests", async () => { + let response = await appFixture.postDocument( + "/parent/child", + new URLSearchParams() + ); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["x-child-action", "success"], + ["x-child-loader", "success"], + ["x-parent-loader", "success"], + ]) + ); + }); + + test("returns headers from failed /parent GET requests", async () => { + let response = await appFixture.requestDocument("/parent?throw=parent"); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["x-parent-loader", "error, error"], // Shows up in loaderHeaders and errorHeaders + ]) + ); + }); + + test("returns bubbled headers from failed /parent/child GET requests", async () => { + let response = await appFixture.requestDocument( + "/parent/child?throw=child" + ); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["x-child-loader", "error"], + ["x-parent-loader", "success"], + ]) + ); + }); + + test("ignores headers from successful non-rendered loaders", async () => { + let response = await appFixture.requestDocument( + "/parent/child?throw=parent" + ); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["x-parent-loader", "error, error"], // Shows up in loaderHeaders and errorHeaders + ]) + ); + }); + + test("chooses higher thrown errors when multiple loaders throw", async () => { + let response = await appFixture.requestDocument( + "/parent/child/grandchild?throw=child" + ); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["x-child-loader", "error"], + ["x-parent-loader", "success"], + ]) + ); + }); + + test("returns headers from failed /parent POST requests", async () => { + let response = await appFixture.postDocument( + "/parent?throw=parent", + new URLSearchParams("throw=parent") + ); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["x-parent-action", "error, error"], // Shows up in actionHeaders and errorHeaders + ]) + ); + }); + + test("returns bubbled headers from failed /parent/child POST requests", async () => { + let response = await appFixture.postDocument( + "/parent/child", + new URLSearchParams("throw=child") + ); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["x-child-action", "error"], + ]) + ); + }); }); diff --git a/packages/remix-server-runtime/headers.ts b/packages/remix-server-runtime/headers.ts index c242b87c051..e13e65aee7c 100644 --- a/packages/remix-server-runtime/headers.ts +++ b/packages/remix-server-runtime/headers.ts @@ -7,14 +7,32 @@ export function getDocumentHeadersRR( build: ServerBuild, context: StaticHandlerContext ): Headers { - let matches = context.errors - ? context.matches.slice( - 0, - context.matches.findIndex((m) => context.errors![m.route.id]) + 1 - ) - : context.matches; - - return matches.reduce((parentHeaders, match) => { + let boundaryIdx = context.errors + ? context.matches.findIndex((m) => context.errors![m.route.id]) + : -1; + let matches = + boundaryIdx >= 0 + ? context.matches.slice(0, boundaryIdx + 1) + : context.matches; + + let errorHeaders: Headers | undefined; + + if (boundaryIdx >= 0) { + // Look for any errorHeaders from the boundary route down, which can be + // identified by the presence of headers but no data + let { actionHeaders, actionData, loaderHeaders, loaderData } = context; + context.matches.slice(boundaryIdx).some((match) => { + let id = match.route.id; + if (actionHeaders[id] && (!actionData || actionData[id] === undefined)) { + errorHeaders = actionHeaders[id]; + } else if (loaderHeaders[id] && loaderData[id] === undefined) { + errorHeaders = loaderHeaders[id]; + } + return errorHeaders != null; + }); + } + + return matches.reduce((parentHeaders, match, idx) => { let { id } = match.route; let routeModule = build.routes[id].module; let loaderHeaders = context.loaderHeaders[id] || new Headers(); @@ -22,7 +40,15 @@ export function getDocumentHeadersRR( let headers = new Headers( routeModule.headers ? typeof routeModule.headers === "function" - ? routeModule.headers({ loaderHeaders, parentHeaders, actionHeaders }) + ? routeModule.headers({ + loaderHeaders, + parentHeaders, + actionHeaders, + // Only expose errorHeaders to the leaf headers() function to + // avoid duplication via parentHeaders + errorHeaders: + idx === matches.length - 1 ? errorHeaders : undefined, + }) : routeModule.headers : undefined ); diff --git a/packages/remix-server-runtime/routeModules.ts b/packages/remix-server-runtime/routeModules.ts index 82794dfa523..0610550928e 100644 --- a/packages/remix-server-runtime/routeModules.ts +++ b/packages/remix-server-runtime/routeModules.ts @@ -70,6 +70,7 @@ export type HeadersArgs = { loaderHeaders: Headers; parentHeaders: Headers; actionHeaders: Headers; + errorHeaders: Headers | undefined; }; /** From 15d6a0d9e574bb3ea7e3eee0a3471f0ad1546bd2 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Mon, 22 May 2023 15:43:04 +1000 Subject: [PATCH 02/29] chore: update playground template CSS setup (#6447) --- scripts/playground/template/.gitignore | 2 -- scripts/playground/template/app/root.tsx | 6 +++++- scripts/playground/template/app/styles/tailwind.css | 3 +++ scripts/playground/template/package.json | 7 ++----- scripts/playground/template/remix.config.js | 1 + 5 files changed, 11 insertions(+), 8 deletions(-) create mode 100644 scripts/playground/template/app/styles/tailwind.css diff --git a/scripts/playground/template/.gitignore b/scripts/playground/template/.gitignore index 9c21620068c..8e3c3201b44 100644 --- a/scripts/playground/template/.gitignore +++ b/scripts/playground/template/.gitignore @@ -2,5 +2,3 @@ **/prisma/data.db **/prisma/data.db-journal - -**/app/styles/tailwind.css diff --git a/scripts/playground/template/app/root.tsx b/scripts/playground/template/app/root.tsx index a9d74ef8ab8..a9df87e5ceb 100644 --- a/scripts/playground/template/app/root.tsx +++ b/scripts/playground/template/app/root.tsx @@ -8,12 +8,16 @@ import { Scripts, ScrollRestoration, } from "@remix-run/react"; +import { cssBundleHref } from "@remix-run/css-bundle"; import tailwindStylesheetUrl from "./styles/tailwind.css"; import { getUser } from "./session.server"; export const links: LinksFunction = () => { - return [{ rel: "stylesheet", href: tailwindStylesheetUrl }]; + return [ + { rel: "stylesheet", href: tailwindStylesheetUrl }, + ...(cssBundleHref ? [{ rel: "stylesheet", href: cssBundleHref }] : []), + ]; }; export async function loader({ request }: LoaderArgs) { diff --git a/scripts/playground/template/app/styles/tailwind.css b/scripts/playground/template/app/styles/tailwind.css new file mode 100644 index 00000000000..b5c61c95671 --- /dev/null +++ b/scripts/playground/template/app/styles/tailwind.css @@ -0,0 +1,3 @@ +@tailwind base; +@tailwind components; +@tailwind utilities; diff --git a/scripts/playground/template/package.json b/scripts/playground/template/package.json index 99c88c0290e..619bb7db42b 100644 --- a/scripts/playground/template/package.json +++ b/scripts/playground/template/package.json @@ -3,14 +3,10 @@ "private": true, "sideEffects": false, "scripts": { - "build": "run-s \"build:*\"", - "build:css": "npm run generate:css -- --minify", - "build:remix": "node ./node_modules/@remix-run/dev/dist/cli build", + "build": "node ./node_modules/@remix-run/dev/dist/cli build", "dev": "run-p \"dev:*\"", - "dev:css": "cross-env NODE_ENV=development npm run generate:css -- --watch", "dev:remix": "cross-env NODE_ENV=development node ./node_modules/@remix-run/dev/dist/cli watch", "dev:server": "cross-env NODE_ENV=development node --inspect --require ./node_modules/dotenv/config ./server.js", - "generate:css": "tailwindcss -o ./app/styles/tailwind.css", "setup": "prisma generate && prisma migrate deploy && prisma db seed", "start": "cross-env NODE_ENV=production node --inspect --require ./node_modules/dotenv/config ./server.js" }, @@ -21,6 +17,7 @@ ], "dependencies": { "@prisma/client": "^3.15.2", + "@remix-run/css-bundle": "*", "@remix-run/express": "*", "@remix-run/node": "*", "@remix-run/react": "*", diff --git a/scripts/playground/template/remix.config.js b/scripts/playground/template/remix.config.js index d88ac009675..4cf3e14cb57 100644 --- a/scripts/playground/template/remix.config.js +++ b/scripts/playground/template/remix.config.js @@ -2,6 +2,7 @@ module.exports = { cacheDirectory: "./node_modules/.cache/remix", ignoredRouteFiles: ["**/.*", "**/*.css", "**/*.test.{js,jsx,ts,tsx}"], + tailwind: true, future: { v2_meta: true, }, From d56574b5969d0417629eb8aba24c310572c7a984 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Mon, 22 May 2023 11:20:06 -0400 Subject: [PATCH 03/29] feat: simplified types for useLoaderData, useActionData, useFetcher, SerializeFrom (#6449) --- .changeset/lemon-beers-fetch.md | 24 ++++++++++++++++++++++ packages/remix-server-runtime/serialize.ts | 8 ++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 .changeset/lemon-beers-fetch.md diff --git a/.changeset/lemon-beers-fetch.md b/.changeset/lemon-beers-fetch.md new file mode 100644 index 00000000000..0e9c3032f41 --- /dev/null +++ b/.changeset/lemon-beers-fetch.md @@ -0,0 +1,24 @@ +--- +"@remix-run/server-runtime": minor +"@remix-run/react": minor +--- + +Force Typescript to simplify type produced by `Serialize`. + +As a result, the following types and functions have simplified return types: +- SerializeFrom +- useLoaderData +- useActionData +- useFetcher + +```ts +type Data = { hello: string; when: Date } + +// BEFORE +type Unsimplified = SerializeFrom +// ^? SerializeObject> + +// AFTER +type Simplified = SerializeFrom +// ^? { hello: string; when: string } +``` diff --git a/packages/remix-server-runtime/serialize.ts b/packages/remix-server-runtime/serialize.ts index d11cbeb169e..d418bc07288 100644 --- a/packages/remix-server-runtime/serialize.ts +++ b/packages/remix-server-runtime/serialize.ts @@ -1,6 +1,9 @@ import type { AppData } from "./data"; import type { TypedDeferredData, TypedResponse } from "./responses"; +// force Typescript to simplify the type +type Pretty = { [K in keyof T]: T[K] } & {}; + type JsonPrimitive = | string | number @@ -18,7 +21,7 @@ type NonJsonPrimitive = undefined | Function | symbol; type IsAny = 0 extends 1 & T ? true : false; // prettier-ignore -type Serialize = +type Serialize = Pretty< IsAny extends true ? any : T extends TypedDeferredData ? SerializeDeferred : T extends JsonPrimitive ? T : @@ -28,7 +31,8 @@ type Serialize = T extends [unknown, ...unknown[]] ? SerializeTuple : T extends ReadonlyArray ? (U extends NonJsonPrimitive ? null : Serialize)[] : T extends object ? SerializeObject> : - never; + never +>; /** JSON serialize [tuples](https://www.typescriptlang.org/docs/handbook/2/objects.html#tuple-types) */ type SerializeTuple = { From 8a50a2a25e7acc9a75c407ef4e43add4fc294751 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 23 May 2023 13:16:49 -0400 Subject: [PATCH 04/29] Properly handle ErrorResponse inside resource routes (#6320) --- .changeset/error-response-resource-route.md | 5 +++ integration/resource-routes-test.ts | 35 ++++++++++++++++ packages/remix-server-runtime/server.ts | 46 ++++++++++++++++----- 3 files changed, 75 insertions(+), 11 deletions(-) create mode 100644 .changeset/error-response-resource-route.md diff --git a/.changeset/error-response-resource-route.md b/.changeset/error-response-resource-route.md new file mode 100644 index 00000000000..7dd7264c40a --- /dev/null +++ b/.changeset/error-response-resource-route.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": patch +--- + +Properly handle thrown `ErrorResponse` instances inside resource routes diff --git a/integration/resource-routes-test.ts b/integration/resource-routes-test.ts index 5307d471113..82d1218e271 100644 --- a/integration/resource-routes-test.ts +++ b/integration/resource-routes-test.ts @@ -8,10 +8,13 @@ import { PlaywrightFixture } from "./helpers/playwright-fixture"; test.describe("loader in an app", async () => { let appFixture: AppFixture; let fixture: Fixture; + let _consoleError: typeof console.error; let SVG_CONTENTS = ``; test.beforeAll(async () => { + _consoleError = console.error; + console.error = () => {}; fixture = await createFixture({ future: { v2_routeConvention: true }, files: { @@ -25,6 +28,9 @@ test.describe("loader in an app", async () => { +
+ +
) `, @@ -94,6 +100,12 @@ test.describe("loader in an app", async () => { throw { but: 'why' }; } `, + "app/routes/no-action.jsx": js` + import { json } from "@remix-run/node"; + export let loader = () => { + return json({ ok: true }); + } + `, }, }); appFixture = await createAppFixture(fixture, ServerMode.Test); @@ -101,6 +113,7 @@ test.describe("loader in an app", async () => { test.afterAll(() => { appFixture.close(); + console.error = _consoleError; }); test.describe("with JavaScript", () => { @@ -190,6 +203,28 @@ test.describe("loader in an app", async () => { "Unexpected Server Error\n\n[object Object]" ); }); + + test("should handle ErrorResponses thrown from resource routes on document requests", async () => { + let res = await fixture.postDocument("/no-action", new FormData()); + expect(res.status).toBe(405); + expect(res.statusText).toBe("Method Not Allowed"); + expect(await res.text()).toBe('{"message":"Unexpected Server Error"}'); + }); + + test("should handle ErrorResponses thrown from resource routes on client submissions", async ({ + page, + }) => { + let logs: string[] = []; + page.on("console", (msg) => logs.push(msg.text())); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + await app.clickSubmitButton("/no-action"); + let html = await app.getHtml(); + expect(html).toMatch("Application Error"); + expect(logs[0]).toContain( + 'Route "routes/no-action" does not have an action' + ); + }); }); test.describe("Development server", async () => { diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 97fa3a420e7..7bc58a54761 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -1,5 +1,6 @@ import type { UNSAFE_DeferredData as DeferredData, + ErrorResponse, StaticHandler, StaticHandlerContext, } from "@remix-run/router"; @@ -8,6 +9,7 @@ import { getStaticContextFromError, isRouteErrorResponse, createStaticHandler, + json as routerJson, } from "@remix-run/router"; import type { AppLoadContext } from "./data"; @@ -23,7 +25,6 @@ import type { ServerRouteManifest } from "./routes"; import { createStaticHandlerDataRoutes, createRoutes } from "./routes"; import { createDeferredReadableStream, - json, isRedirectResponse, isResponse, } from "./responses"; @@ -158,18 +159,16 @@ async function handleDataRequestRR( return error; } - let status = isRouteErrorResponse(error) ? error.status : 500; - let errorInstance = - isRouteErrorResponse(error) && error.error - ? error.error - : error instanceof Error - ? error - : new Error("Unexpected Server Error"); + if (isRouteErrorResponse(error)) { + logServerErrorIfNotAborted(error.error || error, request, serverMode); + return errorResponseToJson(error, serverMode); + } + let errorInstance = + error instanceof Error ? error : new Error("Unexpected Server Error"); logServerErrorIfNotAborted(errorInstance, request, serverMode); - - return json(serializeError(errorInstance, serverMode), { - status, + return routerJson(serializeError(errorInstance, serverMode), { + status: 500, headers: { "X-Remix-Error": "yes", }, @@ -359,6 +358,12 @@ async function handleResourceRequestRR( error.headers.set("X-Remix-Catch", "yes"); return error; } + + if (isRouteErrorResponse(error)) { + logServerErrorIfNotAborted(error.error || error, request, serverMode); + return errorResponseToJson(error, serverMode); + } + logServerErrorIfNotAborted(error, request, serverMode); return returnLastResortErrorResponse(error, serverMode); } @@ -374,6 +379,25 @@ function logServerErrorIfNotAborted( } } +function errorResponseToJson( + errorResponse: ErrorResponse, + serverMode: ServerMode +): Response { + return routerJson( + serializeError( + errorResponse.error || new Error("Unexpected Server Error"), + serverMode + ), + { + status: errorResponse.status, + statusText: errorResponse.statusText, + headers: { + "X-Remix-Error": "yes", + }, + } + ); +} + function returnLastResortErrorResponse(error: any, serverMode?: ServerMode) { let message = "Unexpected Server Error"; From 8dbbbdc1b2b5be717894ffb78b193aef94a2b64c Mon Sep 17 00:00:00 2001 From: Remix Run Bot Date: Tue, 23 May 2023 19:21:35 +0000 Subject: [PATCH 05/29] chore: format --- .changeset/expose-error-headers.md | 2 +- .changeset/headers-args-type.md | 6 +++--- .changeset/lemon-beers-fetch.md | 7 ++++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.changeset/expose-error-headers.md b/.changeset/expose-error-headers.md index 2637ed429eb..0d49a2519bc 100644 --- a/.changeset/expose-error-headers.md +++ b/.changeset/expose-error-headers.md @@ -2,4 +2,4 @@ "@remix-run/server-runtime": minor --- -Add `errorHeaders` parameter to the leaf `headers()` function to expose headers from thrown responses that bubble up to ancestor route boundaries. If the throwing route contains the boundary, then `errorHeaders` will be the same object as `loaderHeaders`/`actionHeaders` for that route. +Add `errorHeaders` parameter to the leaf `headers()` function to expose headers from thrown responses that bubble up to ancestor route boundaries. If the throwing route contains the boundary, then `errorHeaders` will be the same object as `loaderHeaders`/`actionHeaders` for that route. diff --git a/.changeset/headers-args-type.md b/.changeset/headers-args-type.md index c9133fc4575..16456a5d3ca 100644 --- a/.changeset/headers-args-type.md +++ b/.changeset/headers-args-type.md @@ -12,11 +12,11 @@ Add `HeadersArgs` type to be consistent with loaders/actions/meta and allows for using a `function` declaration in addition to an arrow function expression ```tsx -import type { HeadersArgs } from '@remix-run/node'; // or cloudflare/deno +import type { HeadersArgs } from "@remix-run/node"; // or cloudflare/deno export function headers({ loaderHeaders }: HeadersArgs) { return { - "x-my-custom-thing": loaderHeaders.get("x-my-custom-thing") || "fallback" - } + "x-my-custom-thing": loaderHeaders.get("x-my-custom-thing") || "fallback", + }; } ``` diff --git a/.changeset/lemon-beers-fetch.md b/.changeset/lemon-beers-fetch.md index 0e9c3032f41..83458a2fc8c 100644 --- a/.changeset/lemon-beers-fetch.md +++ b/.changeset/lemon-beers-fetch.md @@ -6,19 +6,20 @@ Force Typescript to simplify type produced by `Serialize`. As a result, the following types and functions have simplified return types: + - SerializeFrom - useLoaderData - useActionData - useFetcher ```ts -type Data = { hello: string; when: Date } +type Data = { hello: string; when: Date }; // BEFORE -type Unsimplified = SerializeFrom +type Unsimplified = SerializeFrom; // ^? SerializeObject> // AFTER -type Simplified = SerializeFrom +type Simplified = SerializeFrom; // ^? { hello: string; when: string } ``` From f717d02e06b713a745969b2abea0a888965297bd Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Tue, 23 May 2023 18:16:02 -0400 Subject: [PATCH 06/29] perf(dev): faster server export removal for routes (#6455) --- .changeset/great-geese-compare.md | 9 +++++ packages/remix-dev/compiler/js/compiler.ts | 36 ++++++------------- .../compiler/plugins/cssSideEffectImports.ts | 30 +++++++++------- packages/remix-react/components.tsx | 11 ++---- packages/remix-server-runtime/serialize.ts | 6 +++- 5 files changed, 45 insertions(+), 47 deletions(-) create mode 100644 .changeset/great-geese-compare.md diff --git a/.changeset/great-geese-compare.md b/.changeset/great-geese-compare.md new file mode 100644 index 00000000000..bfc0cf917cd --- /dev/null +++ b/.changeset/great-geese-compare.md @@ -0,0 +1,9 @@ +--- +"@remix-run/react": minor +"@remix-run/dev": patch +--- + +Faster server export removal for routes when `unstable_dev` is enabled. + +Also, only render modulepreloads on SSR. +Do not render modulepreloads when hydrated. diff --git a/packages/remix-dev/compiler/js/compiler.ts b/packages/remix-dev/compiler/js/compiler.ts index d55e9ff98f8..7cc89714ff8 100644 --- a/packages/remix-dev/compiler/js/compiler.ts +++ b/packages/remix-dev/compiler/js/compiler.ts @@ -8,7 +8,6 @@ import { type Manifest } from "../../manifest"; import { getAppDependencies } from "../../dependencies"; import { loaders } from "../utils/loaders"; import { browserRouteModulesPlugin } from "./plugins/routes"; -import { browserRouteModulesPlugin as browserRouteModulesPlugin_v2 } from "./plugins/routes_unstable"; import { cssFilePlugin } from "../plugins/cssImports"; import { absoluteCssUrlsPlugin } from "../plugins/absoluteCssUrlsPlugin"; import { deprecatedRemixPackagePlugin } from "../plugins/deprecatedRemixPackage"; @@ -78,29 +77,12 @@ const createEsbuildConfig = ( "entry.client": ctx.config.entryClientFilePath, }; - let routeModulePaths = new Map(); for (let id of Object.keys(ctx.config.routes)) { entryPoints[id] = ctx.config.routes[id].file; - if (ctx.config.future.unstable_dev) { - // In V2 we are doing AST transforms to remove server code, this means we - // have to re-map all route modules back to the same module in the graph - // otherwise we will have duplicate modules in the graph. We have to resolve - // the path as we get the relative for the entrypoint and absolute for imports - // from other modules. - routeModulePaths.set( - ctx.config.routes[id].file, - ctx.config.routes[id].file - ); - routeModulePaths.set( - path.resolve(ctx.config.appDirectory, ctx.config.routes[id].file), - ctx.config.routes[id].file - ); - } else { - // All route entry points are virtual modules that will be loaded by the - // browserEntryPointsPlugin. This allows us to tree-shake server-only code - // that we don't want to run in the browser (i.e. action & loader). - entryPoints[id] += "?browser"; - } + // All route entry points are virtual modules that will be loaded by the + // browserEntryPointsPlugin. This allows us to tree-shake server-only code + // that we don't want to run in the browser (i.e. action & loader). + entryPoints[id] += "?browser"; } if ( @@ -133,16 +115,18 @@ const createEsbuildConfig = ( } let plugins: esbuild.Plugin[] = [ + browserRouteModulesPlugin(ctx, /\?browser$/), deprecatedRemixPackagePlugin(ctx), cssModulesPlugin(ctx, { outputCss: false }), vanillaExtractPlugin(ctx, { outputCss: false }), - cssSideEffectImportsPlugin(ctx), + cssSideEffectImportsPlugin(ctx, { + hmr: + ctx.options.mode === "development" && + ctx.config.future.unstable_dev !== false, + }), cssFilePlugin(ctx), absoluteCssUrlsPlugin(), externalPlugin(/^https?:\/\//, { sideEffects: false }), - ctx.config.future.unstable_dev - ? browserRouteModulesPlugin_v2(ctx, routeModulePaths) - : browserRouteModulesPlugin(ctx, /\?browser$/), mdxPlugin(ctx), emptyModulesPlugin(ctx, /\.server(\.[jt]sx?)?$/), NodeModulesPolyfillPlugin(), diff --git a/packages/remix-dev/compiler/plugins/cssSideEffectImports.ts b/packages/remix-dev/compiler/plugins/cssSideEffectImports.ts index 9478841b51f..aa4862c180d 100644 --- a/packages/remix-dev/compiler/plugins/cssSideEffectImports.ts +++ b/packages/remix-dev/compiler/plugins/cssSideEffectImports.ts @@ -6,9 +6,9 @@ import { parse, type ParserOptions } from "@babel/parser"; import traverse from "@babel/traverse"; import generate from "@babel/generator"; -import type { RemixConfig } from "../../config"; -import type { Options } from "../options"; import { getPostcssProcessor } from "../utils/postcss"; +import { applyHMR } from "../js/plugins/hmr"; +import type { Context } from "../context"; const pluginName = "css-side-effects-plugin"; const namespace = `${pluginName}-ns`; @@ -44,17 +44,14 @@ const loaderForExtension: Record = { * to the CSS bundle. This is primarily designed to support packages that * import plain CSS files directly within JS files. */ -export const cssSideEffectImportsPlugin = ({ - config, - options, -}: { - config: RemixConfig; - options: Options; -}): Plugin => { +export const cssSideEffectImportsPlugin = ( + ctx: Context, + { hmr = false } = {} +): Plugin => { return { name: pluginName, setup: async (build) => { - let postcssProcessor = await getPostcssProcessor({ config }); + let postcssProcessor = await getPostcssProcessor(ctx); build.onLoad( { filter: allJsFilesFilter, namespace: "file" }, @@ -69,6 +66,15 @@ export const cssSideEffectImportsPlugin = ({ let loader = loaderForExtension[path.extname(args.path) as Extension]; let contents = addSuffixToCssSideEffectImports(loader, code); + if (hmr) { + contents = await applyHMR( + contents, + args, + ctx.config, + !!build.initialOptions.sourcemap + ); + } + return { contents, loader, @@ -94,7 +100,7 @@ export const cssSideEffectImportsPlugin = ({ } return { - path: path.relative(config.rootDirectory, resolvedPath), + path: path.relative(ctx.config.rootDirectory, resolvedPath), namespace, }; } @@ -108,7 +114,7 @@ export const cssSideEffectImportsPlugin = ({ await postcssProcessor.process(contents, { from: args.path, to: args.path, - map: options.sourcemap, + map: ctx.options.sourcemap, }) ).css; } diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 89fa1b7e9ab..cd5fbf8e8a9 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -1087,13 +1087,8 @@ import(${JSON.stringify(manifest.entry.module)});`; let preloads = isHydrated ? [] : manifest.entry.imports.concat(routePreloads); - return ( + return isHydrated ? null : ( <> - ))} - {!isHydrated && initialScripts} - {!isHydrated && deferredScripts} + {initialScripts} + {deferredScripts} ); } diff --git a/packages/remix-server-runtime/serialize.ts b/packages/remix-server-runtime/serialize.ts index d418bc07288..7e2f9c98f71 100644 --- a/packages/remix-server-runtime/serialize.ts +++ b/packages/remix-server-runtime/serialize.ts @@ -46,7 +46,11 @@ type SerializeObject = { // prettier-ignore type SerializeDeferred> = { - [k in keyof T as T[k] extends Promise ? k : T[k] extends NonJsonPrimitive ? never : k]: + [k in keyof T as + T[k] extends Promise ? k : + T[k] extends NonJsonPrimitive ? never : + k + ]: T[k] extends Promise ? Promise> extends never ? "wtf" : Promise> : Serialize extends never ? k : Serialize; From c90c16e7e2ffc08618b3ca85c079b8d1353a9315 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Wed, 24 May 2023 11:24:59 +1000 Subject: [PATCH 07/29] fix: detect Tailwind in PostCSS plugins object (#6468) --- .changeset/gorgeous-fireants-fry.md | 7 +++++++ packages/remix-dev/compiler/utils/postcss.ts | 11 ++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 .changeset/gorgeous-fireants-fry.md diff --git a/.changeset/gorgeous-fireants-fry.md b/.changeset/gorgeous-fireants-fry.md new file mode 100644 index 00000000000..36fe7fb631c --- /dev/null +++ b/.changeset/gorgeous-fireants-fry.md @@ -0,0 +1,7 @@ +--- +"@remix-run/dev": patch +--- + +Fix Tailwind performance issue when `postcss.config.js` contains `plugins: { tailwindcss: {} }` and `remix.config.js` contains both `tailwind: true` and `postcss: true`. + +Note that this was _not_ an issue when the plugin function had been explicitly called, i.e. `plugins: [tailwindcss()]`. Remix avoids adding the Tailwind plugin to PostCSS if it's already present but we were failing to detect when the plugin function hadn't been called — either because the plugin function itself had been passed, i.e. `plugins: [require('tailwindcss')]`, or the plugin config object syntax had been used, i.e. `plugins: { tailwindcss: {} }`. diff --git a/packages/remix-dev/compiler/utils/postcss.ts b/packages/remix-dev/compiler/utils/postcss.ts index bf27ae4377b..8efe0cac24f 100644 --- a/packages/remix-dev/compiler/utils/postcss.ts +++ b/packages/remix-dev/compiler/utils/postcss.ts @@ -63,7 +63,7 @@ export async function loadPostcssPlugins({ if (config.tailwind) { let tailwindPlugin = await loadTailwindPlugin(config); - if (tailwindPlugin && !hasTailwindPlugin(plugins)) { + if (tailwindPlugin && !hasTailwindPlugin(plugins, tailwindPlugin)) { plugins.push(tailwindPlugin); } } @@ -94,10 +94,15 @@ export async function getPostcssProcessor({ return processor; } -function hasTailwindPlugin(plugins: Array) { +function hasTailwindPlugin( + plugins: Array, + tailwindPlugin: AcceptedPlugin +) { return plugins.some( (plugin) => - "postcssPlugin" in plugin && plugin.postcssPlugin === "tailwindcss" + plugin === tailwindPlugin || + (typeof plugin === "function" && plugin.name === "tailwindcss") || + ("postcssPlugin" in plugin && plugin.postcssPlugin === "tailwindcss") ); } From 9efdaea44893210ce5b062f3cb3d126a59ad0fbd Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Wed, 24 May 2023 11:58:26 -0400 Subject: [PATCH 08/29] fix(dev): gracefully handle hdr errors (#6467) --- .changeset/strong-dolphins-tell.md | 5 ++ integration/hmr-log-test.ts | 43 ++++++++++++++ integration/hmr-test.ts | 43 ++++++++++++++ packages/remix-dev/devServer_unstable/hdr.ts | 23 ++++---- .../remix-dev/devServer_unstable/index.ts | 59 ++++++++++++------- 5 files changed, 141 insertions(+), 32 deletions(-) create mode 100644 .changeset/strong-dolphins-tell.md diff --git a/.changeset/strong-dolphins-tell.md b/.changeset/strong-dolphins-tell.md new file mode 100644 index 00000000000..3e109e2b71e --- /dev/null +++ b/.changeset/strong-dolphins-tell.md @@ -0,0 +1,5 @@ +--- +"@remix-run/dev": patch +--- + +fix dev server crashes caused by ungraceful hdr error handling diff --git a/integration/hmr-log-test.ts b/integration/hmr-log-test.ts index cdb02556ed4..d7aa1dfd672 100644 --- a/integration/hmr-log-test.ts +++ b/integration/hmr-log-test.ts @@ -463,6 +463,49 @@ whatsup fs.writeFileSync(mdxPath, originalMdx); await page.waitForSelector(`#crazy`); expect(dataRequests).toBe(5); + + // dev server doesn't crash when rebuild fails + await page.click(`a[href="/"]`); + await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); + await page.waitForLoadState("networkidle"); + + expect(devStderr()).toBe(""); + let withSyntaxError = ` + import { useLoaderData } from "@remix-run/react"; + export function shouldRevalidate(args) { + return true; + } + eport efault functio Index() { + const t = useLoaderData(); + return ( + +

With Syntax Error

+ + ) + } + `; + fs.writeFileSync(indexPath, withSyntaxError); + await wait(() => devStderr().includes('Expected ";" but found "efault"'), { + timeoutMs: HMR_TIMEOUT_MS, + }); + + let withFix = ` + import { useLoaderData } from "@remix-run/react"; + export function shouldRevalidate(args) { + return true; + } + export default function Index() { + // const t = useLoaderData(); + return ( +
+

With Fix

+
+ ) + } + `; + fs.writeFileSync(indexPath, withFix); + await page.waitForLoadState("networkidle"); + await page.getByText("With Fix").waitFor({ timeout: HMR_TIMEOUT_MS }); } catch (e) { console.log("stdout begin -----------------------"); console.log(devStdout()); diff --git a/integration/hmr-test.ts b/integration/hmr-test.ts index b3c418906e7..e369070b9d0 100644 --- a/integration/hmr-test.ts +++ b/integration/hmr-test.ts @@ -462,6 +462,49 @@ whatsup fs.writeFileSync(mdxPath, originalMdx); await page.waitForSelector(`#crazy`); expect(dataRequests).toBe(5); + + // dev server doesn't crash when rebuild fails + await page.click(`a[href="/"]`); + await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); + await page.waitForLoadState("networkidle"); + + expect(devStderr()).toBe(""); + let withSyntaxError = ` + import { useLoaderData } from "@remix-run/react"; + export function shouldRevalidate(args) { + return true; + } + eport efault functio Index() { + const t = useLoaderData(); + return ( + +

With Syntax Error

+ + ) + } + `; + fs.writeFileSync(indexPath, withSyntaxError); + await wait(() => devStderr().includes('Expected ";" but found "efault"'), { + timeoutMs: HMR_TIMEOUT_MS, + }); + + let withFix = ` + import { useLoaderData } from "@remix-run/react"; + export function shouldRevalidate(args) { + return true; + } + export default function Index() { + // const t = useLoaderData(); + return ( +
+

With Fix

+
+ ) + } + `; + fs.writeFileSync(indexPath, withFix); + await page.waitForLoadState("networkidle"); + await page.getByText("With Fix").waitFor({ timeout: HMR_TIMEOUT_MS }); } catch (e) { console.log("stdout begin -----------------------"); console.log(devStdout()); diff --git a/packages/remix-dev/devServer_unstable/hdr.ts b/packages/remix-dev/devServer_unstable/hdr.ts index bacb6475338..69e859b2493 100644 --- a/packages/remix-dev/devServer_unstable/hdr.ts +++ b/packages/remix-dev/devServer_unstable/hdr.ts @@ -16,7 +16,9 @@ function isBareModuleId(id: string): boolean { type Route = Context["config"]["routes"][string]; -export let detectLoaderChanges = async (ctx: Context) => { +export let detectLoaderChanges = async ( + ctx: Context +): Promise> => { let entryPoints: Record = {}; for (let id of Object.keys(ctx.config.routes)) { entryPoints[id] = ctx.config.routes[id].file + "?loader"; @@ -30,6 +32,7 @@ export let detectLoaderChanges = async (ctx: Context) => { write: false, entryNames: "[hash]", loader: loaders, + logLevel: "silent", plugins: [ { name: "hmr-loader", @@ -98,13 +101,13 @@ export let detectLoaderChanges = async (ctx: Context) => { }; let { metafile } = await esbuild.build(options); - let entries = Object.entries(metafile!.outputs).map( - ([hashjs, { entryPoint }]) => { - let file = entryPoint - ?.replace(/^hmr-loader:/, "") - ?.replace(/\?loader$/, ""); - return [file, hashjs.replace(/\.js$/, "")]; - } - ); - return Object.fromEntries(entries); + + let entries: Record = {}; + for (let [hashjs, { entryPoint }] of Object.entries(metafile!.outputs)) { + if (entryPoint === undefined) continue; + let file = entryPoint.replace(/^hmr-loader:/, "").replace(/\?loader$/, ""); + entries[file] = hashjs.replace(/\.js$/, ""); + } + + return entries; }; diff --git a/packages/remix-dev/devServer_unstable/index.ts b/packages/remix-dev/devServer_unstable/index.ts index c3d0e0a2fb5..e4492d350a9 100644 --- a/packages/remix-dev/devServer_unstable/index.ts +++ b/packages/remix-dev/devServer_unstable/index.ts @@ -15,6 +15,8 @@ import * as HMR from "./hmr"; import { warnOnce } from "../warnOnce"; import { detectPackageManager } from "../cli/detectPackageManager"; import * as HDR from "./hdr"; +import type { Result } from "../result"; +import { err, ok } from "../result"; type Origin = { scheme: string; @@ -59,7 +61,7 @@ export let serve = async ( manifest?: Manifest; prevManifest?: Manifest; appReady?: Channel.Type; - hdr?: Promise>; + loaderChanges?: Promise>>; prevLoaderHashes?: Record; } = {}; @@ -122,57 +124,70 @@ export let serve = async ( }, { onBuildStart: async (ctx) => { + // stop listening for previous manifest state.appReady?.err(); + clean(ctx.config); websocket.log(state.prevManifest ? "Rebuilding..." : "Building..."); - state.hdr = HDR.detectLoaderChanges(ctx); + state.loaderChanges = HDR.detectLoaderChanges(ctx).then(ok, err); }, onBuildManifest: (manifest: Manifest) => { state.manifest = manifest; + state.appReady = Channel.create(); }, onBuildFinish: async (ctx, durationMs, succeeded) => { if (!succeeded) return; - websocket.log( (state.prevManifest ? "Rebuilt" : "Built") + ` in ${prettyMs(durationMs)}` ); - state.appReady = Channel.create(); - let start = Date.now(); - console.log(`Waiting for app server (${state.manifest?.version})`); - if ( - options.command && - (state.appServer === undefined || options.restart) - ) { - await kill(state.appServer); - state.appServer = startAppServer(options.command); - } - let { ok } = await state.appReady.result; - // result not ok -> new build started before this one finished. do not process outdated manifest - let loaderHashes = await state.hdr; - if (ok) { + // accumulate new state, but only update state after updates are processed + let newState: typeof state = { prevManifest: state.manifest }; + try { + console.log(`Waiting for app server (${state.manifest?.version})`); + let start = Date.now(); + if ( + options.command && + (state.appServer === undefined || options.restart) + ) { + await kill(state.appServer); + state.appServer = startAppServer(options.command); + } + let appReady = await state.appReady!.result; + if (!appReady.ok) return; console.log(`App server took ${prettyMs(Date.now() - start)}`); - if (state.manifest && loaderHashes && state.prevManifest) { + + // HMR + HDR + let loaderChanges = await state.loaderChanges!; + if (loaderChanges.ok) { + newState.prevLoaderHashes = loaderChanges.value; + } + if (loaderChanges?.ok && state.manifest && state.prevManifest) { let updates = HMR.updates( ctx.config, state.manifest, state.prevManifest, - loaderHashes, + loaderChanges.value, state.prevLoaderHashes ); websocket.hmr(state.manifest, updates); let hdr = updates.some((u) => u.revalidate); console.log("> HMR" + (hdr ? " + HDR" : "")); - } else if (state.prevManifest !== undefined) { + return; + } + + // Live Reload + if (state.prevManifest !== undefined) { websocket.reload(); console.log("> Live reload"); } + } finally { + // commit accumulated state + Object.assign(state, newState); } - state.prevManifest = state.manifest; - state.prevLoaderHashes = loaderHashes; }, onFileCreated: (file) => websocket.log(`File created: ${relativePath(file)}`), From 933a4c9901db233d5da1629f1d40dedf698876de Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 May 2023 13:03:51 -0400 Subject: [PATCH 09/29] feat(server-runtime): v2_headers future flag for inheriting ancestor headers (#6431) Co-authored-by: Scott Kyle --- .changeset/v2-headers.md | 5 + docs/pages/api-development-strategy.md | 1 + docs/pages/v2.md | 8 ++ docs/route/headers.md | 33 +++++- integration/headers-test.ts | 109 ++++++++++++++---- packages/remix-dev/__tests__/create-test.ts | 3 + .../remix-dev/__tests__/readConfig-test.ts | 2 + packages/remix-dev/config.ts | 12 ++ packages/remix-react/entry.ts | 1 + packages/remix-server-runtime/entry.ts | 1 + packages/remix-server-runtime/headers.ts | 10 ++ packages/remix-testing/create-remix-stub.tsx | 1 + 12 files changed, 163 insertions(+), 23 deletions(-) create mode 100644 .changeset/v2-headers.md diff --git a/.changeset/v2-headers.md b/.changeset/v2-headers.md new file mode 100644 index 00000000000..31d7f5f4a67 --- /dev/null +++ b/.changeset/v2-headers.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": minor +--- + +Added a new `future.v2_headers` future flag to opt into automatic inheriting of ancestor route `headers` functions so you do not need to export a `headers` function from every possible leaf route if you don't wish to. diff --git a/docs/pages/api-development-strategy.md b/docs/pages/api-development-strategy.md index ff30a4830a6..152dd1c1900 100644 --- a/docs/pages/api-development-strategy.md +++ b/docs/pages/api-development-strategy.md @@ -56,6 +56,7 @@ Here's the current future flags in Remix v1 today: | ------------------------ | --------------------------------------------------------------------- | | `unstable_dev` | Enable the new development server (including HMR/HDR support) | | `v2_errorBoundary` | Combine `ErrorBoundary`/`CatchBoundary` into a single `ErrorBoundary` | +| `v2_headers` | Leverage ancestor `headers` if children do not export `headers` | | `v2_meta` | Enable the new API for your `meta` functions | | `v2_normalizeFormMethod` | Normalize `useNavigation().formMethod` to be an uppercase HTTP Method | | `v2_routeConvention` | Enable the flat routes style of file-based routing | diff --git a/docs/pages/v2.md b/docs/pages/v2.md index 8307a2c43ef..4435df72256 100644 --- a/docs/pages/v2.md +++ b/docs/pages/v2.md @@ -124,6 +124,14 @@ routes For more background on this change, see the [original "flat routes" proposal][flat-routes]. +## Route `headers` + +In Remix v2, the behavior for route `headers` functions is changing slightly. You can opt-into this new behavior ahead of time via the `future.v2_headers` flag in `remix.config.js`. + +In v1, Remix would only use the result of the leaf "rendered" route `headers` function. It was your responsibility to add a `headers` function to every potential leaf and merge in `parentHeaders` accordingly. This can get tedious quickly and is also easy to forget to add a `headers` function when you add a new route, even if you want it to just share the same headers from it's parent. + +In v2, Remix will use the deepest `headers` function that it finds in the rendered routes. This more easily allows you to share headers across routes from a common ancestor. Then as needed you can add `headers` functions to deeper routes if they require specific behavior. + ## Route `meta` In Remix v2, the signature for route `meta` functions and how Remix handles meta tags under the hood have changed. diff --git a/docs/route/headers.md b/docs/route/headers.md index b214b6fb03b..42a8972e4be 100644 --- a/docs/route/headers.md +++ b/docs/route/headers.md @@ -13,6 +13,7 @@ export const headers: HeadersFunction = ({ actionHeaders, loaderHeaders, parentHeaders, + errorHeaders, }) => ({ "X-Stretchy-Pants": "its for fun", "Cache-Control": "max-age=300, s-maxage=3600", @@ -33,7 +34,11 @@ export const headers: HeadersFunction = ({ Note: `actionHeaders` & `loaderHeaders` are an instance of the [Web Fetch API][headers] `Headers` class. -Because Remix has nested routes, there's a battle of the headers to be won when nested routes match. In this case, the deepest route wins. Consider these files in the routes directory: +If an action or a loader threw a `Response` and we're rendering a boundary, any headers from the thrown `Response` will be available in `errorHeaders`. This allows you to access headers from a child loader that threw in a parent error boundary. + +## Nested Routes + +Because Remix has nested routes, there's a battle of the headers to be won when nested routes match. The default behavior is that Remix only leverages the resulting headers from the leaf rendered route. Consider these files in the routes directory: ``` ├── users.tsx @@ -53,7 +58,11 @@ If we are looking at `/users/123/profile` then three routes are rendering: ``` -If all three define `headers`, the deepest module wins, in this case `profile.tsx`. +If all three define `headers`, the deepest module wins, in this case `profile.tsx`. However, if your `profile.tsx` loader threw and bubbled to a boundary in `userId.tsx` - then `userId.tsx`'s `headers` function would be used as it is the leaf rendered route. + + +We realize that it can be tedious and error-prone to have to define `headers` on every possible leaf route so we're changing the current behavior in v2 behind the [`future.v2_headers`][v2_headers] flag. + We don't want surprise headers in your responses, so it's your job to merge them if you'd like. Remix passes in the `parentHeaders` to your `headers` function. So `users.tsx` headers get passed to `$userId.tsx`, and then `$userId.tsx` headers are passed to `profile.tsx` headers. @@ -118,5 +127,25 @@ export default function handleRequest( Just keep in mind that doing this will apply to _all_ document requests, but does not apply to `data` requests (for client-side transitions for example). For those, use [`handleDataRequest`][handledatarequest]. +## v2 Behavior + +Since it can be tedious and error-prone to define a `header` function in every single possible leaf route, we're changing the behavior slightly in v2 and you can opt-into the new behavior via the `future.v2_headers` [Future Flag][future-flags] in `remix.config.js`. + +When enabling this flag, Remix will now use the deepest `headers` function it finds in the renderable matches (up to and including the boundary route if an error is present). You'll still need to handle merging together headers as shown above for any `headers` functions above this route. + +This means that, re-using the example above: + +``` +├── users.tsx +└── users + ├── $userId.tsx + └── $userId + └── profile.tsx +``` + +If a user is looking at `/users/123/profile` and `profile.tsx` does not export a `headers` function, then Remix will use the return value of `$userId.tsx`'s `headers` function. If that file doesn't export one, then it will use the result of the one in `users.tsx`, and so on. + [headers]: https://developer.mozilla.org/en-US/docs/Web/API/Headers [handledatarequest]: ../file-conventions/entry.server +[v2_headers]: #v2-behavior +[future-flags]: ../pages/api-development-strategy diff --git a/integration/headers-test.ts b/integration/headers-test.ts index 7521cd6a4a8..0bf306be55b 100644 --- a/integration/headers-test.ts +++ b/integration/headers-test.ts @@ -13,7 +13,11 @@ test.describe("headers export", () => { test.beforeAll(async () => { appFixture = await createFixture({ - future: { v2_routeConvention: true, v2_errorBoundary: true }, + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + v2_headers: true, + }, files: { "app/root.jsx": js` import { json } from "@remix-run/node"; @@ -122,15 +126,6 @@ test.describe("headers export", () => { `, "app/routes/parent.child.jsx": js` - export function headers({ actionHeaders, errorHeaders, loaderHeaders, parentHeaders }) { - return new Headers([ - ...(parentHeaders ? Array.from(parentHeaders.entries()) : []), - ...(actionHeaders ? Array.from(actionHeaders.entries()) : []), - ...(loaderHeaders ? Array.from(loaderHeaders.entries()) : []), - ...(errorHeaders ? Array.from(errorHeaders.entries()) : []), - ]); - } - export function loader({ request }) { if (new URL(request.url).searchParams.get('throw') === "child") { throw new Response(null, { @@ -138,24 +133,18 @@ test.describe("headers export", () => { headers: { 'X-Child-Loader': 'error' }, }) } - return new Response(null, { - headers: { 'X-Child-Loader': 'success' }, - }) + return null } export async function action({ request }) { let fd = await request.formData(); if (fd.get('throw') === "child") { - console.log('throwing from child action') throw new Response(null, { status: 400, headers: { 'X-Child-Action': 'error' }, }) } - console.log('returning from child action') - return new Response(null, { - headers: { 'X-Child-Action': 'success' }, - }) + return null } export default function Component() { return
} @@ -256,7 +245,6 @@ test.describe("headers export", () => { expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( JSON.stringify([ ["content-type", "text/html"], - ["x-child-loader", "success"], ["x-parent-loader", "success"], ]) ); @@ -284,8 +272,6 @@ test.describe("headers export", () => { expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( JSON.stringify([ ["content-type", "text/html"], - ["x-child-action", "success"], - ["x-child-loader", "success"], ["x-parent-loader", "success"], ]) ); @@ -365,3 +351,84 @@ test.describe("headers export", () => { ); }); }); + +test.describe("v1 behavior (future.v2_headers=false)", () => { + let appFixture: Fixture; + + test.beforeAll(async () => { + appFixture = await createFixture({ + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + v2_headers: false, + }, + files: { + "app/root.jsx": js` + import { json } from "@remix-run/node"; + import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; + + export const loader = () => json({}); + + export default function Root() { + return ( + + + + + + + + + + + ); + } + `, + + "app/routes/parent.jsx": js` + export function headers({ actionHeaders, errorHeaders, loaderHeaders, parentHeaders }) { + return new Headers([ + ...(parentHeaders ? Array.from(parentHeaders.entries()) : []), + ...(actionHeaders ? Array.from(actionHeaders.entries()) : []), + ...(loaderHeaders ? Array.from(loaderHeaders.entries()) : []), + ...(errorHeaders ? Array.from(errorHeaders.entries()) : []), + ]); + } + + export function loader({ request }) { + return new Response(null, { + headers: { 'X-Parent-Loader': 'success' }, + }) + } + + export default function Component() { return
} + `, + + "app/routes/parent.child.jsx": js` + export async function action({ request }) { + return null; + } + + export default function Component() { return
} + `, + }, + }); + }); + + test("returns no headers when the leaf route doesn't export a header function (GET)", async () => { + let response = await appFixture.requestDocument("/parent/child"); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([["content-type", "text/html"]]) + ); + }); + + test("returns no headers when the leaf route doesn't export a header function (POST)", async () => { + let response = await appFixture.postDocument( + "/parent/child", + new URLSearchParams() + ); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([["content-type", "text/html"]]) + ); + }); +}); diff --git a/packages/remix-dev/__tests__/create-test.ts b/packages/remix-dev/__tests__/create-test.ts index 3891972938f..15d6d7fcc59 100644 --- a/packages/remix-dev/__tests__/create-test.ts +++ b/packages/remix-dev/__tests__/create-test.ts @@ -12,6 +12,7 @@ import { errorBoundaryWarning, flatRoutesWarning, formMethodWarning, + headersWarning, metaWarning, serverModuleFormatWarning, } from "../config"; @@ -360,6 +361,8 @@ describe("the create command", () => { "\n" + metaWarning + "\n" + + headersWarning + + "\n" + serverModuleFormatWarning + "\n" + flatRoutesWarning + diff --git a/packages/remix-dev/__tests__/readConfig-test.ts b/packages/remix-dev/__tests__/readConfig-test.ts index e1f795537c7..3267b8717ef 100644 --- a/packages/remix-dev/__tests__/readConfig-test.ts +++ b/packages/remix-dev/__tests__/readConfig-test.ts @@ -34,6 +34,7 @@ describe("readConfig", () => { unstable_postcss: expect.any(Boolean), unstable_tailwind: expect.any(Boolean), v2_errorBoundary: expect.any(Boolean), + v2_headers: expect.any(Boolean), v2_meta: expect.any(Boolean), v2_normalizeFormMethod: expect.any(Boolean), v2_routeConvention: expect.any(Boolean), @@ -55,6 +56,7 @@ describe("readConfig", () => { "unstable_postcss": Any, "unstable_tailwind": Any, "v2_errorBoundary": Any, + "v2_headers": Any, "v2_meta": Any, "v2_normalizeFormMethod": Any, "v2_routeConvention": Any, diff --git a/packages/remix-dev/config.ts b/packages/remix-dev/config.ts index a795c10cc75..ed90d95a1f9 100644 --- a/packages/remix-dev/config.ts +++ b/packages/remix-dev/config.ts @@ -55,6 +55,7 @@ interface FutureConfig { /** @deprecated Use the `tailwind` config option instead */ unstable_tailwind: boolean; v2_errorBoundary: boolean; + v2_headers: boolean; v2_meta: boolean; v2_normalizeFormMethod: boolean; v2_routeConvention: boolean; @@ -442,6 +443,10 @@ export async function readConfig( warnOnce(metaWarning, "v2_meta"); } + if (!appConfig.future?.v2_headers) { + warnOnce(headersWarning, "v2_headers"); + } + let isCloudflareRuntime = ["cloudflare-pages", "cloudflare-workers"].includes( appConfig.serverBuildTarget ?? "" ); @@ -738,6 +743,7 @@ export async function readConfig( unstable_postcss: appConfig.future?.unstable_postcss === true, unstable_tailwind: appConfig.future?.unstable_tailwind === true, v2_errorBoundary: appConfig.future?.v2_errorBoundary === true, + v2_headers: appConfig.future?.v2_headers === true, v2_meta: appConfig.future?.v2_meta === true, v2_normalizeFormMethod: appConfig.future?.v2_normalizeFormMethod === true, v2_routeConvention: appConfig.future?.v2_routeConvention === true, @@ -928,3 +934,9 @@ export const metaWarning = "You can prepare for this change at your convenience with the `v2_meta` future flag. " + "For instructions on making this change see " + "https://remix.run/docs/en/v1.15.0/pages/v2#meta"; + +export const headersWarning = + "⚠️ REMIX FUTURE CHANGE: The route `headers` export behavior is changing in v2. " + + "You can prepare for this change at your convenience with the `v2_headers` future flag. " + + "For instructions on making this change see " + + "https://remix.run/docs/en/v1.17.0/pages/v2#route-headers"; diff --git a/packages/remix-react/entry.ts b/packages/remix-react/entry.ts index fde03eca196..d2a2ab2f269 100644 --- a/packages/remix-react/entry.ts +++ b/packages/remix-react/entry.ts @@ -33,6 +33,7 @@ export interface FutureConfig { /** @deprecated Use the `tailwind` config option instead */ unstable_tailwind: boolean; v2_errorBoundary: boolean; + v2_headers: boolean; v2_meta: boolean; v2_normalizeFormMethod: boolean; v2_routeConvention: boolean; diff --git a/packages/remix-server-runtime/entry.ts b/packages/remix-server-runtime/entry.ts index 1191e01ccfc..ea09f729b7c 100644 --- a/packages/remix-server-runtime/entry.ts +++ b/packages/remix-server-runtime/entry.ts @@ -25,6 +25,7 @@ export interface FutureConfig { /** @deprecated Use the `tailwind` config option instead */ unstable_tailwind: boolean; v2_errorBoundary: boolean; + v2_headers: boolean; v2_meta: boolean; v2_normalizeFormMethod: boolean; v2_routeConvention: boolean; diff --git a/packages/remix-server-runtime/headers.ts b/packages/remix-server-runtime/headers.ts index e13e65aee7c..98ac4853353 100644 --- a/packages/remix-server-runtime/headers.ts +++ b/packages/remix-server-runtime/headers.ts @@ -37,6 +37,16 @@ export function getDocumentHeadersRR( let routeModule = build.routes[id].module; let loaderHeaders = context.loaderHeaders[id] || new Headers(); let actionHeaders = context.actionHeaders[id] || new Headers(); + + // When the future flag is enabled, use the parent headers for any route + // that doesn't have a `headers` export + if (routeModule.headers == null && build.future.v2_headers) { + let headers = parentHeaders; + prependCookies(actionHeaders, headers); + prependCookies(loaderHeaders, headers); + return headers; + } + let headers = new Headers( routeModule.headers ? typeof routeModule.headers === "function" diff --git a/packages/remix-testing/create-remix-stub.tsx b/packages/remix-testing/create-remix-stub.tsx index 1ef4b85ada0..b7f0f464791 100644 --- a/packages/remix-testing/create-remix-stub.tsx +++ b/packages/remix-testing/create-remix-stub.tsx @@ -128,6 +128,7 @@ export function createRemixStub( unstable_postcss: false, unstable_tailwind: false, v2_errorBoundary: false, + v2_headers: false, v2_meta: false, v2_normalizeFormMethod: false, v2_routeConvention: false, From 5f808a483814066c6174d5492c0e7523a68c3db7 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 May 2023 14:03:44 -0400 Subject: [PATCH 10/29] fix(remix-dev): adjust conventional routes logic for pathless layout routes (#4421) --- .changeset/stale-spoons-tie.md | 8 + integration/conventional-routes-test.ts | 132 ++++++++ integration/flat-routes-test.ts | 129 ++++++++ integration/route-collisions-test.ts | 283 ++++++++++++++++++ .../remix-dev/__tests__/flat-routes-test.ts | 150 ++++++++++ packages/remix-dev/config/flat-routes.ts | 49 ++- packages/remix-dev/config/routesConvention.ts | 49 ++- 7 files changed, 796 insertions(+), 4 deletions(-) create mode 100644 .changeset/stale-spoons-tie.md create mode 100644 integration/conventional-routes-test.ts create mode 100644 integration/route-collisions-test.ts diff --git a/.changeset/stale-spoons-tie.md b/.changeset/stale-spoons-tie.md new file mode 100644 index 00000000000..ffe2da09933 --- /dev/null +++ b/.changeset/stale-spoons-tie.md @@ -0,0 +1,8 @@ +--- +"@remix-run/dev": patch +--- + +- Fix route ranking bug with pathless layout route next to a sibling index route + - Under the hood this is done by removing the trailing slash from all generated `path` values since the number of slash-delimited segments counts towards route ranking so the trailing slash incorrectly increases the score for routes + +- Support sibling pathless layout routes by removing pathless layout routes from the unique route path checks in conventional route generation since they inherently trigger duplicate paths \ No newline at end of file diff --git a/integration/conventional-routes-test.ts b/integration/conventional-routes-test.ts new file mode 100644 index 00000000000..665ad3516c1 --- /dev/null +++ b/integration/conventional-routes-test.ts @@ -0,0 +1,132 @@ +import { test, expect } from "@playwright/test"; + +import { PlaywrightFixture } from "./helpers/playwright-fixture"; +import type { Fixture, AppFixture } from "./helpers/create-fixture"; +import { createAppFixture, createFixture, js } from "./helpers/create-fixture"; + +let fixture: Fixture; +let appFixture: AppFixture; + +test.beforeAll(async () => { + fixture = await createFixture({ + files: { + "app/root.tsx": js` + import { Link, Outlet, Scripts, useMatches } from "@remix-run/react"; + + export default function App() { + let matches = 'Number of matches: ' + useMatches().length; + return ( + + + +

{matches}

+ + + + + ); + } + `, + "app/routes/nested/index.jsx": js` + export default function Index() { + return

Index

; + } + `, + "app/routes/nested/__pathless.jsx": js` + import { Outlet } from "@remix-run/react"; + + export default function Layout() { + return ( + <> +
Pathless Layout
+ + + ); + } + `, + "app/routes/nested/__pathless/foo.jsx": js` + export default function Foo() { + return

Foo

; + } + `, + "app/routes/nested/__pathless2.jsx": js` + import { Outlet } from "@remix-run/react"; + + export default function Layout() { + return ( + <> +
Pathless 2 Layout
+ + + ); + } + `, + "app/routes/nested/__pathless2/bar.jsx": js` + export default function Bar() { + return

Bar

; + } + `, + }, + }); + + appFixture = await createAppFixture(fixture); +}); + +test.afterAll(async () => appFixture.close()); + +test.describe("with JavaScript", () => { + runTests(); +}); + +test.describe("without JavaScript", () => { + test.use({ javaScriptEnabled: false }); + runTests(); +}); + +/** + * Routes for this test look like this, for reference for the matches assertions: + * + * + * + * + * + * + * + * + * + * + */ + +function runTests() { + test("displays index page and not pathless layout page", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/nested"); + expect(await app.getHtml()).toMatch("Index"); + expect(await app.getHtml()).not.toMatch("Pathless Layout"); + expect(await app.getHtml()).toMatch("Number of matches: 2"); + }); + + test("displays page inside of pathless layout", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/nested/foo"); + expect(await app.getHtml()).not.toMatch("Index"); + expect(await app.getHtml()).toMatch("Pathless Layout"); + expect(await app.getHtml()).toMatch("Foo"); + expect(await app.getHtml()).toMatch("Number of matches: 3"); + }); + + // This also asserts that we support multiple sibling pathless route layouts + test("displays page inside of second pathless layout", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/nested/bar"); + expect(await app.getHtml()).not.toMatch("Index"); + expect(await app.getHtml()).toMatch("Pathless 2 Layout"); + expect(await app.getHtml()).toMatch("Bar"); + expect(await app.getHtml()).toMatch("Number of matches: 3"); + }); +} diff --git a/integration/flat-routes-test.ts b/integration/flat-routes-test.ts index f685f4b4fff..31bc29e849c 100644 --- a/integration/flat-routes-test.ts +++ b/integration/flat-routes-test.ts @@ -312,3 +312,132 @@ test.describe("", () => { expect(buildOutput).not.toContain(`Route Path Collision`); }); }); + +test.describe("pathless routes and route collisions", () => { + test.beforeAll(async () => { + fixture = await createFixture({ + future: { v2_routeConvention: true }, + files: { + "app/root.tsx": js` + import { Link, Outlet, Scripts, useMatches } from "@remix-run/react"; + + export default function App() { + let matches = 'Number of matches: ' + useMatches().length; + return ( + + + +

{matches}

+ + + + + ); + } + `, + "app/routes/nested._index.jsx": js` + export default function Index() { + return

Index

; + } + `, + "app/routes/nested._pathless.jsx": js` + import { Outlet } from "@remix-run/react"; + + export default function Layout() { + return ( + <> +
Pathless Layout
+ + + ); + } + `, + "app/routes/nested._pathless.foo.jsx": js` + export default function Foo() { + return

Foo

; + } + `, + "app/routes/nested._pathless2.jsx": js` + import { Outlet } from "@remix-run/react"; + + export default function Layout() { + return ( + <> +
Pathless 2 Layout
+ + + ); + } + `, + "app/routes/nested._pathless2.bar.jsx": js` + export default function Bar() { + return

Bar

; + } + `, + }, + }); + + appFixture = await createAppFixture(fixture); + }); + + test.afterAll(async () => appFixture.close()); + + test.describe("with JavaScript", () => { + runTests(); + }); + + test.describe("without JavaScript", () => { + test.use({ javaScriptEnabled: false }); + runTests(); + }); + + /** + * Routes for this test look like this, for reference for the matches assertions: + * + * + * + * + * + * + * + * + * + * + */ + + function runTests() { + test("displays index page and not pathless layout page", async ({ + page, + }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/nested"); + expect(await app.getHtml()).toMatch("Index"); + expect(await app.getHtml()).not.toMatch("Pathless Layout"); + expect(await app.getHtml()).toMatch("Number of matches: 2"); + }); + + test("displays page inside of pathless layout", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/nested/foo"); + expect(await app.getHtml()).not.toMatch("Index"); + expect(await app.getHtml()).toMatch("Pathless Layout"); + expect(await app.getHtml()).toMatch("Foo"); + expect(await app.getHtml()).toMatch("Number of matches: 3"); + }); + + // This also asserts that we support multiple sibling pathless route layouts + test("displays page inside of second pathless layout", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/nested/bar"); + expect(await app.getHtml()).not.toMatch("Index"); + expect(await app.getHtml()).toMatch("Pathless 2 Layout"); + expect(await app.getHtml()).toMatch("Bar"); + expect(await app.getHtml()).toMatch("Number of matches: 3"); + }); + } +}); diff --git a/integration/route-collisions-test.ts b/integration/route-collisions-test.ts new file mode 100644 index 00000000000..d6ea3606804 --- /dev/null +++ b/integration/route-collisions-test.ts @@ -0,0 +1,283 @@ +import { PassThrough } from "node:stream"; +import { test, expect } from "@playwright/test"; + +import { createFixture, js } from "./helpers/create-fixture"; + +let ROOT_FILE_CONTENTS = js` + import { Outlet, Scripts } from "@remix-run/react"; + + export default function App() { + return ( + + + + + + + ); + } +`; + +let LAYOUT_FILE_CONTENTS = js` + import { Outlet } from "@remix-run/react"; + + export default function Layout() { + return + } +`; + +let LEAF_FILE_CONTENTS = js` + export default function Foo() { + return

Foo

; + } +`; + +test.describe("build failures (v1 routes)", () => { + let errorLogs: string[]; + let oldConsoleError: typeof console.error; + + test.beforeEach(() => { + errorLogs = []; + oldConsoleError = console.error; + console.error = (str) => errorLogs.push(str); + }); + + test.afterEach(() => { + console.error = oldConsoleError; + }); + + test("detects path collisions inside pathless layout routes", async () => { + try { + await createFixture({ + future: { v2_routeConvention: false }, + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/foo.jsx": LEAF_FILE_CONTENTS, + "app/routes/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/foo.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path "foo" defined by route "routes/foo" conflicts with route "routes/__pathless/foo"' + ); + expect(errorLogs.length).toBe(1); + } + }); + + test("detects path collisions across pathless layout routes", async () => { + try { + await createFixture({ + future: { v2_routeConvention: false }, + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/foo.jsx": LEAF_FILE_CONTENTS, + "app/routes/__pathless2.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless2/foo.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path "foo" defined by route "routes/__pathless/foo" conflicts with route "routes/__pathless2/foo"' + ); + expect(errorLogs.length).toBe(1); + } + }); + + test("detects path collisions inside multiple pathless layout routes", async () => { + try { + await createFixture({ + future: { v2_routeConvention: false }, + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/foo.jsx": LEAF_FILE_CONTENTS, + "app/routes/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/__again.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/__again/foo.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path "foo" defined by route "routes/foo" conflicts with route "routes/__pathless/__again/foo"' + ); + expect(errorLogs.length).toBe(1); + } + }); + + test("detects path collisions of index files inside pathless layouts", async () => { + try { + await createFixture({ + future: { v2_routeConvention: false }, + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/index.jsx": LEAF_FILE_CONTENTS, + "app/routes/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/index.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path "/" defined by route "routes/index" conflicts with route "routes/__pathless/index"' + ); + expect(errorLogs.length).toBe(1); + } + }); + + test("detects path collisions of index files across multiple pathless layouts", async () => { + try { + await createFixture({ + future: { v2_routeConvention: false }, + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/nested/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/nested/__pathless/index.jsx": LEAF_FILE_CONTENTS, + "app/routes/nested/__oops.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/nested/__oops/index.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path "nested" defined by route "routes/nested/__oops/index" conflicts with route "routes/nested/__pathless/index"' + ); + expect(errorLogs.length).toBe(1); + } + }); + + test("detects path collisions of param routes inside pathless layouts", async () => { + try { + await createFixture({ + future: { v2_routeConvention: false }, + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/$param.jsx": LEAF_FILE_CONTENTS, + "app/routes/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/$param.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path ":param" defined by route "routes/$param" conflicts with route "routes/__pathless/$param"' + ); + expect(errorLogs.length).toBe(1); + } + }); +}); + +test.describe("build failures (v2 routes)", () => { + let originalConsoleLog = console.log; + let originalConsoleWarn = console.warn; + let originalConsoleError = console.error; + + test.beforeAll(async () => { + console.log = () => {}; + console.warn = () => {}; + console.error = () => {}; + }); + + test.afterAll(() => { + console.log = originalConsoleLog; + console.warn = originalConsoleWarn; + console.error = originalConsoleError; + }); + + async function setup(files: Record) { + let buildStdio = new PassThrough(); + let buildOutput: string; + await createFixture({ + buildStdio, + future: { v2_routeConvention: true }, + files, + }); + let chunks: Buffer[] = []; + buildOutput = await new Promise((resolve, reject) => { + buildStdio.on("data", (chunk) => chunks.push(Buffer.from(chunk))); + buildStdio.on("error", (err) => reject(err)); + buildStdio.on("end", () => + resolve(Buffer.concat(chunks).toString("utf8")) + ); + }); + return buildOutput; + } + + test("detects path collisions inside pathless layout routes", async () => { + let buildOutput = await setup({ + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/foo.jsx": LEAF_FILE_CONTENTS, + "app/routes/_pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/_pathless.foo.jsx": LEAF_FILE_CONTENTS, + }); + expect(buildOutput).toContain(`⚠️ Route Path Collision: "/foo"`); + expect(buildOutput).toContain(`🟢 routes/_pathless.foo.jsx`); + expect(buildOutput).toContain(`⭕️️ routes/foo.jsx`); + }); + + test("detects path collisions across pathless layout routes", async () => { + let buildOutput = await setup({ + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/_pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/_pathless.foo.jsx": LEAF_FILE_CONTENTS, + "app/routes/_pathless2.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/_pathless2.foo.jsx": LEAF_FILE_CONTENTS, + }); + expect(buildOutput).toContain(`⚠️ Route Path Collision: "/foo"`); + expect(buildOutput).toContain(`🟢 routes/_pathless2.foo.jsx`); + expect(buildOutput).toContain(`⭕️️ routes/_pathless.foo.jsx`); + }); + + test("detects path collisions inside multiple pathless layout routes", async () => { + let buildOutput = await setup({ + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/foo.jsx": LEAF_FILE_CONTENTS, + "app/routes/_pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/_pathless._again.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/_pathless._again.foo.jsx": LEAF_FILE_CONTENTS, + }); + expect(buildOutput).toContain(`⚠️ Route Path Collision: "/foo"`); + expect(buildOutput).toContain(`🟢 routes/_pathless._again.foo.jsx`); + expect(buildOutput).toContain(`⭕️️ routes/foo.jsx`); + }); + + test("detects path collisions of index files inside pathless layouts", async () => { + let buildOutput = await setup({ + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/_index.jsx": LEAF_FILE_CONTENTS, + "app/routes/_pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/_pathless._index.jsx": LEAF_FILE_CONTENTS, + }); + expect(buildOutput).toContain(`⚠️ Route Path Collision: "/"`); + expect(buildOutput).toContain(`🟢 routes/_pathless._index.jsx`); + expect(buildOutput).toContain(`⭕️️ routes/_index.jsx`); + }); + + test("detects path collisions of index files across multiple pathless layouts", async () => { + let buildOutput = await setup({ + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/nested._pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/nested._pathless._index.jsx": LEAF_FILE_CONTENTS, + "app/routes/nested._oops.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/nested._oops._index.jsx": LEAF_FILE_CONTENTS, + }); + expect(buildOutput).toContain(`⚠️ Route Path Collision: "/nested"`); + expect(buildOutput).toContain(`🟢 routes/nested._pathless._index.jsx`); + expect(buildOutput).toContain(`⭕️️ routes/nested._oops._index.jsx`); + }); + + test("detects path collisions of param routes inside pathless layouts", async () => { + let buildOutput = await setup({ + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/$param.jsx": LEAF_FILE_CONTENTS, + "app/routes/_pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/_pathless.$param.jsx": LEAF_FILE_CONTENTS, + }); + expect(buildOutput).toContain(`⚠️ Route Path Collision: "/:param"`); + expect(buildOutput).toContain(`🟢 routes/_pathless.$param.jsx`); + expect(buildOutput).toContain(`⭕️️ routes/$param.jsx`); + }); +}); diff --git a/packages/remix-dev/__tests__/flat-routes-test.ts b/packages/remix-dev/__tests__/flat-routes-test.ts index 8929c5084ff..ec8fa4f9ffd 100644 --- a/packages/remix-dev/__tests__/flat-routes-test.ts +++ b/packages/remix-dev/__tests__/flat-routes-test.ts @@ -712,5 +712,155 @@ describe("flatRoutes", () => { getRoutePathConflictErrorMessage("/products/:pid", testFiles) ); }); + + test("pathless layouts should not collide", () => { + let testFiles = [ + path.join(APP_DIR, "routes", "_a.tsx"), + path.join(APP_DIR, "routes", "_a._index.tsx"), + path.join(APP_DIR, "routes", "_a.a.tsx"), + path.join(APP_DIR, "routes", "_b.tsx"), + path.join(APP_DIR, "routes", "_b.b.tsx"), + ]; + + let routeManifest = flatRoutesUniversal(APP_DIR, testFiles); + + let routes = Object.values(routeManifest); + + expect(consoleError).not.toHaveBeenCalled(); + expect(routes).toHaveLength(5); + + // When using folders and route.tsx files + testFiles = [ + path.join(APP_DIR, "routes", "_a", "route.tsx"), + path.join(APP_DIR, "routes", "_a._index", "route.tsx"), + path.join(APP_DIR, "routes", "_a.a", "route.tsx"), + path.join(APP_DIR, "routes", "_b", "route.tsx"), + path.join(APP_DIR, "routes", "_b.b", "route.tsx"), + ]; + + routeManifest = flatRoutesUniversal(APP_DIR, testFiles); + + routes = Object.values(routeManifest); + + expect(consoleError).not.toHaveBeenCalled(); + expect(routes).toHaveLength(5); + }); + + test("nested pathless layouts should not collide", () => { + let testFiles = [ + path.join(APP_DIR, "routes", "nested._a.tsx"), + path.join(APP_DIR, "routes", "nested._a._index.tsx"), + path.join(APP_DIR, "routes", "nested._a.a.tsx"), + path.join(APP_DIR, "routes", "nested._b.tsx"), + path.join(APP_DIR, "routes", "nested._b.b.tsx"), + ]; + + let routeManifest = flatRoutesUniversal(APP_DIR, testFiles); + + let routes = Object.values(routeManifest); + + expect(consoleError).not.toHaveBeenCalled(); + expect(routes).toHaveLength(5); + + // When using folders and route.tsx files + testFiles = [ + path.join(APP_DIR, "routes", "nested._a", "route.tsx"), + path.join(APP_DIR, "routes", "nested._a._index", "route.tsx"), + path.join(APP_DIR, "routes", "nested._a.a", "route.tsx"), + path.join(APP_DIR, "routes", "nested._b", "route.tsx"), + path.join(APP_DIR, "routes", "nested._b.b", "route.tsx"), + ]; + + routeManifest = flatRoutesUniversal(APP_DIR, testFiles); + + routes = Object.values(routeManifest); + + expect(consoleError).not.toHaveBeenCalled(); + expect(routes).toHaveLength(5); + }); + + test("legit collisions without nested pathless layouts should collide (paths)", () => { + let testFiles = [ + path.join(APP_DIR, "routes", "nested._a.tsx"), + path.join(APP_DIR, "routes", "nested._a.a.tsx"), + path.join(APP_DIR, "routes", "nested._b.tsx"), + path.join(APP_DIR, "routes", "nested._b.a.tsx"), + ]; + + let routeManifest = flatRoutesUniversal(APP_DIR, testFiles); + + let routes = Object.values(routeManifest); + + expect(consoleError).toHaveBeenCalledWith( + getRoutePathConflictErrorMessage("/nested/a", [ + "routes/nested._a.a.tsx", + "routes/nested._b.a.tsx", + ]) + ); + expect(routes).toHaveLength(3); + + // When using folders and route.tsx files + consoleError.mockClear(); + testFiles = [ + path.join(APP_DIR, "routes", "nested._a", "route.tsx"), + path.join(APP_DIR, "routes", "nested._a.a", "route.tsx"), + path.join(APP_DIR, "routes", "nested._b", "route.tsx"), + path.join(APP_DIR, "routes", "nested._b.a", "route.tsx"), + ]; + + routeManifest = flatRoutesUniversal(APP_DIR, testFiles); + + routes = Object.values(routeManifest); + + expect(consoleError).toHaveBeenCalledWith( + getRoutePathConflictErrorMessage("/nested/a", [ + "routes/nested._a.a/route.tsx", + "routes/nested._b.a/route.tsx", + ]) + ); + expect(routes).toHaveLength(3); + }); + + test("legit collisions without nested pathless layouts should collide (index routes)", () => { + let testFiles = [ + path.join(APP_DIR, "routes", "nested._a.tsx"), + path.join(APP_DIR, "routes", "nested._a._index.tsx"), + path.join(APP_DIR, "routes", "nested._b.tsx"), + path.join(APP_DIR, "routes", "nested._b._index.tsx"), + ]; + + let routeManifest = flatRoutesUniversal(APP_DIR, testFiles); + + let routes = Object.values(routeManifest); + + expect(consoleError).toHaveBeenCalledWith( + getRoutePathConflictErrorMessage("/nested", [ + "routes/nested._a._index.tsx", + "routes/nested._b._index.tsx", + ]) + ); + expect(routes).toHaveLength(3); + + // When using folders and route.tsx files + consoleError.mockClear(); + testFiles = [ + path.join(APP_DIR, "routes", "nested._a", "route.tsx"), + path.join(APP_DIR, "routes", "nested._a._index", "route.tsx"), + path.join(APP_DIR, "routes", "nested._b", "route.tsx"), + path.join(APP_DIR, "routes", "nested._b._index", "route.tsx"), + ]; + + routeManifest = flatRoutesUniversal(APP_DIR, testFiles); + + routes = Object.values(routeManifest); + + expect(consoleError).toHaveBeenCalledWith( + getRoutePathConflictErrorMessage("/nested", [ + "routes/nested._a._index/route.tsx", + "routes/nested._b._index/route.tsx", + ]) + ); + expect(routes).toHaveLength(3); + }); }); }); diff --git a/packages/remix-dev/config/flat-routes.ts b/packages/remix-dev/config/flat-routes.ts index e4f49393f94..70811b727cb 100644 --- a/packages/remix-dev/config/flat-routes.ts +++ b/packages/remix-dev/config/flat-routes.ts @@ -214,10 +214,57 @@ export function flatRoutesUniversal( .replace(/\/$/, ""); } + if (!config.parentId) config.parentId = "root"; + + /** + * We do not try to detect path collisions for pathless layout route + * files because, by definition, they create the potential for route + * collisions _at that level in the tree_. + * + * Consider example where a user may want multiple pathless layout routes + * for different subfolders + * + * routes/ + * account.tsx + * account._private.tsx + * account._private.orders.tsx + * account._private.profile.tsx + * account._public.tsx + * account._public.login.tsx + * account._public.perks.tsx + * + * In order to support both a public and private layout for `/account/*` + * URLs, we are creating a mutually exclusive set of URLs beneath 2 + * separate pathless layout routes. In this case, the route paths for + * both account._public.tsx and account._private.tsx is the same + * (/account), but we're again not expecting to match at that level. + * + * By only ignoring this check when the final portion of the filename is + * pathless, we will still detect path collisions such as: + * + * routes/parent._pathless.foo.tsx + * routes/parent._pathless2.foo.tsx + * + * and + * + * routes/parent._pathless/index.tsx + * routes/parent._pathless2/index.tsx + */ + let lastRouteSegment = config.id + .replace(new RegExp(`^${prefix}/`), "") + .split(".") + .pop(); + let isPathlessLayoutRoute = + lastRouteSegment && + lastRouteSegment.startsWith("_") && + lastRouteSegment !== "_index"; + if (isPathlessLayoutRoute) { + continue; + } + let conflictRouteId = originalPathname + (config.index ? "?index" : ""); let conflict = uniqueRoutes.get(conflictRouteId); - if (!config.parentId) config.parentId = "root"; config.path = pathname || undefined; uniqueRoutes.set(conflictRouteId, config); diff --git a/packages/remix-dev/config/routesConvention.ts b/packages/remix-dev/config/routesConvention.ts index 742d1bb0e7c..35713e756bc 100644 --- a/packages/remix-dev/config/routesConvention.ts +++ b/packages/remix-dev/config/routesConvention.ts @@ -73,11 +73,52 @@ export function defineConventionalRoutes( let isIndexRoute = routeId.endsWith("/index"); let fullPath = createRoutePath(routeId.slice("routes".length + 1)); let uniqueRouteId = (fullPath || "") + (isIndexRoute ? "?index" : ""); - - if (uniqueRouteId) { + let isPathlessLayoutRoute = + routeId.split("/").pop()?.startsWith("__") === true; + + /** + * We do not try to detect path collisions for pathless layout route + * files because, by definition, they create the potential for route + * collisions _at that level in the tree_. + * + * Consider example where a user may want multiple pathless layout routes + * for different subfolders + * + * routes/ + * account.tsx + * account/ + * __public/ + * login.tsx + * perks.tsx + * __private/ + * orders.tsx + * profile.tsx + * __public.tsx + * __private.tsx + * + * In order to support both a public and private layout for `/account/*` + * URLs, we are creating a mutually exclusive set of URLs beneath 2 + * separate pathless layout routes. In this case, the route paths for + * both account/__public.tsx and account/__private.tsx is the same + * (/account), but we're again not expecting to match at that level. + * + * By only ignoring this check when the final portion of the filename is + * pathless, we will still detect path collisions such as: + * + * routes/parent/__pathless/foo.tsx + * routes/parent/__pathless2/foo.tsx + * + * and + * + * routes/parent/__pathless/index.tsx + * routes/parent/__pathless2/index.tsx + */ + if (uniqueRouteId && !isPathlessLayoutRoute) { if (uniqueRoutes.has(uniqueRouteId)) { throw new Error( - `Path ${JSON.stringify(fullPath)} defined by route ${JSON.stringify( + `Path ${JSON.stringify( + fullPath || "/" + )} defined by route ${JSON.stringify( routeId )} conflicts with route ${JSON.stringify( uniqueRoutes.get(uniqueRouteId) @@ -245,6 +286,8 @@ export function createRoutePath(partialRouteId: string): string | undefined { if (rawSegmentBuffer === "index" && result.endsWith("index")) { result = result.replace(/\/?index$/, ""); + } else { + result = result.replace(/\/$/, ""); } if (rawSegmentBuffer === "index" && result.endsWith("index?")) { From 33c904d934290206a7143af30c75d55c678a91bb Mon Sep 17 00:00:00 2001 From: Remix Run Bot Date: Wed, 24 May 2023 18:05:37 +0000 Subject: [PATCH 11/29] chore: format --- .changeset/stale-spoons-tie.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.changeset/stale-spoons-tie.md b/.changeset/stale-spoons-tie.md index ffe2da09933..27cdd94fce3 100644 --- a/.changeset/stale-spoons-tie.md +++ b/.changeset/stale-spoons-tie.md @@ -3,6 +3,7 @@ --- - Fix route ranking bug with pathless layout route next to a sibling index route + - Under the hood this is done by removing the trailing slash from all generated `path` values since the number of slash-delimited segments counts towards route ranking so the trailing slash incorrectly increases the score for routes -- Support sibling pathless layout routes by removing pathless layout routes from the unique route path checks in conventional route generation since they inherently trigger duplicate paths \ No newline at end of file +- Support sibling pathless layout routes by removing pathless layout routes from the unique route path checks in conventional route generation since they inherently trigger duplicate paths From d04a11ef30e4289c042a7c98d728ce73b9234303 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 May 2023 15:42:41 -0400 Subject: [PATCH 12/29] fix: include set-cookie headers from bubbled thrown responses (#6475) --- .changeset/bubbled-response-cookies.md | 5 + integration/headers-test.ts | 134 +++++++++++++++++++++++ integration/hmr-log-test.ts | 1 + integration/hmr-test.ts | 1 + packages/remix-server-runtime/headers.ts | 29 +++-- 5 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 .changeset/bubbled-response-cookies.md diff --git a/.changeset/bubbled-response-cookies.md b/.changeset/bubbled-response-cookies.md new file mode 100644 index 00000000000..0d863c64a79 --- /dev/null +++ b/.changeset/bubbled-response-cookies.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": patch +--- + +Automatically include set-cookie headers from bubbled thrown responses diff --git a/integration/headers-test.ts b/integration/headers-test.ts index 0bf306be55b..9b1fed79ad0 100644 --- a/integration/headers-test.ts +++ b/integration/headers-test.ts @@ -160,6 +160,43 @@ test.describe("headers export", () => { export default function Component() { return
} `, + + "app/routes/cookie.jsx": js` + import { json } from "@remix-run/server-runtime"; + import { Outlet } from "@remix-run/react"; + + export function loader({ request }) { + if (new URL(request.url).searchParams.has("parent-throw")) { + throw json(null, { headers: { "Set-Cookie": "parent-thrown-cookie=true" } }); + } + return null + }; + + export default function Parent() { + return ; + } + + export function ErrorBoundary() { + return

Caught!

; + } + `, + + "app/routes/cookie.child.jsx": js` + import { json } from "@remix-run/node"; + + export function loader({ request }) { + if (new URL(request.url).searchParams.has("throw")) { + throw json(null, { headers: { "Set-Cookie": "thrown-cookie=true" } }); + } + return json(null, { + headers: { "Set-Cookie": "normal-cookie=true" }, + }); + }; + + export default function Child() { + return

Child

; + } + `, }, }); }); @@ -350,6 +387,36 @@ test.describe("headers export", () => { ]) ); }); + + test("automatically includes cookie headers from normal responses", async () => { + let response = await appFixture.requestDocument("/cookie/child"); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["set-cookie", "normal-cookie=true"], + ]) + ); + }); + + test("automatically includes cookie headers from thrown responses", async () => { + let response = await appFixture.requestDocument("/cookie/child?throw"); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["set-cookie", "thrown-cookie=true"], + ]) + ); + }); + + test("does not duplicate thrown cookie headers from boundary route", async () => { + let response = await appFixture.requestDocument("/cookie?parent-throw"); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["set-cookie", "parent-thrown-cookie=true"], + ]) + ); + }); }); test.describe("v1 behavior (future.v2_headers=false)", () => { @@ -411,6 +478,43 @@ test.describe("v1 behavior (future.v2_headers=false)", () => { export default function Component() { return
} `, + + "app/routes/cookie.jsx": js` + import { json } from "@remix-run/server-runtime"; + import { Outlet } from "@remix-run/react"; + + export function loader({ request }) { + if (new URL(request.url).searchParams.has("parent-throw")) { + throw json(null, { headers: { "Set-Cookie": "parent-thrown-cookie=true" } }); + } + return null + }; + + export default function Parent() { + return ; + } + + export function ErrorBoundary() { + return

Caught!

; + } + `, + + "app/routes/cookie.child.jsx": js` + import { json } from "@remix-run/node"; + + export function loader({ request }) { + if (new URL(request.url).searchParams.has("throw")) { + throw json(null, { headers: { "Set-Cookie": "thrown-cookie=true" } }); + } + return json(null, { + headers: { "Set-Cookie": "normal-cookie=true" }, + }); + }; + + export default function Child() { + return

Child

; + } + `, }, }); }); @@ -431,4 +535,34 @@ test.describe("v1 behavior (future.v2_headers=false)", () => { JSON.stringify([["content-type", "text/html"]]) ); }); + + test("automatically includes cookie headers from normal responses", async () => { + let response = await appFixture.requestDocument("/cookie/child"); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["set-cookie", "normal-cookie=true"], + ]) + ); + }); + + test("automatically includes cookie headers from thrown responses", async () => { + let response = await appFixture.requestDocument("/cookie/child?throw"); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["set-cookie", "thrown-cookie=true"], + ]) + ); + }); + + test("does not duplicate thrown cookie headers from boundary route", async () => { + let response = await appFixture.requestDocument("/cookie?parent-throw"); + expect(JSON.stringify(Array.from(response.headers.entries()))).toBe( + JSON.stringify([ + ["content-type", "text/html"], + ["set-cookie", "parent-thrown-cookie=true"], + ]) + ); + }); }); diff --git a/integration/hmr-log-test.ts b/integration/hmr-log-test.ts index d7aa1dfd672..1a9fe3af396 100644 --- a/integration/hmr-log-test.ts +++ b/integration/hmr-log-test.ts @@ -28,6 +28,7 @@ let fixture = (options: { v2_errorBoundary: true, v2_normalizeFormMethod: true, v2_meta: true, + v2_headers: true, }, }; `, diff --git a/integration/hmr-test.ts b/integration/hmr-test.ts index e369070b9d0..6f6b63fd291 100644 --- a/integration/hmr-test.ts +++ b/integration/hmr-test.ts @@ -28,6 +28,7 @@ let fixture = (options: { v2_errorBoundary: true, v2_normalizeFormMethod: true, v2_meta: true, + v2_headers: true, }, }; `, diff --git a/packages/remix-server-runtime/headers.ts b/packages/remix-server-runtime/headers.ts index 98ac4853353..12bd2d66d99 100644 --- a/packages/remix-server-runtime/headers.ts +++ b/packages/remix-server-runtime/headers.ts @@ -38,10 +38,25 @@ export function getDocumentHeadersRR( let loaderHeaders = context.loaderHeaders[id] || new Headers(); let actionHeaders = context.actionHeaders[id] || new Headers(); + // Only expose errorHeaders to the leaf headers() function to + // avoid duplication via parentHeaders + let includeErrorHeaders = + errorHeaders != undefined && idx === matches.length - 1; + // Only prepend cookies from errorHeaders at the leaf renderable route + // when it's not the same as loaderHeaders/actionHeaders to avoid + // duplicate cookies + let includeErrorCookies = + includeErrorHeaders && + errorHeaders !== loaderHeaders && + errorHeaders !== actionHeaders; + // When the future flag is enabled, use the parent headers for any route // that doesn't have a `headers` export if (routeModule.headers == null && build.future.v2_headers) { - let headers = parentHeaders; + let headers = new Headers(parentHeaders); + if (includeErrorCookies) { + prependCookies(errorHeaders!, headers); + } prependCookies(actionHeaders, headers); prependCookies(loaderHeaders, headers); return headers; @@ -54,17 +69,17 @@ export function getDocumentHeadersRR( loaderHeaders, parentHeaders, actionHeaders, - // Only expose errorHeaders to the leaf headers() function to - // avoid duplication via parentHeaders - errorHeaders: - idx === matches.length - 1 ? errorHeaders : undefined, + errorHeaders: includeErrorHeaders ? errorHeaders : undefined, }) : routeModule.headers : undefined ); - // Automatically preserve Set-Cookie headers that were set either by the - // loader or by a parent route. + // Automatically preserve Set-Cookie headers from bubbled responses, + // loaders, errors, and parent routes + if (includeErrorCookies) { + prependCookies(errorHeaders!, headers); + } prependCookies(actionHeaders, headers); prependCookies(loaderHeaders, headers); prependCookies(parentHeaders, headers); From a1301ed3017225627ec8e2a940fa0ee01e51fc79 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Wed, 24 May 2023 22:57:25 -0400 Subject: [PATCH 13/29] feat(dev): reuse dev server port for websocket (#6476) --- .changeset/wicked-pandas-give.md | 23 ++++++ integration/hmr-log-test.ts | 36 ++++----- integration/hmr-test.ts | 39 +++++----- packages/remix-dev/__tests__/cli-test.ts | 7 +- packages/remix-dev/cli/commands.ts | 78 +++++++------------ packages/remix-dev/cli/run.ts | 34 ++------ packages/remix-dev/compiler/options.ts | 3 +- .../remix-dev/compiler/server/compiler.ts | 2 +- .../compiler/server/plugins/entry.ts | 4 +- packages/remix-dev/config.ts | 10 +-- .../remix-dev/devServer_unstable/index.ts | 66 ++++++++-------- .../remix-dev/devServer_unstable/socket.ts | 5 +- .../remix-react/__tests__/components-test.tsx | 6 +- packages/remix-react/browser.tsx | 2 +- packages/remix-react/components.tsx | 8 +- packages/remix-react/entry.ts | 2 +- packages/remix-server-runtime/build.ts | 2 +- .../remix-server-runtime/serverHandoff.ts | 2 +- 18 files changed, 153 insertions(+), 176 deletions(-) create mode 100644 .changeset/wicked-pandas-give.md diff --git a/.changeset/wicked-pandas-give.md b/.changeset/wicked-pandas-give.md new file mode 100644 index 00000000000..0a998218e57 --- /dev/null +++ b/.changeset/wicked-pandas-give.md @@ -0,0 +1,23 @@ +--- +"@remix-run/dev": minor +"@remix-run/react": minor +"@remix-run/server-runtime": minor +--- + +Reuse dev server port for WebSocket (Live Reload,HMR,HDR) + +As a result the `webSocketPort`/`--websocket-port` option has been obsoleted. +Additionally, scheme/host/port options for the dev server have been renamed. + +Available options are: + +| Option | flag | config | default | +| -------------- | ------------------ | ---------------- | --------------------------------- | +| Command | `-c` / `--command` | `command` | `remix-serve ` | +| Scheme | `--scheme` | `scheme` | `http` | +| Host | `--host` | `host` | `localhost` | +| Port | `--port` | `port` | Dynamically chosen open port | +| No restart | `--no-restart` | `restart: false` | `restart: true` | + +Note that scheme/host/port options are for the _dev server_, not your app server. +You probably don't need to use scheme/host/port option if you aren't configuring networking (e.g. for Docker or SSL). diff --git a/integration/hmr-log-test.ts b/integration/hmr-log-test.ts index 1a9fe3af396..54ab8b6c01a 100644 --- a/integration/hmr-log-test.ts +++ b/integration/hmr-log-test.ts @@ -9,11 +9,7 @@ import { createFixtureProject, css, js, json } from "./helpers/create-fixture"; test.setTimeout(120_000); -let fixture = (options: { - appServerPort: number; - httpPort: number; - webSocketPort: number; -}) => ({ +let fixture = (options: { appPort: number; devPort: number }) => ({ files: { "remix.config.js": js` module.exports = { @@ -21,8 +17,7 @@ let fixture = (options: { tailwind: true, future: { unstable_dev: { - httpPort: ${options.httpPort}, - webSocketPort: ${options.webSocketPort}, + port: ${options.devPort}, }, v2_routeConvention: true, v2_errorBoundary: true, @@ -80,7 +75,7 @@ let fixture = (options: { }) ); - let port = ${options.appServerPort}; + let port = ${options.appPort}; app.listen(port, () => { let build = require(BUILD_DIR); console.log('✅ app ready: http://localhost:' + port); @@ -250,12 +245,9 @@ test("HMR", async ({ page }) => { }); let portRange = makeRange(4080, 4099); - let appServerPort = await getPort({ port: portRange }); - let httpPort = await getPort({ port: portRange }); - let webSocketPort = await getPort({ port: portRange }); - let projectDir = await createFixtureProject( - fixture({ appServerPort, httpPort, webSocketPort }) - ); + let appPort = await getPort({ port: portRange }); + let devPort = await getPort({ port: portRange }); + let projectDir = await createFixtureProject(fixture({ appPort, devPort })); // spin up dev server let dev = execa("npm", ["run", "dev"], { cwd: projectDir }); @@ -270,7 +262,7 @@ test("HMR", async ({ page }) => { { timeoutMs: 10_000 } ); - await page.goto(`http://localhost:${appServerPort}`, { + await page.goto(`http://localhost:${appPort}`, { waitUntil: "networkidle", }); @@ -470,7 +462,7 @@ whatsup await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); await page.waitForLoadState("networkidle"); - expect(devStderr()).toBe(""); + let stderr = devStderr(); let withSyntaxError = ` import { useLoaderData } from "@remix-run/react"; export function shouldRevalidate(args) { @@ -486,9 +478,15 @@ whatsup } `; fs.writeFileSync(indexPath, withSyntaxError); - await wait(() => devStderr().includes('Expected ";" but found "efault"'), { - timeoutMs: HMR_TIMEOUT_MS, - }); + await wait( + () => + devStderr() + .replace(stderr, "") + .includes('Expected ";" but found "efault"'), + { + timeoutMs: HMR_TIMEOUT_MS, + } + ); let withFix = ` import { useLoaderData } from "@remix-run/react"; diff --git a/integration/hmr-test.ts b/integration/hmr-test.ts index 6f6b63fd291..45aa771ec01 100644 --- a/integration/hmr-test.ts +++ b/integration/hmr-test.ts @@ -9,11 +9,7 @@ import { createFixtureProject, css, js, json } from "./helpers/create-fixture"; test.setTimeout(120_000); -let fixture = (options: { - appServerPort: number; - httpPort: number; - webSocketPort: number; -}) => ({ +let fixture = (options: { appPort: number; devPort: number }) => ({ files: { "remix.config.js": js` module.exports = { @@ -21,8 +17,7 @@ let fixture = (options: { tailwind: true, future: { unstable_dev: { - httpPort: ${options.httpPort}, - webSocketPort: ${options.webSocketPort}, + port: ${options.devPort}, }, v2_routeConvention: true, v2_errorBoundary: true, @@ -69,18 +64,17 @@ let fixture = (options: { const app = express(); app.use(express.static("public", { immutable: true, maxAge: "1y" })); - const MODE = process.env.NODE_ENV; const BUILD_DIR = path.join(process.cwd(), "build"); app.all( "*", createRequestHandler({ build: require(BUILD_DIR), - mode: MODE, + mode: process.env.NODE_ENV, }) ); - let port = ${options.appServerPort}; + let port = ${options.appPort}; app.listen(port, () => { let build = require(BUILD_DIR); console.log('✅ app ready: http://localhost:' + port); @@ -249,12 +243,9 @@ test("HMR", async ({ page }) => { }); let portRange = makeRange(3080, 3099); - let appServerPort = await getPort({ port: portRange }); - let httpPort = await getPort({ port: portRange }); - let webSocketPort = await getPort({ port: portRange }); - let projectDir = await createFixtureProject( - fixture({ appServerPort, httpPort, webSocketPort }) - ); + let appPort = await getPort({ port: portRange }); + let devPort = await getPort({ port: portRange }); + let projectDir = await createFixtureProject(fixture({ appPort, devPort })); // spin up dev server let dev = execa("npm", ["run", "dev"], { cwd: projectDir }); @@ -269,7 +260,7 @@ test("HMR", async ({ page }) => { { timeoutMs: 10_000 } ); - await page.goto(`http://localhost:${appServerPort}`, { + await page.goto(`http://localhost:${appPort}`, { waitUntil: "networkidle", }); @@ -469,7 +460,7 @@ whatsup await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); await page.waitForLoadState("networkidle"); - expect(devStderr()).toBe(""); + let stderr = devStderr(); let withSyntaxError = ` import { useLoaderData } from "@remix-run/react"; export function shouldRevalidate(args) { @@ -485,9 +476,15 @@ whatsup } `; fs.writeFileSync(indexPath, withSyntaxError); - await wait(() => devStderr().includes('Expected ";" but found "efault"'), { - timeoutMs: HMR_TIMEOUT_MS, - }); + await wait( + () => + devStderr() + .replace(stderr, "") + .includes('Expected ";" but found "efault"'), + { + timeoutMs: HMR_TIMEOUT_MS, + } + ); let withFix = ` import { useLoaderData } from "@remix-run/react"; diff --git a/packages/remix-dev/__tests__/cli-test.ts b/packages/remix-dev/__tests__/cli-test.ts index 0ce72093343..5e14cc52a1a 100644 --- a/packages/remix-dev/__tests__/cli-test.ts +++ b/packages/remix-dev/__tests__/cli-test.ts @@ -118,11 +118,10 @@ describe("remix CLI", () => { [unstable_dev] --command, -c Command used to run your app server - --http-scheme HTTP(S) scheme for the dev server. Default: http - --http-host HTTP(S) host for the dev server. Default: localhost - --http-port HTTP(S) port for the dev server. Default: any open port + --scheme Scheme for the dev server. Default: http + --host Host for the dev server. Default: localhost + --port Port for the dev server. Default: any open port --no-restart Do not restart the app server when rebuilds occur. - --websocket-port WebSocket port for the dev server. Default: any open port \`init\` Options: --no-delete Skip deleting the \`remix.init\` script \`routes\` Options: diff --git a/packages/remix-dev/cli/commands.ts b/packages/remix-dev/cli/commands.ts index 200a7a81dc6..95bec84c03e 100644 --- a/packages/remix-dev/cli/commands.ts +++ b/packages/remix-dev/cli/commands.ts @@ -177,13 +177,8 @@ export async function build( onWarning: warnOnce, }; if (mode === "development" && config.future.unstable_dev) { - let dev = await resolveDevBuild(config); - options.devHttpOrigin = { - scheme: dev.httpScheme, - host: dev.httpHost, - port: dev.httpPort, - }; - options.devWebSocketPort = dev.webSocketPort; + let origin = await resolveDevOrigin(config); + options.devOrigin = origin; } fse.emptyDirSync(config.assetsBuildDirectory); @@ -216,20 +211,20 @@ export async function dev( remixRoot: string, flags: { debug?: boolean; - port?: number; // TODO: remove for v2 // unstable_dev command?: string; - httpScheme?: string; - httpHost?: string; - httpPort?: number; + scheme?: string; + host?: string; + port?: number; restart?: boolean; - websocketPort?: number; } = {} ) { if (process.env.NODE_ENV && process.env.NODE_ENV !== "development") { console.warn( - `Forcing NODE_ENV to be 'development'. Was: ${process.env.NODE_ENV}` + `Forcing NODE_ENV to be 'development'. Was: ${JSON.stringify( + process.env.NODE_ENV + )}` ); } process.env.NODE_ENV = "development"; @@ -471,49 +466,42 @@ let parseMode = ( let findPort = async () => getPort({ port: makeRange(3001, 3100) }); -type DevBuildFlags = { - httpScheme: string; - httpHost: string; - httpPort: number; - webSocketPort: number; +type DevOrigin = { + scheme: string; + host: string; + port: number; }; -let resolveDevBuild = async ( +let resolveDevOrigin = async ( config: RemixConfig, - flags: Partial = {} -): Promise => { + flags: Partial = {} +): Promise => { let dev = config.future.unstable_dev; if (dev === false) throw Error("This should never happen"); // prettier-ignore - let httpScheme = - flags.httpScheme ?? - (dev === true ? undefined : dev.httpScheme) ?? + let scheme = + flags.scheme ?? + (dev === true ? undefined : dev.scheme) ?? "http"; // prettier-ignore - let httpHost = - flags.httpHost ?? - (dev === true ? undefined : dev.httpHost) ?? + let host = + flags.host ?? + (dev === true ? undefined : dev.host) ?? "localhost"; // prettier-ignore - let httpPort = - flags.httpPort ?? - (dev === true ? undefined : dev.httpPort) ?? - (await findPort()); - // prettier-ignore - let webSocketPort = - flags.webSocketPort ?? - (dev === true ? undefined : dev.webSocketPort) ?? + let port = + flags.port ?? + (dev === true ? undefined : dev.port) ?? (await findPort()); return { - httpScheme, - httpHost, - httpPort, - webSocketPort, + scheme, + host, + port, }; }; -type DevServeFlags = DevBuildFlags & { +type DevServeFlags = DevOrigin & { command: string; restart: boolean; }; @@ -524,10 +512,7 @@ let resolveDevServe = async ( let dev = config.future.unstable_dev; if (dev === false) throw Error("Cannot resolve dev options"); - let { httpScheme, httpHost, httpPort, webSocketPort } = await resolveDevBuild( - config, - flags - ); + let origin = await resolveDevOrigin(config, flags); // prettier-ignore let command = @@ -558,10 +543,7 @@ let resolveDevServe = async ( return { command, - httpScheme, - httpHost, - httpPort, - webSocketPort, + ...origin, restart, }; }; diff --git a/packages/remix-dev/cli/run.ts b/packages/remix-dev/cli/run.ts index 524b1bdc844..f1d8bcde78f 100644 --- a/packages/remix-dev/cli/run.ts +++ b/packages/remix-dev/cli/run.ts @@ -44,11 +44,10 @@ ${colors.logoBlue("R")} ${colors.logoGreen("E")} ${colors.logoYellow( [unstable_dev] --command, -c Command used to run your app server - --http-scheme HTTP(S) scheme for the dev server. Default: http - --http-host HTTP(S) host for the dev server. Default: localhost - --http-port HTTP(S) port for the dev server. Default: any open port + --scheme Scheme for the dev server. Default: http + --host Host for the dev server. Default: localhost + --port Port for the dev server. Default: any open port --no-restart Do not restart the app server when rebuilds occur. - --websocket-port WebSocket port for the dev server. Default: any open port \`init\` Options: --no-delete Skip deleting the \`remix.init\` script \`routes\` Options: @@ -170,8 +169,6 @@ export async function run(argv: string[] = process.argv.slice(2)) { "--interactive": Boolean, "--no-interactive": Boolean, "--json": Boolean, - "--port": Number, - "-p": "--port", "--remix-version": String, "--sourcemap": Boolean, "--template": String, @@ -184,11 +181,11 @@ export async function run(argv: string[] = process.argv.slice(2)) { // dev server "--command": String, "-c": "--command", - "--http-scheme": String, - "--http-host": String, - "--http-port": Number, + "--scheme": String, + "--host": String, + "--port": Number, + "-p": "--port", "--no-restart": Boolean, - "--websocket-port": Number, }, { argv, @@ -213,23 +210,6 @@ export async function run(argv: string[] = process.argv.slice(2)) { return; } - if (flags["http-scheme"]) { - flags.httpScheme = flags["http-scheme"]; - delete flags["http-scheme"]; - } - if (flags["http-host"]) { - flags.httpHost = flags["http-host"]; - delete flags["http-host"]; - } - if (flags["http-port"]) { - flags.httpPort = flags["http-port"]; - delete flags["http-port"]; - } - if (flags["websocket-port"]) { - flags.webSocketPort = flags["websocket-port"]; - delete flags["websocket-port"]; - } - if (args["--no-delete"]) { flags.delete = false; } diff --git a/packages/remix-dev/compiler/options.ts b/packages/remix-dev/compiler/options.ts index 165b89cbab0..d128116d7de 100644 --- a/packages/remix-dev/compiler/options.ts +++ b/packages/remix-dev/compiler/options.ts @@ -6,10 +6,9 @@ export type Options = { onWarning?: (message: string, key: string) => void; // TODO: required in v2 - devHttpOrigin?: { + devOrigin?: { scheme: string; host: string; port: number; }; - devWebSocketPort?: number; }; diff --git a/packages/remix-dev/compiler/server/compiler.ts b/packages/remix-dev/compiler/server/compiler.ts index 3dd99e80ffe..47af4230248 100644 --- a/packages/remix-dev/compiler/server/compiler.ts +++ b/packages/remix-dev/compiler/server/compiler.ts @@ -103,7 +103,7 @@ const createEsbuildConfig = ( ctx.config.devServerPort ), "process.env.REMIX_DEV_HTTP_ORIGIN": JSON.stringify( - ctx.options.devHttpOrigin ?? "" // TODO: remove nullish check in v2 + ctx.options.devOrigin ?? "" // TODO: remove nullish check in v2 ), }, jsx: "automatic", diff --git a/packages/remix-dev/compiler/server/plugins/entry.ts b/packages/remix-dev/compiler/server/plugins/entry.ts index 3d64193e3b4..4966dd3ad93 100644 --- a/packages/remix-dev/compiler/server/plugins/entry.ts +++ b/packages/remix-dev/compiler/server/plugins/entry.ts @@ -51,9 +51,9 @@ ${Object.keys(config.routes) export const publicPath = ${JSON.stringify(config.publicPath)}; export const entry = { module: entryServer }; ${ - options.devWebSocketPort + options.devOrigin ? `export const dev = ${JSON.stringify({ - websocketPort: options.devWebSocketPort, + port: options.devOrigin.port, })}` : "" } diff --git a/packages/remix-dev/config.ts b/packages/remix-dev/config.ts index ed90d95a1f9..e431e14d919 100644 --- a/packages/remix-dev/config.ts +++ b/packages/remix-dev/config.ts @@ -37,15 +37,11 @@ export type ServerModuleFormat = "esm" | "cjs"; export type ServerPlatform = "node" | "neutral"; type Dev = { - port?: number; // TODO: remove in v2 - command?: string; - httpScheme?: string; - httpHost?: string; - httpPort?: number; - webSocketPort?: number; + scheme?: string; + host?: string; + port?: number; restart?: boolean; - publicDirectory?: string; }; interface FutureConfig { diff --git a/packages/remix-dev/devServer_unstable/index.ts b/packages/remix-dev/devServer_unstable/index.ts index e4492d350a9..01c29d8b71e 100644 --- a/packages/remix-dev/devServer_unstable/index.ts +++ b/packages/remix-dev/devServer_unstable/index.ts @@ -1,5 +1,6 @@ import * as path from "node:path"; import * as stream from "node:stream"; +import * as http from "node:http"; import fs from "fs-extra"; import prettyMs from "pretty-ms"; import execa from "execa"; @@ -41,21 +42,13 @@ export let serve = async ( initialConfig: RemixConfig, options: { command: string; - httpScheme: string; - httpHost: string; - httpPort: number; - webSocketPort: number; + scheme: string; + host: string; + port: number; restart: boolean; } ) => { await loadEnv(initialConfig.rootDirectory); - let websocket = Socket.serve({ port: options.webSocketPort }); - let httpOrigin: Origin = { - scheme: options.httpScheme, - host: options.httpHost, - port: options.httpPort, - }; - let state: { appServer?: execa.ExecaChildProcess; manifest?: Manifest; @@ -65,6 +58,30 @@ export let serve = async ( prevLoaderHashes?: Record; } = {}; + let app = express() + // handle `broadcastDevReady` messages + .use(express.json()) + .post("/ping", (req, res) => { + let { buildHash } = req.body; + if (typeof buildHash !== "string") { + console.warn(`Unrecognized payload: ${req.body}`); + res.sendStatus(400); + } + if (buildHash === state.manifest?.version) { + state.appReady?.ok(); + } + res.sendStatus(200); + }); + + let server = http.createServer(app); + let websocket = Socket.serve(server); + + let origin: Origin = { + scheme: options.scheme, + host: options.host, + port: options.port, + }; + let bin = await detectBin(); let startAppServer = (command: string) => { console.log(`> ${command}`); @@ -74,7 +91,7 @@ export let serve = async ( NODE_ENV: "development", PATH: bin + (process.platform === "win32" ? ";" : ":") + process.env.PATH, - REMIX_DEV_HTTP_ORIGIN: stringifyOrigin(httpOrigin), + REMIX_DEV_HTTP_ORIGIN: stringifyOrigin(origin), }, // https://github.com/sindresorhus/execa/issues/433 windowsHide: false, @@ -118,8 +135,7 @@ export let serve = async ( mode: "development", sourcemap: true, onWarning: warnOnce, - devHttpOrigin: httpOrigin, - devWebSocketPort: options.webSocketPort, + devOrigin: origin, }, }, { @@ -198,28 +214,14 @@ export let serve = async ( } ); - let httpServer = express() - // handle `broadcastDevReady` messages - .use(express.json()) - .post("/ping", (req, res) => { - let { buildHash } = req.body; - if (typeof buildHash !== "string") { - console.warn(`Unrecognized payload: ${req.body}`); - res.sendStatus(400); - } - if (buildHash === state.manifest?.version) { - state.appReady?.ok(); - } - res.sendStatus(200); - }) - .listen(httpOrigin.port, () => { - console.log("Remix dev server ready"); - }); + server.listen(origin.port, () => { + console.log("Remix dev server ready"); + }); return new Promise(() => {}).finally(async () => { await kill(state.appServer); websocket.close(); - httpServer.close(); + server.close(); await dispose(); }); }; diff --git a/packages/remix-dev/devServer_unstable/socket.ts b/packages/remix-dev/devServer_unstable/socket.ts index 83ea95fb5ca..a66ca95a0db 100644 --- a/packages/remix-dev/devServer_unstable/socket.ts +++ b/packages/remix-dev/devServer_unstable/socket.ts @@ -1,4 +1,5 @@ import WebSocket from "ws"; +import type { Server as HTTPServer } from "http"; import { type Manifest } from "../manifest"; import type * as HMR from "./hmr"; @@ -14,8 +15,8 @@ type Message = type Broadcast = (message: Message) => void; -export let serve = (options: { port: number }) => { - let wss = new WebSocket.Server({ port: options.port }); +export let serve = (server: HTTPServer) => { + let wss = new WebSocket.Server({ server }); let broadcast: Broadcast = (message) => { wss.clients.forEach((client) => { diff --git a/packages/remix-react/__tests__/components-test.tsx b/packages/remix-react/__tests__/components-test.tsx index b088805a445..6cb621925b1 100644 --- a/packages/remix-react/__tests__/components-test.tsx +++ b/packages/remix-react/__tests__/components-test.tsx @@ -47,14 +47,14 @@ describe("", () => { LiveReload = require("../components").LiveReload; let { container } = render(); expect(container.querySelector("script")).toHaveTextContent( - "let port = (window.__remixContext && window.__remixContext.dev && window.__remixContext.dev.websocketPort) || 8002;" + "let port = undefined || (window.__remixContext && window.__remixContext.dev && window.__remixContext.dev.port) || 8002;" ); }); it("can set the port explicitly", () => { let { container } = render(); expect(container.querySelector("script")).toHaveTextContent( - "let port = (window.__remixContext && window.__remixContext.dev && window.__remixContext.dev.websocketPort) || 4321;" + "let port = 4321 || (window.__remixContext && window.__remixContext.dev && window.__remixContext.dev.port) || 8002;" ); }); @@ -62,7 +62,7 @@ describe("", () => { process.env.REMIX_DEV_SERVER_WS_PORT = "1234"; let { container } = render(); expect(container.querySelector("script")).toHaveTextContent( - "let port = (window.__remixContext && window.__remixContext.dev && window.__remixContext.dev.websocketPort) || 1234;" + "let port = undefined || (window.__remixContext && window.__remixContext.dev && window.__remixContext.dev.port) || 1234;" ); }); diff --git a/packages/remix-react/browser.tsx b/packages/remix-react/browser.tsx index c81a6ed80fc..821731a3dad 100644 --- a/packages/remix-react/browser.tsx +++ b/packages/remix-react/browser.tsx @@ -24,7 +24,7 @@ declare global { // The number of active deferred keys rendered on the server a?: number; dev?: { - websocketPort?: number; + port?: number; hmrRuntime?: string; }; }; diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index cd5fbf8e8a9..661447f5c53 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -1714,7 +1714,7 @@ export const LiveReload = ? () => null : function LiveReload({ // TODO: remove REMIX_DEV_SERVER_WS_PORT in v2 - port = Number(process.env.REMIX_DEV_SERVER_WS_PORT || 8002), + port, timeoutMs = 1000, nonce = undefined, }: { @@ -1732,9 +1732,9 @@ export const LiveReload = function remixLiveReloadConnect(config) { let protocol = location.protocol === "https:" ? "wss:" : "ws:"; let host = location.hostname; - let port = (window.__remixContext && window.__remixContext.dev && window.__remixContext.dev.websocketPort) || ${String( - port - )}; + let port = ${port} || (window.__remixContext && window.__remixContext.dev && window.__remixContext.dev.port) || ${Number( + process.env.REMIX_DEV_SERVER_WS_PORT || 8002 + )}; let socketPath = protocol + "//" + host + ":" + port + "/socket"; let ws = new WebSocket(socketPath); ws.onmessage = async (message) => { diff --git a/packages/remix-react/entry.ts b/packages/remix-react/entry.ts index d2a2ab2f269..cfef8123a4d 100644 --- a/packages/remix-react/entry.ts +++ b/packages/remix-react/entry.ts @@ -10,7 +10,7 @@ export interface RemixContextObject { serverHandoffString?: string; future: FutureConfig; abortDelay?: number; - dev?: { websocketPort: number }; + dev?: { port: number }; } // Additional React-Router information needed at runtime, but not hydrated diff --git a/packages/remix-server-runtime/build.ts b/packages/remix-server-runtime/build.ts index 4e1b831b1f2..dfa5e281054 100644 --- a/packages/remix-server-runtime/build.ts +++ b/packages/remix-server-runtime/build.ts @@ -15,7 +15,7 @@ export interface ServerBuild { publicPath: string; assetsBuildDirectory: string; future: FutureConfig; - dev?: { websocketPort: number }; + dev?: { port: number }; } export interface HandleDocumentRequestFunction { diff --git a/packages/remix-server-runtime/serverHandoff.ts b/packages/remix-server-runtime/serverHandoff.ts index edc4d4b3055..0ea04f24493 100644 --- a/packages/remix-server-runtime/serverHandoff.ts +++ b/packages/remix-server-runtime/serverHandoff.ts @@ -20,7 +20,7 @@ export function createServerHandoffString(serverHandoff: { // we'd end up including duplicate info state: ValidateShape; future: FutureConfig; - dev?: { websocketPort: number }; + dev?: { port: number }; }): string { // Uses faster alternative of jsesc to escape data returned from the loaders. // This string is inserted directly into the HTML in the `` element. From 33cc4c043209d59bf3523cb0aaeab0b150cf373c Mon Sep 17 00:00:00 2001 From: Remix Run Bot Date: Thu, 25 May 2023 02:59:20 +0000 Subject: [PATCH 14/29] chore: format --- .changeset/wicked-pandas-give.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.changeset/wicked-pandas-give.md b/.changeset/wicked-pandas-give.md index 0a998218e57..5d0f26f8718 100644 --- a/.changeset/wicked-pandas-give.md +++ b/.changeset/wicked-pandas-give.md @@ -11,13 +11,13 @@ Additionally, scheme/host/port options for the dev server have been renamed. Available options are: -| Option | flag | config | default | -| -------------- | ------------------ | ---------------- | --------------------------------- | -| Command | `-c` / `--command` | `command` | `remix-serve ` | -| Scheme | `--scheme` | `scheme` | `http` | -| Host | `--host` | `host` | `localhost` | -| Port | `--port` | `port` | Dynamically chosen open port | -| No restart | `--no-restart` | `restart: false` | `restart: true` | +| Option | flag | config | default | +| ---------- | ------------------ | ---------------- | --------------------------------- | +| Command | `-c` / `--command` | `command` | `remix-serve ` | +| Scheme | `--scheme` | `scheme` | `http` | +| Host | `--host` | `host` | `localhost` | +| Port | `--port` | `port` | Dynamically chosen open port | +| No restart | `--no-restart` | `restart: false` | `restart: true` | Note that scheme/host/port options are for the _dev server_, not your app server. You probably don't need to use scheme/host/port option if you aren't configuring networking (e.g. for Docker or SSL). From 10bcb92eff8121882bde2fd18464a421299e9bd8 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Thu, 25 May 2023 10:50:09 -0400 Subject: [PATCH 15/29] fix(dev): better error message when `remix-serve` is not found (#6477) --- .changeset/rude-geese-remain.md | 5 ++ packages/remix-dev/cli/commands.ts | 22 +------ .../remix-dev/devServer_unstable/index.ts | 62 +++++++++++++------ 3 files changed, 50 insertions(+), 39 deletions(-) create mode 100644 .changeset/rude-geese-remain.md diff --git a/.changeset/rude-geese-remain.md b/.changeset/rude-geese-remain.md new file mode 100644 index 00000000000..a2b43df3f62 --- /dev/null +++ b/.changeset/rude-geese-remain.md @@ -0,0 +1,5 @@ +--- +"@remix-run/dev": patch +--- + +better error message when `remix-serve` is not found diff --git a/packages/remix-dev/cli/commands.ts b/packages/remix-dev/cli/commands.ts index 95bec84c03e..8df2b505024 100644 --- a/packages/remix-dev/cli/commands.ts +++ b/packages/remix-dev/cli/commands.ts @@ -25,7 +25,6 @@ import { TaskError } from "../codemod/utils/task"; import { transpile as convertFileToJS } from "./useJavascript"; import { warnOnce } from "../warnOnce"; import type { Options } from "../compiler/options"; -import { getAppDependencies } from "../dependencies"; export async function create({ appTemplate, @@ -502,7 +501,7 @@ let resolveDevOrigin = async ( }; type DevServeFlags = DevOrigin & { - command: string; + command?: string; restart: boolean; }; let resolveDevServe = async ( @@ -518,25 +517,6 @@ let resolveDevServe = async ( let command = flags.command ?? (dev === true ? undefined : dev.command) - if (!command) { - command = `remix-serve ${path.relative( - process.cwd(), - config.serverBuildPath - )}`; - - let usingRemixAppServer = - getAppDependencies(config, true)["@remix-run/serve"] !== undefined; - if (!usingRemixAppServer) { - console.error( - [ - `Remix dev server command defaulted to '${command}', but @remix-run/serve is not installed.`, - "If you are using another server, specify how to run it with `-c` or `--command` flag.", - "For example, `remix dev -c 'node ./server.js'`", - ].join("\n") - ); - process.exit(1); - } - } let restart = flags.restart ?? (dev === true ? undefined : dev.restart) ?? true; diff --git a/packages/remix-dev/devServer_unstable/index.ts b/packages/remix-dev/devServer_unstable/index.ts index 01c29d8b71e..009a24e7439 100644 --- a/packages/remix-dev/devServer_unstable/index.ts +++ b/packages/remix-dev/devServer_unstable/index.ts @@ -18,6 +18,7 @@ import { detectPackageManager } from "../cli/detectPackageManager"; import * as HDR from "./hdr"; import type { Result } from "../result"; import { err, ok } from "../result"; +import invariant from "../invariant"; type Origin = { scheme: string; @@ -41,7 +42,7 @@ let detectBin = async (): Promise => { export let serve = async ( initialConfig: RemixConfig, options: { - command: string; + command?: string; scheme: string; host: string; port: number; @@ -83,19 +84,47 @@ export let serve = async ( }; let bin = await detectBin(); - let startAppServer = (command: string) => { - console.log(`> ${command}`); - let newAppServer = execa.command(command, { - stdio: "pipe", - env: { - NODE_ENV: "development", - PATH: - bin + (process.platform === "win32" ? ";" : ":") + process.env.PATH, - REMIX_DEV_HTTP_ORIGIN: stringifyOrigin(origin), - }, - // https://github.com/sindresorhus/execa/issues/433 - windowsHide: false, - }); + let startAppServer = (command?: string) => { + let cmd = + command ?? + `remix-serve ${path.relative( + process.cwd(), + initialConfig.serverBuildPath + )}`; + console.log(`> ${cmd}`); + let newAppServer = execa + .command(cmd, { + stdio: "pipe", + env: { + NODE_ENV: "development", + PATH: + bin + (process.platform === "win32" ? ";" : ":") + process.env.PATH, + REMIX_DEV_HTTP_ORIGIN: stringifyOrigin(origin), + }, + // https://github.com/sindresorhus/execa/issues/433 + windowsHide: false, + }) + .on("error", (e) => { + // patch execa error types + invariant("errno" in e && typeof e.errno === "number", "errno missing"); + invariant("code" in e && typeof e.code === "string", "code missing"); + invariant("path" in e && typeof e.path === "string", "path missing"); + + if (command === undefined) { + console.error( + [ + "", + `┏ [error] command not found: ${e.path}`, + `┃ \`remix dev\` did not receive \`--command\` nor \`-c\`, defaulting to \`${cmd}\`.`, + "┃ You probably meant to use `-c` for your app server command.", + "┗ For example: `remix dev -c 'node ./server.js'`", + "", + ].join("\n") + ); + process.exit(1); + } + throw e; + }); if (newAppServer.stdin) process.stdin.pipe(newAppServer.stdin, { end: true }); @@ -164,10 +193,7 @@ export let serve = async ( try { console.log(`Waiting for app server (${state.manifest?.version})`); let start = Date.now(); - if ( - options.command && - (state.appServer === undefined || options.restart) - ) { + if (state.appServer === undefined || options.restart) { await kill(state.appServer); state.appServer = startAppServer(options.command); } From 4f858ee589acbf176328132059100469ebb65cc4 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Thu, 25 May 2023 11:16:03 -0400 Subject: [PATCH 16/29] refactor(dev): remove vestigial plugin (#6479) --- .../compiler/js/plugins/routes_unstable.ts | 171 ------------------ 1 file changed, 171 deletions(-) delete mode 100644 packages/remix-dev/compiler/js/plugins/routes_unstable.ts diff --git a/packages/remix-dev/compiler/js/plugins/routes_unstable.ts b/packages/remix-dev/compiler/js/plugins/routes_unstable.ts deleted file mode 100644 index 7640a0dd953..00000000000 --- a/packages/remix-dev/compiler/js/plugins/routes_unstable.ts +++ /dev/null @@ -1,171 +0,0 @@ -import * as fs from "node:fs"; -import * as path from "node:path"; -import type esbuild from "esbuild"; -import generate from "@babel/generator"; - -import { routeModuleExts } from "../../../config/routesConvention"; -import * as Transform from "../../../transform"; -import { getLoaderForFile } from "../../utils/loaders"; -import { applyHMR } from "./hmr"; -import type { Context } from "../../context"; -import { processMDX } from "../../plugins/mdx"; - -const serverOnlyExports = new Set(["action", "loader"]); - -let removeServerExports = () => - Transform.create(({ types: t }) => { - return { - visitor: { - ExportNamedDeclaration: (path) => { - let { node } = path; - if (node.source) { - let specifiers = node.specifiers.filter(({ exported }) => { - let name = t.isIdentifier(exported) - ? exported.name - : exported.value; - return !serverOnlyExports.has(name); - }); - if (specifiers.length === node.specifiers.length) return; - if (specifiers.length === 0) return path.remove(); - path.replaceWith( - t.exportNamedDeclaration( - node.declaration, - specifiers, - node.source - ) - ); - } - if (t.isFunctionDeclaration(node.declaration)) { - let name = node.declaration.id?.name; - if (!name) return; - if (serverOnlyExports.has(name)) return path.remove(); - } - if (t.isVariableDeclaration(node.declaration)) { - let declarations = node.declaration.declarations.filter((d) => { - let name = t.isIdentifier(d.id) ? d.id.name : undefined; - if (!name) return false; - return !serverOnlyExports.has(name); - }); - if (declarations.length === 0) return path.remove(); - if (declarations.length === node.declaration.declarations.length) - return; - path.replaceWith( - t.variableDeclaration(node.declaration.kind, declarations) - ); - } - }, - }, - }; - }); - -/** - * This plugin loads route modules for the browser build, using module shims - * that re-export only the route module exports that are safe for the browser. - */ -export function browserRouteModulesPlugin( - { config, options }: Context, - routeModulePaths: Map -): esbuild.Plugin { - return { - name: "browser-route-modules", - async setup(build) { - let [xdm, { default: remarkFrontmatter }] = await Promise.all([ - import("xdm"), - import("remark-frontmatter") as any, - ]); - - build.onResolve({ filter: /.*/ }, (args) => { - // We have to map all imports from route modules back to the virtual - // module in the graph otherwise we will be duplicating portions of - // route modules across the build. - let routeModulePath = routeModulePaths.get(args.path); - if (!routeModulePath && args.resolveDir && args.path.startsWith(".")) { - let lookup = resolvePath(path.join(args.resolveDir, args.path)); - routeModulePath = routeModulePaths.get(lookup); - } - if (!routeModulePath) return; - return { - path: routeModulePath, - namespace: "browser-route-module", - }; - }); - - let cache = new Map(); - - build.onLoad( - { filter: /.*/, namespace: "browser-route-module" }, - async (args) => { - let file = args.path; - let routeFile = path.resolve(config.appDirectory, file); - - if (/\.mdx?$/.test(file)) { - let mdxResult = await processMDX( - xdm, - remarkFrontmatter, - config, - args.path, - routeFile - ); - if (!mdxResult.contents || mdxResult.errors?.length) { - return mdxResult; - } - - let transform = removeServerExports(); - mdxResult.contents = transform( - mdxResult.contents, - // Trick babel into allowing JSX syntax. - args.path + ".jsx" - ); - return mdxResult; - } - - let sourceCode = fs.readFileSync(routeFile, "utf8"); - - let value = cache.get(file); - if (!value || value.sourceCode !== sourceCode) { - let transform = removeServerExports(); - let contents = transform(sourceCode, routeFile); - - if (options.mode === "development" && config.future.unstable_dev) { - contents = await applyHMR( - contents, - { - ...args, - path: routeFile, - }, - config, - !!build.initialOptions.sourcemap - ); - } - value = { - sourceCode, - output: { - contents, - loader: getLoaderForFile(routeFile), - resolveDir: path.dirname(routeFile), - }, - }; - cache.set(file, value); - } - - return value.output; - } - ); - }, - }; -} - -function resolvePath(path: string) { - if (fs.existsSync(path) && fs.statSync(path).isFile()) { - return path; - } - - for (let ext of routeModuleExts) { - let withExt = path + ext; - if (fs.existsSync(withExt) && fs.statSync(withExt).isFile()) { - return withExt; - } - } - - return path; -} From 786e968b965dc171cf1f93881fbd33149c16282a Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Thu, 25 May 2023 13:05:24 -0400 Subject: [PATCH 17/29] test(hmr): expect exactly 2 console errors in browser (#6480) --- integration/hmr-log-test.ts | 44 ++++++++++++++++++++++++++++++++++++- integration/hmr-test.ts | 44 ++++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/integration/hmr-log-test.ts b/integration/hmr-log-test.ts index 54ab8b6c01a..2c3b62ff5f3 100644 --- a/integration/hmr-log-test.ts +++ b/integration/hmr-log-test.ts @@ -230,12 +230,28 @@ let bufferize = (stream: Readable): (() => string) => { return () => buffer; }; +let logConsoleError = (error: Error) => { + console.error(`[console] ${error.name}: ${error.message}`); +}; + +let expectConsoleError = ( + isExpected: (error: Error) => boolean, + unexpected = logConsoleError +) => { + return (error: Error) => { + if (isExpected(error)) { + return; + } + unexpected(error); + }; +}; + let HMR_TIMEOUT_MS = 10_000; test("HMR", async ({ page }) => { // uncomment for debugging // page.on("console", (msg) => console.log(msg.text())); - page.on("pageerror", (err) => console.log(err.message)); + page.on("pageerror", logConsoleError); let dataRequests = 0; page.on("request", (request) => { let url = new URL(request.url()); @@ -488,6 +504,27 @@ whatsup } ); + // React Router integration w/ React Refresh has a bug where sometimes rerenders happen with old UI and new data + // in this case causing `TypeError: Cannot destructure property`. + // Need to fix that bug, but it only shows a harmless console error in the browser in dev + page.removeListener("pageerror", logConsoleError); + let expectedErrorCount = 0; + let expectDestructureTypeError = expectConsoleError((error) => { + let expectedMessage = new Set([ + // chrome, edge + "Cannot destructure property 'hello' of 'useLoaderData(...)' as it is null.", + // firefox + "(intermediate value)() is null", + // webkit + "Right side of assignment cannot be destructured", + ]); + let isExpected = + error.name === "TypeError" && expectedMessage.has(error.message); + if (isExpected) expectedErrorCount += 1; + return isExpected; + }); + page.on("pageerror", expectDestructureTypeError); + let withFix = ` import { useLoaderData } from "@remix-run/react"; export function shouldRevalidate(args) { @@ -505,6 +542,11 @@ whatsup fs.writeFileSync(indexPath, withFix); await page.waitForLoadState("networkidle"); await page.getByText("With Fix").waitFor({ timeout: HMR_TIMEOUT_MS }); + + // Restore normal console error handling + page.removeListener("pageerror", expectDestructureTypeError); + expect(expectedErrorCount).toBe(2); + page.addListener("pageerror", logConsoleError); } catch (e) { console.log("stdout begin -----------------------"); console.log(devStdout()); diff --git a/integration/hmr-test.ts b/integration/hmr-test.ts index 45aa771ec01..1d71aabd87b 100644 --- a/integration/hmr-test.ts +++ b/integration/hmr-test.ts @@ -228,12 +228,28 @@ let bufferize = (stream: Readable): (() => string) => { return () => buffer; }; +let logConsoleError = (error: Error) => { + console.error(`[console] ${error.name}: ${error.message}`); +}; + +let expectConsoleError = ( + isExpected: (error: Error) => boolean, + unexpected = logConsoleError +) => { + return (error: Error) => { + if (isExpected(error)) { + return; + } + unexpected(error); + }; +}; + let HMR_TIMEOUT_MS = 10_000; test("HMR", async ({ page }) => { // uncomment for debugging // page.on("console", (msg) => console.log(msg.text())); - page.on("pageerror", (err) => console.log(err.message)); + page.on("pageerror", logConsoleError); let dataRequests = 0; page.on("request", (request) => { let url = new URL(request.url()); @@ -486,6 +502,27 @@ whatsup } ); + // React Router integration w/ React Refresh has a bug where sometimes rerenders happen with old UI and new data + // in this case causing `TypeError: Cannot destructure property`. + // Need to fix that bug, but it only shows a harmless console error in the browser in dev + page.removeListener("pageerror", logConsoleError); + let expectedErrorCount = 0; + let expectDestructureTypeError = expectConsoleError((error) => { + let expectedMessage = new Set([ + // chrome, edge + "Cannot destructure property 'hello' of 'useLoaderData(...)' as it is null.", + // firefox + "(intermediate value)() is null", + // webkit + "Right side of assignment cannot be destructured" + ]); + let isExpected = + error.name === "TypeError" && expectedMessage.has(error.message); + if (isExpected) expectedErrorCount += 1; + return isExpected; + }); + page.on("pageerror", expectDestructureTypeError); + let withFix = ` import { useLoaderData } from "@remix-run/react"; export function shouldRevalidate(args) { @@ -503,6 +540,11 @@ whatsup fs.writeFileSync(indexPath, withFix); await page.waitForLoadState("networkidle"); await page.getByText("With Fix").waitFor({ timeout: HMR_TIMEOUT_MS }); + + // Restore normal console error handling + page.removeListener("pageerror", expectDestructureTypeError); + expect(expectedErrorCount).toBe(2); + page.addListener("pageerror", logConsoleError); } catch (e) { console.log("stdout begin -----------------------"); console.log(devStdout()); From 2b31855c4a9cca042b4e038ad662023c19713892 Mon Sep 17 00:00:00 2001 From: Remix Run Bot Date: Thu, 25 May 2023 17:07:24 +0000 Subject: [PATCH 18/29] chore: format --- integration/hmr-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/hmr-test.ts b/integration/hmr-test.ts index 1d71aabd87b..44631799523 100644 --- a/integration/hmr-test.ts +++ b/integration/hmr-test.ts @@ -514,7 +514,7 @@ whatsup // firefox "(intermediate value)() is null", // webkit - "Right side of assignment cannot be destructured" + "Right side of assignment cannot be destructured", ]); let isExpected = error.name === "TypeError" && expectedMessage.has(error.message); From 88d286dcb3c408ea7eb6f7bc1fa0d4dd963105e9 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Thu, 25 May 2023 15:52:22 -0400 Subject: [PATCH 19/29] feat(dev): built-in tls support (#6483) --- .changeset/ninety-hornets-brake.md | 85 +++++++++++++++++++ packages/remix-dev/__tests__/cli-test.ts | 2 + packages/remix-dev/cli/commands.ts | 18 +++- packages/remix-dev/cli/run.ts | 13 +++ packages/remix-dev/config.ts | 2 + .../remix-dev/devServer_unstable/index.ts | 14 ++- 6 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 .changeset/ninety-hornets-brake.md diff --git a/.changeset/ninety-hornets-brake.md b/.changeset/ninety-hornets-brake.md new file mode 100644 index 00000000000..37035a1eccf --- /dev/null +++ b/.changeset/ninety-hornets-brake.md @@ -0,0 +1,85 @@ +--- +"@remix-run/dev": minor +--- + +built-in tls support + +New options: +- `--tls-key` / `tlsKey`: TLS key +- `--tls-cert` / `tlsCert`: TLS Certificate + +If both TLS options are set, `scheme` defaults to `https` + +## Example + +Install [mkcert](https://github.com/FiloSottile/mkcert) and create a local CA: + +```sh +brew install mkcert +mkcert -install +``` + +Then make sure you inform `node` about your CA certs: + +```sh +export NODE_EXTRA_CA_CERTS="$(mkcert -CAROOT)/rootCA.pem" +``` + +👆 You'll probably want to put that env var in your scripts or `.bashrc`/`.zshrc` + +Now create `key.pem` and `cert.pem`: + +```sh +mkcert -key-file key.pem -cert-file cert.pem localhost +``` + +See `mkcert` docs for more details. + +Finally, pass in the paths to the key and cert via flags: + +```sh +remix dev --tls-key=key.pem --tls-cert=cert.pem +``` + +or via config: + +```js +module.exports = { + future: { + unstable_dev: { + tlsKey: "key.pem", + tlsCert: "cert.pem", + } + } +} +``` + +That's all that's needed to set up the Remix Dev Server with TLS. + +🚨 Make sure to update your app server for TLS as well. + +For example, with `express`: + +```ts +import express from 'express' +import https from 'node:https' +import fs from 'node:fs' + +let app = express() + +// ...code setting up your express app... + +let appServer = https.createServer({ + key: fs.readFileSync("key.pem"), + cert: fs.readFileSync("cert.pem"), +}, app) + +appServer.listen(3000, () => { + console.log('Ready on https://localhost:3000') +}) +``` + +## Known limitations + +`remix-serve` does not yet support TLS. +That means this only works for custom app server using the `-c` flag for now. diff --git a/packages/remix-dev/__tests__/cli-test.ts b/packages/remix-dev/__tests__/cli-test.ts index 5e14cc52a1a..91d8fea10b5 100644 --- a/packages/remix-dev/__tests__/cli-test.ts +++ b/packages/remix-dev/__tests__/cli-test.ts @@ -122,6 +122,8 @@ describe("remix CLI", () => { --host Host for the dev server. Default: localhost --port Port for the dev server. Default: any open port --no-restart Do not restart the app server when rebuilds occur. + --tls-key Path to TLS key (key.pem) + --tls-cert Path to TLS certificate (cert.pem) \`init\` Options: --no-delete Skip deleting the \`remix.init\` script \`routes\` Options: diff --git a/packages/remix-dev/cli/commands.ts b/packages/remix-dev/cli/commands.ts index 8df2b505024..0a9074923bc 100644 --- a/packages/remix-dev/cli/commands.ts +++ b/packages/remix-dev/cli/commands.ts @@ -217,6 +217,8 @@ export async function dev( host?: string; port?: number; restart?: boolean; + tlsKey?: string; + tlsCert?: string; } = {} ) { if (process.env.NODE_ENV && process.env.NODE_ENV !== "development") { @@ -472,7 +474,10 @@ type DevOrigin = { }; let resolveDevOrigin = async ( config: RemixConfig, - flags: Partial = {} + flags: Partial & { + tlsKey?: string; + tlsCert?: string; + } = {} ): Promise => { let dev = config.future.unstable_dev; if (dev === false) throw Error("This should never happen"); @@ -481,7 +486,7 @@ let resolveDevOrigin = async ( let scheme = flags.scheme ?? (dev === true ? undefined : dev.scheme) ?? - "http"; + (flags.tlsKey && flags.tlsCert) ? "https": "http"; // prettier-ignore let host = flags.host ?? @@ -503,6 +508,8 @@ let resolveDevOrigin = async ( type DevServeFlags = DevOrigin & { command?: string; restart: boolean; + tlsKey?: string; + tlsCert?: string; }; let resolveDevServe = async ( config: RemixConfig, @@ -521,9 +528,16 @@ let resolveDevServe = async ( let restart = flags.restart ?? (dev === true ? undefined : dev.restart) ?? true; + let tlsKey = flags.tlsKey ?? (dev === true ? undefined : dev.tlsKey); + if (tlsKey) tlsKey = path.resolve(tlsKey); + let tlsCert = flags.tlsCert ?? (dev === true ? undefined : dev.tlsCert); + if (tlsCert) tlsCert = path.resolve(tlsCert); + return { command, ...origin, restart, + tlsKey, + tlsCert, }; }; diff --git a/packages/remix-dev/cli/run.ts b/packages/remix-dev/cli/run.ts index f1d8bcde78f..abba2fdc95e 100644 --- a/packages/remix-dev/cli/run.ts +++ b/packages/remix-dev/cli/run.ts @@ -48,6 +48,8 @@ ${colors.logoBlue("R")} ${colors.logoGreen("E")} ${colors.logoYellow( --host Host for the dev server. Default: localhost --port Port for the dev server. Default: any open port --no-restart Do not restart the app server when rebuilds occur. + --tls-key Path to TLS key (key.pem) + --tls-cert Path to TLS certificate (cert.pem) \`init\` Options: --no-delete Skip deleting the \`remix.init\` script \`routes\` Options: @@ -186,6 +188,8 @@ export async function run(argv: string[] = process.argv.slice(2)) { "--port": Number, "-p": "--port", "--no-restart": Boolean, + "--tls-key": String, + "--tls-cert": String, }, { argv, @@ -210,6 +214,15 @@ export async function run(argv: string[] = process.argv.slice(2)) { return; } + if (flags["tls-key"]) { + flags.tlsKey = flags["tls-key"]; + delete flags["tls-key"]; + } + if (flags["tls-cert"]) { + flags.tlsCert = flags["tls-cert"]; + delete flags["tls-cert"]; + } + if (args["--no-delete"]) { flags.delete = false; } diff --git a/packages/remix-dev/config.ts b/packages/remix-dev/config.ts index e431e14d919..68e91d48641 100644 --- a/packages/remix-dev/config.ts +++ b/packages/remix-dev/config.ts @@ -42,6 +42,8 @@ type Dev = { host?: string; port?: number; restart?: boolean; + tlsKey?: string; + tlsCert?: string; }; interface FutureConfig { diff --git a/packages/remix-dev/devServer_unstable/index.ts b/packages/remix-dev/devServer_unstable/index.ts index 009a24e7439..f9b47c0f114 100644 --- a/packages/remix-dev/devServer_unstable/index.ts +++ b/packages/remix-dev/devServer_unstable/index.ts @@ -1,6 +1,7 @@ import * as path from "node:path"; import * as stream from "node:stream"; import * as http from "node:http"; +import * as https from "node:https"; import fs from "fs-extra"; import prettyMs from "pretty-ms"; import execa from "execa"; @@ -47,6 +48,8 @@ export let serve = async ( host: string; port: number; restart: boolean; + tlsKey?: string; + tlsCert?: string; } ) => { await loadEnv(initialConfig.rootDirectory); @@ -74,7 +77,16 @@ export let serve = async ( res.sendStatus(200); }); - let server = http.createServer(app); + let server = + options.tlsKey && options.tlsCert + ? https.createServer( + { + key: fs.readFileSync(options.tlsKey), + cert: fs.readFileSync(options.tlsCert), + }, + app + ) + : http.createServer(app); let websocket = Socket.serve(server); let origin: Origin = { From a6e8805206b0b695cb85990d8f57eed3727ef2b7 Mon Sep 17 00:00:00 2001 From: Remix Run Bot Date: Thu, 25 May 2023 19:54:17 +0000 Subject: [PATCH 20/29] chore: format --- .changeset/ninety-hornets-brake.md | 32 +++++++++++++++++------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/.changeset/ninety-hornets-brake.md b/.changeset/ninety-hornets-brake.md index 37035a1eccf..1f91dfb4344 100644 --- a/.changeset/ninety-hornets-brake.md +++ b/.changeset/ninety-hornets-brake.md @@ -5,6 +5,7 @@ built-in tls support New options: + - `--tls-key` / `tlsKey`: TLS key - `--tls-cert` / `tlsCert`: TLS Certificate @@ -46,12 +47,12 @@ or via config: ```js module.exports = { future: { - unstable_dev: { + unstable_dev: { tlsKey: "key.pem", tlsCert: "cert.pem", - } - } -} + }, + }, +}; ``` That's all that's needed to set up the Remix Dev Server with TLS. @@ -61,22 +62,25 @@ That's all that's needed to set up the Remix Dev Server with TLS. For example, with `express`: ```ts -import express from 'express' -import https from 'node:https' -import fs from 'node:fs' +import express from "express"; +import https from "node:https"; +import fs from "node:fs"; -let app = express() +let app = express(); // ...code setting up your express app... -let appServer = https.createServer({ - key: fs.readFileSync("key.pem"), - cert: fs.readFileSync("cert.pem"), -}, app) +let appServer = https.createServer( + { + key: fs.readFileSync("key.pem"), + cert: fs.readFileSync("cert.pem"), + }, + app +); appServer.listen(3000, () => { - console.log('Ready on https://localhost:3000') -}) + console.log("Ready on https://localhost:3000"); +}); ``` ## Known limitations From 21504cf3e980ede28d1f3609fa144800c2516d81 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Thu, 25 May 2023 16:40:05 -0400 Subject: [PATCH 21/29] fix(dev): color output for app server (#6485) --- .changeset/serious-wolves-attend.md | 5 +++++ packages/remix-dev/devServer_unstable/index.ts | 1 + 2 files changed, 6 insertions(+) create mode 100644 .changeset/serious-wolves-attend.md diff --git a/.changeset/serious-wolves-attend.md b/.changeset/serious-wolves-attend.md new file mode 100644 index 00000000000..f8f8b840cca --- /dev/null +++ b/.changeset/serious-wolves-attend.md @@ -0,0 +1,5 @@ +--- +"@remix-run/dev": patch +--- + +restore color for app server output diff --git a/packages/remix-dev/devServer_unstable/index.ts b/packages/remix-dev/devServer_unstable/index.ts index f9b47c0f114..1bbf1de6aee 100644 --- a/packages/remix-dev/devServer_unstable/index.ts +++ b/packages/remix-dev/devServer_unstable/index.ts @@ -112,6 +112,7 @@ export let serve = async ( PATH: bin + (process.platform === "win32" ? ";" : ":") + process.env.PATH, REMIX_DEV_HTTP_ORIGIN: stringifyOrigin(origin), + FORCE_COLOR: process.env.NO_COLOR === undefined ? "1" : "0", }, // https://github.com/sindresorhus/execa/issues/433 windowsHide: false, From 8d456e3042d71957b2e6c44938ad6eb95747b657 Mon Sep 17 00:00:00 2001 From: Leo Singer Date: Fri, 26 May 2023 10:51:59 -0400 Subject: [PATCH 22/29] fix(remix-react): fix typos in V2 warnings (#6434) --- packages/remix-react/components.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 661447f5c53..6e364e1e727 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -364,19 +364,19 @@ export function composeEventHandlers< let linksWarning = "⚠️ REMIX FUTURE CHANGE: The behavior of links `imagesizes` and `imagesrcset` will be changing in v2. " + - "Only the React camel case versions will be valid. Please change to `imageSizes` and `imageSrcSet`." + + "Only the React camel case versions will be valid. Please change to `imageSizes` and `imageSrcSet`. " + "For instructions on making this change see " + "https://remix.run/docs/en/v1.15.0/pages/v2#links-imagesizes-and-imagesrcset"; let useTransitionWarning = "⚠️ REMIX FUTURE CHANGE: `useTransition` will be removed in v2 in favor of `useNavigation`. " + - "You can prepare for this change at your convenience by updating to `useNavigation`." + + "You can prepare for this change at your convenience by updating to `useNavigation`. " + "For instructions on making this change see " + "https://remix.run/docs/en/v1.15.0/pages/v2#usetransition"; let fetcherTypeWarning = "⚠️ REMIX FUTURE CHANGE: `fetcher.type` will be removed in v2. " + - "Please use `fetcher.state`, `fetcher.formData`, and `fetcher.data` to achieve the same UX." + + "Please use `fetcher.state`, `fetcher.formData`, and `fetcher.data` to achieve the same UX. " + "For instructions on making this change see " + "https://remix.run/docs/en/v1.15.0/pages/v2#usefetcher"; From 00698e3f94ac13fc3064f858a1de9e7eb474aa87 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Fri, 26 May 2023 13:40:30 -0400 Subject: [PATCH 23/29] fix(dev): only apply hmr for remix routes (#6493) --- packages/remix-dev/compiler/plugins/cssSideEffectImports.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/remix-dev/compiler/plugins/cssSideEffectImports.ts b/packages/remix-dev/compiler/plugins/cssSideEffectImports.ts index aa4862c180d..f2a944b0348 100644 --- a/packages/remix-dev/compiler/plugins/cssSideEffectImports.ts +++ b/packages/remix-dev/compiler/plugins/cssSideEffectImports.ts @@ -66,7 +66,7 @@ export const cssSideEffectImportsPlugin = ( let loader = loaderForExtension[path.extname(args.path) as Extension]; let contents = addSuffixToCssSideEffectImports(loader, code); - if (hmr) { + if (args.path.startsWith(ctx.config.appDirectory) && hmr) { contents = await applyHMR( contents, args, From 408ebf88e40fff73e161305e1e7dcc4c1f6271dc Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Wed, 31 May 2023 13:17:15 +1000 Subject: [PATCH 24/29] test: allow config overrides for Playwright tests (#6508) --- integration/abort-signal-test.ts | 4 +- integration/action-test.ts | 10 ++- integration/bug-report-test.ts | 4 +- integration/catch-boundary-data-test.ts | 12 ++- integration/catch-boundary-test.ts | 12 ++- integration/cf-compiler-test.ts | 4 +- integration/compiler-mjs-output-test.ts | 18 ++--- integration/compiler-test.ts | 6 +- integration/css-modules-test.ts | 6 +- integration/css-side-effect-imports-test.ts | 14 ++-- integration/custom-entry-server-test.ts | 4 +- integration/defer-loader-test.ts | 4 +- integration/defer-test.ts | 14 ++-- integration/deno-compiler-test.ts | 4 +- .../deterministic-build-output-test.ts | 6 +- integration/error-boundary-test.ts | 74 ++++++++++++------- integration/error-boundary-v2-test.ts | 8 +- integration/error-data-request-test.ts | 4 +- integration/esm-only-warning-test.ts | 4 +- integration/fetcher-layout-test.ts | 4 +- integration/fetcher-state-test.ts | 4 +- integration/fetcher-test.ts | 10 ++- integration/flat-routes-test.ts | 31 ++++---- integration/form-data-test.ts | 4 +- integration/form-test.ts | 4 +- integration/headers-test.ts | 24 +++--- .../helpers/cf-template/remix.config.js | 2 +- integration/helpers/create-fixture.ts | 58 +++++++++++---- .../helpers/deno-template/remix.config.js | 2 +- .../helpers/node-template/remix.config.js | 2 +- integration/hmr-log-test.ts | 33 ++++----- integration/hmr-test.ts | 33 ++++----- integration/hook-useSubmit-test.ts | 4 +- integration/js-routes-test.ts | 6 +- integration/layout-route-test.ts | 6 +- integration/link-test.ts | 8 +- integration/loader-test.ts | 10 ++- integration/matches-test.ts | 4 +- integration/mdx-test.ts | 4 +- integration/meta-test.ts | 21 +++--- integration/multiple-cookies-test.ts | 6 +- integration/navigation-state-v2-test.ts | 6 +- integration/path-mapping-test.ts | 4 +- integration/postcss-test.ts | 30 ++++---- integration/prefetch-test.ts | 12 ++- integration/redirects-test.ts | 4 +- integration/rendering-test.ts | 4 +- integration/request-test.ts | 4 +- integration/resource-routes-test.ts | 12 ++- integration/revalidate-test.ts | 6 +- integration/route-collisions-test.ts | 28 +++++-- integration/scroll-test.ts | 4 +- .../server-code-in-browser-message-test.ts | 4 +- integration/server-entry-test.ts | 4 +- integration/server-source-maps-test.ts | 4 +- integration/set-cookie-revalidation-test.ts | 4 +- integration/shared-route-imports-test.ts | 8 +- integration/splat-routes-test.ts | 4 +- integration/tailwind-test.ts | 30 ++++---- integration/transition-state-test.ts | 4 +- integration/transition-test.ts | 4 +- integration/upload-test.ts | 4 +- integration/vanilla-extract-test.ts | 6 +- package.json | 4 + packages/remix-dev/package.json | 1 - yarn.lock | 26 ++++++- 66 files changed, 459 insertions(+), 254 deletions(-) diff --git a/integration/abort-signal-test.ts b/integration/abort-signal-test.ts index 17c55a06acd..db047de7ab0 100644 --- a/integration/abort-signal-test.ts +++ b/integration/abort-signal-test.ts @@ -9,7 +9,9 @@ let appFixture: AppFixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/_index.jsx": js` import { json } from "@remix-run/node"; diff --git a/integration/action-test.ts b/integration/action-test.ts index 00bd5a059e7..4154ba7696c 100644 --- a/integration/action-test.ts +++ b/integration/action-test.ts @@ -17,10 +17,12 @@ test.describe("actions", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, - v2_errorBoundary: true, - v2_normalizeFormMethod: true, + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + v2_normalizeFormMethod: true, + }, }, files: { "app/routes/urlencoded.jsx": js` diff --git a/integration/bug-report-test.ts b/integration/bug-report-test.ts index 8c6048ffe02..ef78611e6d1 100644 --- a/integration/bug-report-test.ts +++ b/integration/bug-report-test.ts @@ -42,7 +42,9 @@ let appFixture: AppFixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, //////////////////////////////////////////////////////////////////////////// // 💿 Next, add files to this object, just like files in a real app, // `createFixture` will make an app and run your tests against it. diff --git a/integration/catch-boundary-data-test.ts b/integration/catch-boundary-data-test.ts index becde4b6a42..5e401a7835d 100644 --- a/integration/catch-boundary-data-test.ts +++ b/integration/catch-boundary-data-test.ts @@ -26,7 +26,9 @@ let LAYOUT_DATA = "root data"; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { json } from "@remix-run/node"; @@ -236,9 +238,11 @@ test("renders self boundary with layout data available on transition", async ({ test.describe("v2_errorBoundary", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, - v2_errorBoundary: true, + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + }, }, files: { "app/root.jsx": js` diff --git a/integration/catch-boundary-test.ts b/integration/catch-boundary-test.ts index d6ce148f187..c28af3be810 100644 --- a/integration/catch-boundary-test.ts +++ b/integration/catch-boundary-test.ts @@ -24,7 +24,9 @@ test.describe("CatchBoundary", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { json } from "@remix-run/node"; @@ -382,9 +384,11 @@ test.describe("v2_errorBoundary", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, - v2_errorBoundary: true, + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + }, }, files: { "app/root.jsx": js` diff --git a/integration/cf-compiler-test.ts b/integration/cf-compiler-test.ts index 6aafd8122e9..dd43597f99e 100644 --- a/integration/cf-compiler-test.ts +++ b/integration/cf-compiler-test.ts @@ -28,7 +28,9 @@ test.describe("cloudflare compiler", () => { test.beforeAll(async () => { projectDir = await createFixtureProject({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, setup: "cloudflare", template: "cf-template", files: { diff --git a/integration/compiler-mjs-output-test.ts b/integration/compiler-mjs-output-test.ts index 97719a1e5e8..90869843f5d 100644 --- a/integration/compiler-mjs-output-test.ts +++ b/integration/compiler-mjs-output-test.ts @@ -8,8 +8,15 @@ let projectDir: string; test.beforeAll(async () => { projectDir = await createFixtureProject({ - future: { v2_routeConvention: true }, files: { + // Ensure the config is valid ESM + "remix.config.js": js` + export default { + serverModuleFormat: "esm", + serverBuildPath: "build/index.mjs", + future: { v2_routeConvention: true }, + }; + `, "package.json": js` { "name": "remix-template-remix", @@ -40,15 +47,6 @@ test.beforeAll(async () => { } } `, - "remix.config.js": js` - export default { - serverModuleFormat: "esm", - serverBuildPath: "build/index.mjs", - future: { - v2_routeConvention: true, - }, - }; - `, "app/routes/_index.jsx": js` import { json } from "@remix-run/node"; import { useLoaderData, Link } from "@remix-run/react"; diff --git a/integration/compiler-test.ts b/integration/compiler-test.ts index 2f8b3866ed1..648a5e8ca42 100644 --- a/integration/compiler-test.ts +++ b/integration/compiler-test.ts @@ -22,6 +22,8 @@ test.describe("compiler", () => { fixture = await createFixture({ setup: "node", files: { + // We need a custom config file here to test usage of `getDependenciesToBundle` + // since this can't be serialized from the fixture object. "remix.config.js": js` let { getDependenciesToBundle } = require("@remix-run/dev"); module.exports = { @@ -386,7 +388,9 @@ test.describe("compiler", () => { await expect(() => createFixtureProject({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, buildStdio, files: { "app/routes/_index.jsx": js` diff --git a/integration/css-modules-test.ts b/integration/css-modules-test.ts index e8a1b7c9467..d2869cb35c7 100644 --- a/integration/css-modules-test.ts +++ b/integration/css-modules-test.ts @@ -19,8 +19,10 @@ test.describe("CSS Modules", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: { "app/root.jsx": js` diff --git a/integration/css-side-effect-imports-test.ts b/integration/css-side-effect-imports-test.ts index 16a90a4eb58..2dc88893ede 100644 --- a/integration/css-side-effect-imports-test.ts +++ b/integration/css-side-effect-imports-test.ts @@ -18,15 +18,13 @@ test.describe("CSS side-effect imports", () => { test.beforeAll(async () => { fixture = await createFixture({ + config: { + serverDependenciesToBundle: [/@test-package/], + future: { + v2_routeConvention: true, + }, + }, files: { - "remix.config.js": js` - module.exports = { - serverDependenciesToBundle: [/@test-package/], - future: { - v2_routeConvention: true, - }, - }; - `, "app/root.jsx": js` import { Links, Outlet } from "@remix-run/react"; import { cssBundleHref } from "@remix-run/css-bundle"; diff --git a/integration/custom-entry-server-test.ts b/integration/custom-entry-server-test.ts index c9594d40e38..86115324ca7 100644 --- a/integration/custom-entry-server-test.ts +++ b/integration/custom-entry-server-test.ts @@ -9,7 +9,9 @@ let appFixture: AppFixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/entry.server.jsx": js` import * as React from "react"; diff --git a/integration/defer-loader-test.ts b/integration/defer-loader-test.ts index 22a109b6ee2..543bcc79b00 100644 --- a/integration/defer-loader-test.ts +++ b/integration/defer-loader-test.ts @@ -9,7 +9,9 @@ let appFixture: AppFixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/_index.jsx": js` import { useLoaderData, Link } from "@remix-run/react"; diff --git a/integration/defer-test.ts b/integration/defer-test.ts index 1d952148dbf..f08f35ab7f0 100644 --- a/integration/defer-test.ts +++ b/integration/defer-test.ts @@ -33,10 +33,12 @@ test.describe("non-aborted", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, - v2_errorBoundary: true, - v2_normalizeFormMethod: true, + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + v2_normalizeFormMethod: true, + }, }, files: { "app/components/counter.tsx": js` @@ -941,7 +943,9 @@ test.describe("aborted", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, //////////////////////////////////////////////////////////////////////////// // 💿 Next, add files to this object, just like files in a real app, // `createFixture` will make an app and run your tests against it. diff --git a/integration/deno-compiler-test.ts b/integration/deno-compiler-test.ts index 3c79bc1b7e5..3c74e03f8b5 100644 --- a/integration/deno-compiler-test.ts +++ b/integration/deno-compiler-test.ts @@ -34,7 +34,9 @@ const searchFiles = async (pattern: string | RegExp, files: string[]) => { test.beforeAll(async () => { projectDir = await createFixtureProject({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, template: "deno-template", files: { "package.json": json({ diff --git a/integration/deterministic-build-output-test.ts b/integration/deterministic-build-output-test.ts index 8dfb52c732f..4004e194ab3 100644 --- a/integration/deterministic-build-output-test.ts +++ b/integration/deterministic-build-output-test.ts @@ -26,8 +26,10 @@ test("builds deterministically under different paths", async () => { // * serverRouteModulesPlugin (implicitly tested by build) // * vanillaExtractPlugin (via app/routes/foo.tsx' .css.ts file import) let init: FixtureInit = { - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: { "app/routes/_index.mdx": "# hello world", diff --git a/integration/error-boundary-test.ts b/integration/error-boundary-test.ts index 16a2406a406..a81e78abb71 100644 --- a/integration/error-boundary-test.ts +++ b/integration/error-boundary-test.ts @@ -42,7 +42,9 @@ test.describe("ErrorBoundary", () => { console.error = () => {}; fixture = await createFixture( { - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; @@ -494,8 +496,10 @@ test.describe("ErrorBoundary", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: { "app/root.jsx": js` @@ -662,7 +666,9 @@ test.describe("loaderData in ErrorBoundary", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; @@ -1008,8 +1014,10 @@ test.describe("Default ErrorBoundary", () => { fixture = await createFixture( { files: getFiles({ includeRootErrorBoundary: false }), - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, }, ServerMode.Development @@ -1081,8 +1089,10 @@ test.describe("Default ErrorBoundary", () => { test.beforeAll(async () => { fixture = await createFixture( { - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: getFiles({ includeRootErrorBoundary: true }), }, @@ -1150,8 +1160,10 @@ test.describe("Default ErrorBoundary", () => { test.beforeAll(async () => { fixture = await createFixture( { - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: getFiles({ includeRootErrorBoundary: true, @@ -1274,9 +1286,11 @@ test.describe("v2_errorBoundary", () => { console.error = () => {}; fixture = await createFixture( { - future: { - v2_routeConvention: true, - v2_errorBoundary: true, + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + }, }, files: { "app/root.jsx": js` @@ -1735,8 +1749,10 @@ test.describe("v2_errorBoundary", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: { "app/root.jsx": js` @@ -1914,9 +1930,11 @@ test.describe("v2_errorBoundary", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, - v2_errorBoundary: true, + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + }, }, files: { "app/root.jsx": js` @@ -2266,9 +2284,11 @@ test.describe("v2_errorBoundary", () => { test.beforeAll(async () => { fixture = await createFixture( { - future: { - v2_routeConvention: true, - v2_errorBoundary: true, + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + }, }, files: getFiles({ includeRootErrorBoundary: false }), }, @@ -2341,8 +2361,10 @@ test.describe("v2_errorBoundary", () => { test.beforeAll(async () => { fixture = await createFixture( { - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: getFiles({ includeRootErrorBoundary: true }), }, @@ -2409,8 +2431,10 @@ test.describe("v2_errorBoundary", () => { test.describe("When the root route has a boundary but it also throws 😦", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: getFiles({ includeRootErrorBoundary: true, diff --git a/integration/error-boundary-v2-test.ts b/integration/error-boundary-v2-test.ts index 8f0a627c576..9afb9527599 100644 --- a/integration/error-boundary-v2-test.ts +++ b/integration/error-boundary-v2-test.ts @@ -13,9 +13,11 @@ test.describe("V2 Singular ErrorBoundary (future.v2_errorBoundary)", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_errorBoundary: true, - v2_routeConvention: true, + config: { + future: { + v2_errorBoundary: true, + v2_routeConvention: true, + }, }, files: { "app/root.jsx": js` diff --git a/integration/error-data-request-test.ts b/integration/error-data-request-test.ts index e9af0467b43..f852d4786c1 100644 --- a/integration/error-data-request-test.ts +++ b/integration/error-data-request-test.ts @@ -16,7 +16,9 @@ test.describe("ErrorBoundary", () => { }; fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; diff --git a/integration/esm-only-warning-test.ts b/integration/esm-only-warning-test.ts index 7e1101226b4..d287bedd265 100644 --- a/integration/esm-only-warning-test.ts +++ b/integration/esm-only-warning-test.ts @@ -10,7 +10,9 @@ test.beforeAll(async () => { await createFixtureProject({ buildStdio, - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "package.json": json({ name: "remix-integration-9v4bpv66vd", diff --git a/integration/fetcher-layout-test.ts b/integration/fetcher-layout-test.ts index 90e3e30cccd..1a8d8c44ba8 100644 --- a/integration/fetcher-layout-test.ts +++ b/integration/fetcher-layout-test.ts @@ -9,7 +9,9 @@ let appFixture: AppFixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/layout-action.jsx": js` import { json } from "@remix-run/node"; diff --git a/integration/fetcher-state-test.ts b/integration/fetcher-state-test.ts index 22d27dda4fd..92a0021a8b6 100644 --- a/integration/fetcher-state-test.ts +++ b/integration/fetcher-state-test.ts @@ -19,7 +19,9 @@ test.describe("fetcher states", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { useMemo, useRef } from "react"; diff --git a/integration/fetcher-test.ts b/integration/fetcher-test.ts index 3e62c92a429..7c7dfda98cc 100644 --- a/integration/fetcher-test.ts +++ b/integration/fetcher-test.ts @@ -17,7 +17,9 @@ test.describe("useFetcher", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/resource-route-action-only.ts": js` import { json } from "@remix-run/node"; @@ -359,8 +361,10 @@ test.describe("fetcher aborts and adjacent forms", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: { "app/routes/_index.jsx": js` diff --git a/integration/flat-routes-test.ts b/integration/flat-routes-test.ts index 31bc29e849c..f7c6ba13e52 100644 --- a/integration/flat-routes-test.ts +++ b/integration/flat-routes-test.ts @@ -14,16 +14,13 @@ test.describe("flat routes", () => { let IGNORED_ROUTE = "/ignore-me-pls"; test.beforeAll(async () => { fixture = await createFixture({ + config: { + ignoredRouteFiles: [IGNORED_ROUTE], + future: { + v2_routeConvention: true, + }, + }, files: { - "remix.config.js": js` - /** @type {import('@remix-run/dev').AppConfig} */ - module.exports = { - future: { - v2_routeConvention: true, - }, - ignoredRouteFiles: ['${IGNORED_ROUTE}'], - }; - `, "app/root.jsx": js` import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; @@ -183,7 +180,9 @@ test.describe("warns when v1 routesConvention is used", () => { console.error = () => {}; await createFixtureProject({ buildStdio, - future: { v2_routeConvention: false }, + config: { + future: { v2_routeConvention: false }, + }, files: { "routes/index.tsx": js` export default function () { @@ -229,7 +228,9 @@ test.describe("emits warnings for route conflicts", async () => { console.error = () => {}; await createFixtureProject({ buildStdio, - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "routes/_dashboard._index.tsx": js` export default function () { @@ -285,7 +286,9 @@ test.describe("", () => { console.error = () => {}; await createFixtureProject({ buildStdio, - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/_index/route.jsx": js``, "app/routes/_index/utils.js": js``, @@ -316,7 +319,9 @@ test.describe("", () => { test.describe("pathless routes and route collisions", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.tsx": js` import { Link, Outlet, Scripts, useMatches } from "@remix-run/react"; diff --git a/integration/form-data-test.ts b/integration/form-data-test.ts index 1d272dacef7..2697c512e61 100644 --- a/integration/form-data-test.ts +++ b/integration/form-data-test.ts @@ -7,7 +7,9 @@ let fixture: Fixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/_index.jsx": js` import { json } from "@remix-run/node"; diff --git a/integration/form-test.ts b/integration/form-test.ts index b1bb6096ac1..31b2724f4ea 100644 --- a/integration/form-test.ts +++ b/integration/form-test.ts @@ -49,7 +49,9 @@ test.describe("Forms", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/get-submission.jsx": js` import { useLoaderData, Form } from "@remix-run/react"; diff --git a/integration/headers-test.ts b/integration/headers-test.ts index 9b1fed79ad0..454c83ac319 100644 --- a/integration/headers-test.ts +++ b/integration/headers-test.ts @@ -13,10 +13,12 @@ test.describe("headers export", () => { test.beforeAll(async () => { appFixture = await createFixture({ - future: { - v2_routeConvention: true, - v2_errorBoundary: true, - v2_headers: true, + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + v2_headers: true, + }, }, files: { "app/root.jsx": js` @@ -219,7 +221,9 @@ test.describe("headers export", () => { let HEADER_VALUE = "SUCCESS"; let fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; @@ -424,10 +428,12 @@ test.describe("v1 behavior (future.v2_headers=false)", () => { test.beforeAll(async () => { appFixture = await createFixture({ - future: { - v2_routeConvention: true, - v2_errorBoundary: true, - v2_headers: false, + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + v2_headers: false, + }, }, files: { "app/root.jsx": js` diff --git a/integration/helpers/cf-template/remix.config.js b/integration/helpers/cf-template/remix.config.js index 0cc72340a31..c29ad1ce95a 100644 --- a/integration/helpers/cf-template/remix.config.js +++ b/integration/helpers/cf-template/remix.config.js @@ -16,5 +16,5 @@ module.exports = { // !!! Don't adust this without changing the code that overwrites this // in createFixtureProject() - future: {}, + ...global.INJECTED_FIXTURE_REMIX_CONFIG, }; diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts index 3920557bcd9..334679fe7c5 100644 --- a/integration/helpers/create-fixture.ts +++ b/integration/helpers/create-fixture.ts @@ -3,11 +3,13 @@ import path from "node:path"; import fse from "fs-extra"; import express from "express"; import getPort from "get-port"; +import dedent from "dedent"; import stripIndent from "strip-indent"; +import serializeJavaScript from "serialize-javascript"; import { sync as spawnSync } from "cross-spawn"; import type { JsonObject } from "type-fest"; +import type { AppConfig } from "@remix-run/dev"; import { ServerMode } from "@remix-run/server-runtime/mode"; -import type { FutureConfig } from "@remix-run/server-runtime/entry"; import type { ServerBuild } from "../../build/node_modules/@remix-run/server-runtime"; import { createRequestHandler } from "../../build/node_modules/@remix-run/server-runtime"; @@ -21,9 +23,20 @@ export interface FixtureInit { files?: { [filename: string]: string }; template?: "cf-template" | "deno-template" | "node-template"; setup?: "node" | "cloudflare"; - future?: Partial; + config?: Partial; } +type MyObject = { + [key: string]: any; +}; + +type MyObjectWithoutExcludedKey = { + [K in keyof T]: K extends "keyToExclude" ? never : T[K]; +}; + +const foo: MyObjectWithoutExcludedKey = { keyToExclude: 123 }; +console.log(foo); + export type Fixture = Awaited>; export type AppFixture = Awaited>; @@ -178,22 +191,35 @@ export async function createFixtureProject( } } - if (init.future) { - let contents = fse.readFileSync( - path.join(projectDir, "remix.config.js"), - "utf-8" - ); - if (!contents.includes("future: {},")) { - throw new Error("Invalid formatted remix.config.js in template"); - } - contents = contents.replace( - "future: {},", - "future: " + JSON.stringify(init.future) + "," - ); - fse.writeFileSync(path.join(projectDir, "remix.config.js"), contents); + await writeTestFiles(init, projectDir); + + // We update the config file *after* writing test files so that tests can provide a custom + // `remix.config.js` file while still supporting the type-checked `config` + // property on the fixture object. This is useful for config values that can't + // be serialized, e.g. usage of imported functions within `remix.config.js`. + let contents = fse.readFileSync( + path.join(projectDir, "remix.config.js"), + "utf-8" + ); + if ( + init.config && + !contents.includes("global.INJECTED_FIXTURE_REMIX_CONFIG") + ) { + throw new Error(dedent` + Cannot provide a \`config\` fixture option and a \`remix.config.js\` file + at the same time, unless the \`remix.config.js\` file contains a reference + to the \`global.INJECTED_FIXTURE_REMIX_CONFIG\` placeholder so it can + accept the injected config values. Either move all config values into + \`remix.config.js\` file, or spread the injected config, + e.g. \`module.exports = { ...global.INJECTED_FIXTURE_REMIX_CONFIG }\`. + `); } + contents = contents.replace( + "global.INJECTED_FIXTURE_REMIX_CONFIG", + `${serializeJavaScript(init.config ?? {})}` + ); + fse.writeFileSync(path.join(projectDir, "remix.config.js"), contents); - await writeTestFiles(init, projectDir); build(projectDir, init.buildStdio, init.sourcemap, mode); return projectDir; diff --git a/integration/helpers/deno-template/remix.config.js b/integration/helpers/deno-template/remix.config.js index 85187b850be..696ba3c0b1a 100644 --- a/integration/helpers/deno-template/remix.config.js +++ b/integration/helpers/deno-template/remix.config.js @@ -21,5 +21,5 @@ module.exports = { // !!! Don't adust this without changing the code that overwrites this // in createFixtureProject() - future: {}, + ...global.INJECTED_FIXTURE_REMIX_CONFIG, }; diff --git a/integration/helpers/node-template/remix.config.js b/integration/helpers/node-template/remix.config.js index b7f693265aa..3f56260c2a8 100644 --- a/integration/helpers/node-template/remix.config.js +++ b/integration/helpers/node-template/remix.config.js @@ -8,5 +8,5 @@ module.exports = { // !!! Don't adust this without changing the code that overwrites this // in createFixtureProject() - future: {}, + ...global.INJECTED_FIXTURE_REMIX_CONFIG, }; diff --git a/integration/hmr-log-test.ts b/integration/hmr-log-test.ts index 2c3b62ff5f3..c1b53341e2c 100644 --- a/integration/hmr-log-test.ts +++ b/integration/hmr-log-test.ts @@ -5,28 +5,27 @@ import path from "node:path"; import type { Readable } from "node:stream"; import getPort, { makeRange } from "get-port"; +import type { FixtureInit } from "./helpers/create-fixture"; import { createFixtureProject, css, js, json } from "./helpers/create-fixture"; test.setTimeout(120_000); -let fixture = (options: { appPort: number; devPort: number }) => ({ +let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ + config: { + serverModuleFormat: "cjs", + tailwind: true, + future: { + unstable_dev: { + port: options.devPort, + }, + v2_routeConvention: true, + v2_errorBoundary: true, + v2_normalizeFormMethod: true, + v2_meta: true, + v2_headers: true, + }, + }, files: { - "remix.config.js": js` - module.exports = { - serverModuleFormat: "cjs", - tailwind: true, - future: { - unstable_dev: { - port: ${options.devPort}, - }, - v2_routeConvention: true, - v2_errorBoundary: true, - v2_normalizeFormMethod: true, - v2_meta: true, - v2_headers: true, - }, - }; - `, "package.json": json({ private: true, sideEffects: false, diff --git a/integration/hmr-test.ts b/integration/hmr-test.ts index 44631799523..f4becd5ad94 100644 --- a/integration/hmr-test.ts +++ b/integration/hmr-test.ts @@ -5,28 +5,27 @@ import path from "node:path"; import type { Readable } from "node:stream"; import getPort, { makeRange } from "get-port"; +import type { FixtureInit } from "./helpers/create-fixture"; import { createFixtureProject, css, js, json } from "./helpers/create-fixture"; test.setTimeout(120_000); -let fixture = (options: { appPort: number; devPort: number }) => ({ +let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({ + config: { + serverModuleFormat: "cjs", + tailwind: true, + future: { + unstable_dev: { + port: options.devPort, + }, + v2_routeConvention: true, + v2_errorBoundary: true, + v2_normalizeFormMethod: true, + v2_meta: true, + v2_headers: true, + }, + }, files: { - "remix.config.js": js` - module.exports = { - serverModuleFormat: "cjs", - tailwind: true, - future: { - unstable_dev: { - port: ${options.devPort}, - }, - v2_routeConvention: true, - v2_errorBoundary: true, - v2_normalizeFormMethod: true, - v2_meta: true, - v2_headers: true, - }, - }; - `, "package.json": json({ private: true, sideEffects: false, diff --git a/integration/hook-useSubmit-test.ts b/integration/hook-useSubmit-test.ts index aae3fbb6f6a..8a2b4dab938 100644 --- a/integration/hook-useSubmit-test.ts +++ b/integration/hook-useSubmit-test.ts @@ -10,7 +10,9 @@ test.describe("`useSubmit()` returned function", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/_index.jsx": js` import { useLoaderData, useSubmit } from "@remix-run/react"; diff --git a/integration/js-routes-test.ts b/integration/js-routes-test.ts index 5b3b314aeda..fa0d08c0590 100644 --- a/integration/js-routes-test.ts +++ b/integration/js-routes-test.ts @@ -10,8 +10,10 @@ test.describe(".js route files", () => { test.beforeAll(async () => { appFixture = await createAppFixture( await createFixture({ - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: { "app/routes/js.js": js` diff --git a/integration/layout-route-test.ts b/integration/layout-route-test.ts index eeab576a210..749c759188b 100644 --- a/integration/layout-route-test.ts +++ b/integration/layout-route-test.ts @@ -10,8 +10,10 @@ test.describe("pathless layout routes", () => { test.beforeAll(async () => { appFixture = await createAppFixture( await createFixture({ - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: { "app/routes/_layout.jsx": js` diff --git a/integration/link-test.ts b/integration/link-test.ts index 3a9ecc4946c..77c05d99370 100644 --- a/integration/link-test.ts +++ b/integration/link-test.ts @@ -32,9 +32,11 @@ test.describe("route module link export", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, - v2_errorBoundary: true, + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + }, }, files: { "app/favicon.ico": js``, diff --git a/integration/loader-test.ts b/integration/loader-test.ts index d0e810f0779..297db10b3d1 100644 --- a/integration/loader-test.ts +++ b/integration/loader-test.ts @@ -12,7 +12,9 @@ test.describe("loader", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { json } from "@remix-run/node"; @@ -76,8 +78,10 @@ test.describe("loader in an app", () => { test.beforeAll(async () => { appFixture = await createAppFixture( await createFixture({ - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: { "app/root.jsx": js` diff --git a/integration/matches-test.ts b/integration/matches-test.ts index 7cf0862bf03..c1b20037fe8 100644 --- a/integration/matches-test.ts +++ b/integration/matches-test.ts @@ -10,7 +10,9 @@ test.describe("useMatches", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import * as React from 'react'; diff --git a/integration/mdx-test.ts b/integration/mdx-test.ts index cd68870d169..28e628a7736 100644 --- a/integration/mdx-test.ts +++ b/integration/mdx-test.ts @@ -16,7 +16,9 @@ test.describe("mdx", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; diff --git a/integration/meta-test.ts b/integration/meta-test.ts index 9274e9e7a2f..a04a61e91bc 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -15,7 +15,9 @@ test.describe("meta", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { json } from "@remix-run/node"; @@ -397,17 +399,14 @@ test.describe("v2_meta", () => { test.beforeAll(async () => { fixture = await createFixture({ + config: { + ignoredRouteFiles: ["**/.*"], + future: { + v2_meta: true, + v2_routeConvention: true, + }, + }, files: { - "remix.config.js": js` - module.exports = { - ignoredRouteFiles: ["**/.*"], - future: { - v2_meta: true, - v2_routeConvention: true, - }, - }; - `, - "app/root.jsx": js` import { json } from "@remix-run/node"; import { Meta, Links, Outlet, Scripts } from "@remix-run/react"; diff --git a/integration/multiple-cookies-test.ts b/integration/multiple-cookies-test.ts index 7aacbb2152d..7b261cb55de 100644 --- a/integration/multiple-cookies-test.ts +++ b/integration/multiple-cookies-test.ts @@ -10,8 +10,10 @@ test.describe("pathless layout routes", () => { test.beforeAll(async () => { appFixture = await createAppFixture( await createFixture({ - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: { "app/routes/_index.jsx": js` diff --git a/integration/navigation-state-v2-test.ts b/integration/navigation-state-v2-test.ts index e87053a636f..66bbfe24548 100644 --- a/integration/navigation-state-v2-test.ts +++ b/integration/navigation-state-v2-test.ts @@ -27,8 +27,10 @@ test.describe("navigation states", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_normalizeFormMethod: true, + config: { + future: { + v2_normalizeFormMethod: true, + }, }, files: { "app/root.jsx": js` diff --git a/integration/path-mapping-test.ts b/integration/path-mapping-test.ts index 4064b9903e3..6e8ea1ffb21 100644 --- a/integration/path-mapping-test.ts +++ b/integration/path-mapping-test.ts @@ -7,7 +7,9 @@ let fixture: Fixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/components/my-lib/index.ts": js` export const pizza = "this is a pizza"; diff --git a/integration/postcss-test.ts b/integration/postcss-test.ts index b476f90aad2..c78c7bf9946 100644 --- a/integration/postcss-test.ts +++ b/integration/postcss-test.ts @@ -33,16 +33,14 @@ test.describe("PostCSS enabled", () => { test.beforeAll(async () => { fixture = await createFixture({ + config: { + postcss: true, + tailwind: true, + future: { + v2_routeConvention: true, + }, + }, files: { - "remix.config.js": js` - module.exports = { - postcss: true, - tailwind: true, - future: { - v2_routeConvention: true, - }, - }; - `, // We provide a test plugin that replaces the strings // "TEST_PADDING_VALUE" and "TEST_POSTCSS_CONTEXT". // This lets us assert that the plugin is being run @@ -358,8 +356,10 @@ test.describe("PostCSS enabled via unstable future flag", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - unstable_postcss: true, + config: { + future: { + unstable_postcss: true, + }, }, files: { "postcss.config.js": js` @@ -442,12 +442,10 @@ test.describe("PostCSS disabled", () => { test.beforeAll(async () => { fixture = await createFixture({ + config: { + postcss: false, + }, files: { - "remix.config.js": js` - module.exports = { - postcss: false, - }; - `, "postcss.config.js": js` module.exports = (ctx) => ({ plugins: [ diff --git a/integration/prefetch-test.ts b/integration/prefetch-test.ts index 62e8fcde5e5..b005c46a774 100644 --- a/integration/prefetch-test.ts +++ b/integration/prefetch-test.ts @@ -1,14 +1,20 @@ import { test, expect } from "@playwright/test"; import { createAppFixture, createFixture, js } from "./helpers/create-fixture"; -import type { Fixture, AppFixture } from "./helpers/create-fixture"; +import type { + Fixture, + FixtureInit, + AppFixture, +} from "./helpers/create-fixture"; import type { RemixLinkProps } from "../build/node_modules/@remix-run/react/dist/components"; import { PlaywrightFixture } from "./helpers/playwright-fixture"; // Generate the test app using the given prefetch mode -function fixtureFactory(mode: RemixLinkProps["prefetch"]) { +function fixtureFactory(mode: RemixLinkProps["prefetch"]): FixtureInit { return { - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { diff --git a/integration/redirects-test.ts b/integration/redirects-test.ts index 73a4897e6c3..afdb7eddb4e 100644 --- a/integration/redirects-test.ts +++ b/integration/redirects-test.ts @@ -10,7 +10,9 @@ test.describe("redirects", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/action.jsx": js` import { Outlet, useLoaderData } from "@remix-run/react"; diff --git a/integration/rendering-test.ts b/integration/rendering-test.ts index 0724ea9a050..44a357a84f3 100644 --- a/integration/rendering-test.ts +++ b/integration/rendering-test.ts @@ -10,7 +10,9 @@ test.describe("rendering", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; diff --git a/integration/request-test.ts b/integration/request-test.ts index 27c4707143b..9458f456263 100644 --- a/integration/request-test.ts +++ b/integration/request-test.ts @@ -9,7 +9,9 @@ let appFixture: AppFixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/_index.jsx": js` import { json } from "@remix-run/node"; diff --git a/integration/resource-routes-test.ts b/integration/resource-routes-test.ts index 82d1218e271..84b4f670ec3 100644 --- a/integration/resource-routes-test.ts +++ b/integration/resource-routes-test.ts @@ -16,7 +16,9 @@ test.describe("loader in an app", async () => { _consoleError = console.error; console.error = () => {}; fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/_index.jsx": js` import { Form, Link } from "@remix-run/react"; @@ -238,9 +240,11 @@ test.describe("Development server", async () => { fixture = await createFixture( { - future: { - v2_routeConvention: true, - v2_errorBoundary: true, + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + }, }, files: { "app/routes/_index.jsx": js` diff --git a/integration/revalidate-test.ts b/integration/revalidate-test.ts index 00ed60ca619..492245de0f8 100644 --- a/integration/revalidate-test.ts +++ b/integration/revalidate-test.ts @@ -10,8 +10,10 @@ test.describe("Revalidation", () => { test.beforeAll(async () => { appFixture = await createAppFixture( await createFixture({ - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: { "app/root.jsx": js` diff --git a/integration/route-collisions-test.ts b/integration/route-collisions-test.ts index d6ea3606804..f136d223cb0 100644 --- a/integration/route-collisions-test.ts +++ b/integration/route-collisions-test.ts @@ -49,7 +49,9 @@ test.describe("build failures (v1 routes)", () => { test("detects path collisions inside pathless layout routes", async () => { try { await createFixture({ - future: { v2_routeConvention: false }, + config: { + future: { v2_routeConvention: false }, + }, files: { "app/root.tsx": ROOT_FILE_CONTENTS, "app/routes/foo.jsx": LEAF_FILE_CONTENTS, @@ -69,7 +71,9 @@ test.describe("build failures (v1 routes)", () => { test("detects path collisions across pathless layout routes", async () => { try { await createFixture({ - future: { v2_routeConvention: false }, + config: { + future: { v2_routeConvention: false }, + }, files: { "app/root.tsx": ROOT_FILE_CONTENTS, "app/routes/__pathless.jsx": LAYOUT_FILE_CONTENTS, @@ -90,7 +94,9 @@ test.describe("build failures (v1 routes)", () => { test("detects path collisions inside multiple pathless layout routes", async () => { try { await createFixture({ - future: { v2_routeConvention: false }, + config: { + future: { v2_routeConvention: false }, + }, files: { "app/root.tsx": ROOT_FILE_CONTENTS, "app/routes/foo.jsx": LEAF_FILE_CONTENTS, @@ -111,7 +117,9 @@ test.describe("build failures (v1 routes)", () => { test("detects path collisions of index files inside pathless layouts", async () => { try { await createFixture({ - future: { v2_routeConvention: false }, + config: { + future: { v2_routeConvention: false }, + }, files: { "app/root.tsx": ROOT_FILE_CONTENTS, "app/routes/index.jsx": LEAF_FILE_CONTENTS, @@ -131,7 +139,9 @@ test.describe("build failures (v1 routes)", () => { test("detects path collisions of index files across multiple pathless layouts", async () => { try { await createFixture({ - future: { v2_routeConvention: false }, + config: { + future: { v2_routeConvention: false }, + }, files: { "app/root.tsx": ROOT_FILE_CONTENTS, "app/routes/nested/__pathless.jsx": LAYOUT_FILE_CONTENTS, @@ -152,7 +162,9 @@ test.describe("build failures (v1 routes)", () => { test("detects path collisions of param routes inside pathless layouts", async () => { try { await createFixture({ - future: { v2_routeConvention: false }, + config: { + future: { v2_routeConvention: false }, + }, files: { "app/root.tsx": ROOT_FILE_CONTENTS, "app/routes/$param.jsx": LEAF_FILE_CONTENTS, @@ -192,7 +204,9 @@ test.describe("build failures (v2 routes)", () => { let buildOutput: string; await createFixture({ buildStdio, - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files, }); let chunks: Buffer[] = []; diff --git a/integration/scroll-test.ts b/integration/scroll-test.ts index 534476fd01d..70d1a4eec0c 100644 --- a/integration/scroll-test.ts +++ b/integration/scroll-test.ts @@ -9,7 +9,9 @@ let appFixture: AppFixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/_index.jsx": js` import { redirect } from "@remix-run/node"; diff --git a/integration/server-code-in-browser-message-test.ts b/integration/server-code-in-browser-message-test.ts index ee58bdae575..f1221620e02 100644 --- a/integration/server-code-in-browser-message-test.ts +++ b/integration/server-code-in-browser-message-test.ts @@ -14,7 +14,9 @@ let appFixture: AppFixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "node_modules/has-side-effects/package.json": json({ name: "has-side-effects", diff --git a/integration/server-entry-test.ts b/integration/server-entry-test.ts index 477880f5939..029b8caec61 100644 --- a/integration/server-entry-test.ts +++ b/integration/server-entry-test.ts @@ -12,7 +12,9 @@ test.describe("Custom Server Entry", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/entry.server.jsx": js` export default function handleRequest() { diff --git a/integration/server-source-maps-test.ts b/integration/server-source-maps-test.ts index ca4e61ad193..f0833d7c18d 100644 --- a/integration/server-source-maps-test.ts +++ b/integration/server-source-maps-test.ts @@ -9,7 +9,9 @@ let fixture: Fixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, sourcemap: true, files: { "app/routes/_index.jsx": js` diff --git a/integration/set-cookie-revalidation-test.ts b/integration/set-cookie-revalidation-test.ts index bf49d2945f0..017bcb6a3a1 100644 --- a/integration/set-cookie-revalidation-test.ts +++ b/integration/set-cookie-revalidation-test.ts @@ -11,7 +11,9 @@ let BANNER_MESSAGE = "you do not have permission to view /protected"; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/session.server.js": js` import { createCookieSessionStorage } from "@remix-run/node"; diff --git a/integration/shared-route-imports-test.ts b/integration/shared-route-imports-test.ts index 474adcea092..cbedef0fad6 100644 --- a/integration/shared-route-imports-test.ts +++ b/integration/shared-route-imports-test.ts @@ -10,7 +10,9 @@ let appFixture: AppFixture; test.describe("v1 compiler", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/parent.jsx": js` import { createContext, useContext } from "react"; @@ -98,7 +100,9 @@ export function UseParentContext() { test.describe("v2 compiler", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true, unstable_dev: true }, + config: { + future: { v2_routeConvention: true, unstable_dev: true }, + }, files: { "app/routes/parent.jsx": js` import { createContext, useContext } from "react"; diff --git a/integration/splat-routes-test.ts b/integration/splat-routes-test.ts index 0a15ae5e68b..17e37e5afa0 100644 --- a/integration/splat-routes-test.ts +++ b/integration/splat-routes-test.ts @@ -16,7 +16,9 @@ test.describe("rendering", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; diff --git a/integration/tailwind-test.ts b/integration/tailwind-test.ts index 12e53209b95..cda2f2ad845 100644 --- a/integration/tailwind-test.ts +++ b/integration/tailwind-test.ts @@ -43,16 +43,13 @@ function runTests(ext: typeof extensions[number]) { test.beforeAll(async () => { fixture = await createFixture({ + config: { + tailwind: true, + future: { + v2_routeConvention: true, + }, + }, files: { - "remix.config.js": js` - module.exports = { - tailwind: true, - future: { - v2_routeConvention: true, - }, - }; - `, - [tailwindConfigName]: tailwindConfig, "app/tailwind.css": css` @@ -338,8 +335,10 @@ test.describe("Tailwind enabled via unstable future flag", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - unstable_tailwind: true, + config: { + future: { + unstable_tailwind: true, + }, }, files: { "tailwind.config.js": js` @@ -413,13 +412,10 @@ test.describe("Tailwind disabled", () => { test.beforeAll(async () => { fixture = await createFixture({ + config: { + tailwind: false, + }, files: { - "remix.config.js": js` - module.exports = { - tailwind: false, - }; - `, - "tailwind.config.js": js` module.exports = { content: ["./app/**/*.{ts,tsx,jsx,js}"], diff --git a/integration/transition-state-test.ts b/integration/transition-state-test.ts index 6e2fbcfc69e..c44aa7d6a91 100644 --- a/integration/transition-state-test.ts +++ b/integration/transition-state-test.ts @@ -25,7 +25,9 @@ test.describe("rendering", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { useMemo, useRef } from "react"; diff --git a/integration/transition-test.ts b/integration/transition-test.ts index b73f41945e1..a1a99f4f33c 100644 --- a/integration/transition-test.ts +++ b/integration/transition-test.ts @@ -19,7 +19,9 @@ test.describe("rendering", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/root.jsx": js` import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; diff --git a/integration/upload-test.ts b/integration/upload-test.ts index 9c2e9be3a25..ba08e5f00f4 100644 --- a/integration/upload-test.ts +++ b/integration/upload-test.ts @@ -10,7 +10,9 @@ let appFixture: AppFixture; test.beforeAll(async () => { fixture = await createFixture({ - future: { v2_routeConvention: true }, + config: { + future: { v2_routeConvention: true }, + }, files: { "app/routes/file-upload-handler.jsx": js` import { diff --git a/integration/vanilla-extract-test.ts b/integration/vanilla-extract-test.ts index 119f974f5cd..47cc7e335c8 100644 --- a/integration/vanilla-extract-test.ts +++ b/integration/vanilla-extract-test.ts @@ -12,8 +12,10 @@ test.describe("Vanilla Extract", () => { test.beforeAll(async () => { fixture = await createFixture({ - future: { - v2_routeConvention: true, + config: { + future: { + v2_routeConvention: true, + }, }, files: { "app/root.jsx": js` diff --git a/package.json b/package.json index 6c42995821f..2cdbef9827e 100644 --- a/package.json +++ b/package.json @@ -74,6 +74,7 @@ "@testing-library/react": "^13.3.0", "@types/cheerio": "^0.22.22", "@types/cross-spawn": "^6.0.2", + "@types/dedent": "^0.7.0", "@types/glob": "7.2.0", "@types/jest": "^27.4.1", "@types/jsonfile": "^6.1.0", @@ -83,6 +84,7 @@ "@types/react-test-renderer": "^18.0.0", "@types/retry": "^0.12.0", "@types/semver": "^7.3.4", + "@types/serialize-javascript": "^5.0.2", "@types/ssri": "^7.1.0", "@vanilla-extract/css": "^1.10.0", "abort-controller": "^3.0.0", @@ -95,6 +97,7 @@ "cross-env": "^7.0.3", "cross-spawn": "^7.0.3", "cypress": "^9.6.0", + "dedent": "^0.7.0", "eslint": "^8.23.1", "eslint-plugin-markdown": "^2.2.1", "eslint-plugin-prefer-let": "^3.0.1", @@ -121,6 +124,7 @@ "rollup": "^2.36.1", "rollup-plugin-copy": "^3.3.0", "semver": "^7.3.7", + "serialize-javascript": "^6.0.1", "simple-git": "^3.16.0", "sort-package-json": "^1.55.0", "strip-indent": "^3.0.0", diff --git a/packages/remix-dev/package.json b/packages/remix-dev/package.json index d790225b8a8..2ae2d9b9711 100644 --- a/packages/remix-dev/package.json +++ b/packages/remix-dev/package.json @@ -83,7 +83,6 @@ "@types/shelljs": "^0.8.11", "@types/tar-fs": "^2.0.1", "@types/ws": "^7.4.1", - "dedent": "^0.7.0", "esbuild-register": "^3.3.2", "msw": "^0.39.2", "shelljs": "^0.8.5", diff --git a/yarn.lock b/yarn.lock index 81b41895ba5..f8f79349c6e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2962,6 +2962,11 @@ dependencies: "@types/ms" "*" +"@types/dedent@^0.7.0": + version "0.7.0" + resolved "https://registry.npmjs.org/@types/dedent/-/dedent-0.7.0.tgz#155f339ca404e6dd90b9ce46a3f78fd69ca9b050" + integrity sha512-EGlKlgMhnLt/cM4DbUSafFdrkeJoC9Mvnj0PUCU7tFmTjMjNRT957kXCx0wYm3JuEq4o4ZsS5vG+NlkM2DMd2A== + "@types/eslint@^8.37.0": version "8.37.0" resolved "https://registry.npmjs.org/@types/eslint/-/eslint-8.37.0.tgz#29cebc6c2a3ac7fea7113207bf5a828fdf4d7ef1" @@ -3351,6 +3356,11 @@ resolved "https://registry.npmjs.org/@types/semver/-/semver-7.3.8.tgz" integrity sha512-D/2EJvAlCEtYFEYmmlGwbGXuK886HzyCc3nZX/tkFTQdEU8jZDAgiv08P162yB17y4ZXZoq7yFAnW4GDBb9Now== +"@types/serialize-javascript@^5.0.2": + version "5.0.2" + resolved "https://registry.npmjs.org/@types/serialize-javascript/-/serialize-javascript-5.0.2.tgz#c4b29f763e407def2502c9dfcc0b8c4c96ef0387" + integrity sha512-BRLlwZzRoZukGaBtcUxkLsZsQfWZpvog6MZk3PWQO9Q6pXmXFzjU5iGzZ+943evp6tkkbN98N1Z31KT0UG1yRw== + "@types/serve-static@*": version "1.13.10" resolved "https://registry.npmjs.org/@types/serve-static/-/serve-static-1.13.10.tgz" @@ -11344,6 +11354,13 @@ random-bytes@~1.0.0: resolved "https://registry.npmjs.org/random-bytes/-/random-bytes-1.0.0.tgz" integrity sha1-T2ih3Arli9P7lYSMMDJNt11kNgs= +randombytes@^2.1.0: + version "2.1.0" + resolved "https://registry.npmjs.org/randombytes/-/randombytes-2.1.0.tgz#df6f84372f0270dc65cdf6291349ab7a473d4f2a" + integrity sha512-vYl3iOX+4CKUWuxGi9Ukhie6fsqXqS9FE2Zaic4tNFD2N2QQaXOMFbuKK4QmDHC0JO6B1Zp41J0LpT0oR68amQ== + dependencies: + safe-buffer "^5.1.0" + range-parser@^1.2.0, range-parser@~1.2.1: version "1.2.1" resolved "https://registry.npmjs.org/range-parser/-/range-parser-1.2.1.tgz" @@ -11916,7 +11933,7 @@ safe-buffer@5.1.2, safe-buffer@^5.0.1, safe-buffer@^5.1.2, safe-buffer@~5.1.0, s resolved "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.2.tgz" integrity sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g== -safe-buffer@5.2.1: +safe-buffer@5.2.1, safe-buffer@^5.1.0: version "5.2.1" resolved "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.1.tgz" integrity sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ== @@ -11985,6 +12002,13 @@ send@0.17.2: range-parser "~1.2.1" statuses "~1.5.0" +serialize-javascript@^6.0.1: + version "6.0.1" + resolved "https://registry.npmjs.org/serialize-javascript/-/serialize-javascript-6.0.1.tgz#b206efb27c3da0b0ab6b52f48d170b7996458e5c" + integrity sha512-owoXEFjWRllis8/M1Q+Cw5k8ZH40e3zhp/ovX+Xr/vi1qj6QesbyXXViFbpNvWvPNAD62SutwEXavefrLJWj7w== + dependencies: + randombytes "^2.1.0" + serve-static@1.14.2: version "1.14.2" resolved "https://registry.npmjs.org/serve-static/-/serve-static-1.14.2.tgz" From a6b5532cf6187319318ea635c93b8da3673c3de2 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Wed, 31 May 2023 15:19:46 +1000 Subject: [PATCH 25/29] refactor: clean up leftover debugging code (#6510) --- integration/helpers/create-fixture.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts index 334679fe7c5..3c2e31ed9ca 100644 --- a/integration/helpers/create-fixture.ts +++ b/integration/helpers/create-fixture.ts @@ -26,17 +26,6 @@ export interface FixtureInit { config?: Partial; } -type MyObject = { - [key: string]: any; -}; - -type MyObjectWithoutExcludedKey = { - [K in keyof T]: K extends "keyToExclude" ? never : T[K]; -}; - -const foo: MyObjectWithoutExcludedKey = { keyToExclude: 123 }; -console.log(foo); - export type Fixture = Awaited>; export type AppFixture = Awaited>; From 76218ed0e9e921f4778655bf49da273c9704412e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20De=20Boey?= Date: Wed, 31 May 2023 15:46:41 +0200 Subject: [PATCH 26/29] templates: enable `v2_headers` future flag (#6490) --- templates/arc/remix.config.js | 1 + templates/cloudflare-pages/remix.config.js | 1 + templates/cloudflare-workers/remix.config.js | 1 + templates/deno/remix.config.js | 1 + templates/express/remix.config.js | 1 + templates/fly/remix.config.js | 1 + templates/netlify/remix.config.js | 1 + templates/remix/remix.config.js | 1 + templates/vercel/remix.config.js | 1 + 9 files changed, 9 insertions(+) diff --git a/templates/arc/remix.config.js b/templates/arc/remix.config.js index 06a93128b2e..b76efda0f3a 100644 --- a/templates/arc/remix.config.js +++ b/templates/arc/remix.config.js @@ -8,6 +8,7 @@ module.exports = { // assetsBuildDirectory: "public/build", future: { v2_errorBoundary: true, + v2_headers: true, v2_meta: true, v2_normalizeFormMethod: true, v2_routeConvention: true, diff --git a/templates/cloudflare-pages/remix.config.js b/templates/cloudflare-pages/remix.config.js index 7aa3011d30f..6f5ffe0fffe 100644 --- a/templates/cloudflare-pages/remix.config.js +++ b/templates/cloudflare-pages/remix.config.js @@ -15,6 +15,7 @@ module.exports = { // publicPath: "/build/", future: { v2_errorBoundary: true, + v2_headers: true, v2_meta: true, v2_normalizeFormMethod: true, v2_routeConvention: true, diff --git a/templates/cloudflare-workers/remix.config.js b/templates/cloudflare-workers/remix.config.js index 07cb0fd2811..c4210dd979a 100644 --- a/templates/cloudflare-workers/remix.config.js +++ b/templates/cloudflare-workers/remix.config.js @@ -15,6 +15,7 @@ module.exports = { // publicPath: "/build/", future: { v2_errorBoundary: true, + v2_headers: true, v2_meta: true, v2_normalizeFormMethod: true, v2_routeConvention: true, diff --git a/templates/deno/remix.config.js b/templates/deno/remix.config.js index e2fe7337c15..d0c78e97eef 100644 --- a/templates/deno/remix.config.js +++ b/templates/deno/remix.config.js @@ -20,6 +20,7 @@ module.exports = { // publicPath: "/build/", future: { v2_errorBoundary: true, + v2_headers: true, v2_meta: true, v2_normalizeFormMethod: true, v2_routeConvention: true, diff --git a/templates/express/remix.config.js b/templates/express/remix.config.js index 30d26861b22..29287a0d890 100644 --- a/templates/express/remix.config.js +++ b/templates/express/remix.config.js @@ -8,6 +8,7 @@ module.exports = { serverModuleFormat: "cjs", future: { v2_errorBoundary: true, + v2_headers: true, v2_meta: true, v2_normalizeFormMethod: true, v2_routeConvention: true, diff --git a/templates/fly/remix.config.js b/templates/fly/remix.config.js index a1a396661bf..cf359205c5e 100644 --- a/templates/fly/remix.config.js +++ b/templates/fly/remix.config.js @@ -7,6 +7,7 @@ module.exports = { // publicPath: "/build/", future: { v2_errorBoundary: true, + v2_headers: true, v2_meta: true, v2_normalizeFormMethod: true, v2_routeConvention: true, diff --git a/templates/netlify/remix.config.js b/templates/netlify/remix.config.js index 17af2a70863..c510d3cc2d2 100644 --- a/templates/netlify/remix.config.js +++ b/templates/netlify/remix.config.js @@ -11,6 +11,7 @@ module.exports = { // publicPath: "/build/", future: { v2_errorBoundary: true, + v2_headers: true, v2_meta: true, v2_normalizeFormMethod: true, v2_routeConvention: true, diff --git a/templates/remix/remix.config.js b/templates/remix/remix.config.js index 30d26861b22..29287a0d890 100644 --- a/templates/remix/remix.config.js +++ b/templates/remix/remix.config.js @@ -8,6 +8,7 @@ module.exports = { serverModuleFormat: "cjs", future: { v2_errorBoundary: true, + v2_headers: true, v2_meta: true, v2_normalizeFormMethod: true, v2_routeConvention: true, diff --git a/templates/vercel/remix.config.js b/templates/vercel/remix.config.js index f5aa112961e..79e2b95fd7e 100644 --- a/templates/vercel/remix.config.js +++ b/templates/vercel/remix.config.js @@ -11,6 +11,7 @@ module.exports = { // publicPath: "/build/", future: { v2_errorBoundary: true, + v2_headers: true, v2_meta: true, v2_normalizeFormMethod: true, v2_routeConvention: true, From 65d601c93df277a4909973bb966f44d997ddb4fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20De=20Boey?= Date: Wed, 31 May 2023 16:11:14 +0200 Subject: [PATCH 27/29] fix(remix-dev): loosen `prettier` version (#6488) --- package.json | 2 +- .../fixtures/replace-remix-magic-imports/package.json | 2 +- packages/remix-dev/package.json | 2 +- yarn.lock | 5 ----- 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 2cdbef9827e..bce85e6c015 100644 --- a/package.json +++ b/package.json @@ -112,7 +112,7 @@ "lodash": "^4.17.21", "npm-run-all": "^4.1.5", "patch-package": "^6.5.0", - "prettier": "2.7.1", + "prettier": "^2.7.1", "prompt-confirm": "^2.0.4", "react": "^18.2.0", "react-dom": "^18.2.0", diff --git a/packages/remix-dev/__tests__/fixtures/replace-remix-magic-imports/package.json b/packages/remix-dev/__tests__/fixtures/replace-remix-magic-imports/package.json index bcbdf75e47a..db097f595ab 100644 --- a/packages/remix-dev/__tests__/fixtures/replace-remix-magic-imports/package.json +++ b/packages/remix-dev/__tests__/fixtures/replace-remix-magic-imports/package.json @@ -51,7 +51,7 @@ "happy-dom": "^2.49.0", "msw": "^0.39.2", "npm-run-all": "^4.1.5", - "prettier": "2.6.0", + "prettier": "^2.7.1", "prettier-plugin-tailwindcss": "^0.1.8", "prisma": "^3.11.0", "start-server-and-test": "^1.14.0", diff --git a/packages/remix-dev/package.json b/packages/remix-dev/package.json index 2ae2d9b9711..952bab9f446 100644 --- a/packages/remix-dev/package.json +++ b/packages/remix-dev/package.json @@ -57,7 +57,7 @@ "postcss-discard-duplicates": "^5.1.0", "postcss-load-config": "^4.0.1", "postcss-modules": "^6.0.0", - "prettier": "2.7.1", + "prettier": "^2.7.1", "pretty-ms": "^7.0.1", "proxy-agent": "^5.0.0", "react-refresh": "^0.14.0", diff --git a/yarn.lock b/yarn.lock index f8f79349c6e..c0d4a6ba4a7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11115,11 +11115,6 @@ prelude-ls@~1.1.2: resolved "https://registry.npmjs.org/prelude-ls/-/prelude-ls-1.1.2.tgz" integrity sha1-IZMqVJ9eUv/ZqCf1cOBL5iqX2lQ= -prettier@2.7.1: - version "2.7.1" - resolved "https://registry.npmjs.org/prettier/-/prettier-2.7.1.tgz" - integrity sha512-ujppO+MkdPqoVINuDFDRLClm7D78qbDt0/NR+wp5FqEZOoTNAjPHWj17QRhu7geIHJfcNhRk1XVQmF8Bp3ye+g== - prettier@^2.7.1: version "2.8.1" resolved "https://registry.npmjs.org/prettier/-/prettier-2.8.1.tgz" From 92a570ee7773242064d50bd0b7a5e406f4f3eb22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20De=20Boey?= Date: Wed, 31 May 2023 16:11:30 +0200 Subject: [PATCH 28/29] refactor(remix-react): remove unnecessary type-checks (#6323) --- packages/remix-react/components.tsx | 26 ++++++++----------------- packages/remix-react/data.ts | 30 +++++++++-------------------- 2 files changed, 17 insertions(+), 39 deletions(-) diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 6e364e1e727..17fbe9313ca 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -7,7 +7,6 @@ import * as React from "react"; import type { AgnosticDataRouteMatch, UNSAFE_DeferredData as DeferredData, - ErrorResponse, Navigation, TrackedPromise, } from "@remix-run/router"; @@ -174,23 +173,14 @@ export function RemixRouteError({ id }: { id: string }) { } if (isRouteErrorResponse(error)) { - let tError = error as any; - if ( - tError?.error instanceof Error && - tError.status !== 404 && - ErrorBoundary - ) { + let tError = error; + if (!!tError?.error && tError.status !== 404 && ErrorBoundary) { // Internal framework-thrown ErrorResponses return ; } if (CatchBoundary) { // User-thrown ErrorResponses - return ( - - ); + return ; } } @@ -222,11 +212,11 @@ export interface RemixNavLinkProps extends NavLinkProps { } interface PrefetchHandlers { - onFocus?: FocusEventHandler; - onBlur?: FocusEventHandler; - onMouseEnter?: MouseEventHandler; - onMouseLeave?: MouseEventHandler; - onTouchStart?: TouchEventHandler; + onFocus?: FocusEventHandler; + onBlur?: FocusEventHandler; + onMouseEnter?: MouseEventHandler; + onMouseLeave?: MouseEventHandler; + onTouchStart?: TouchEventHandler; } function usePrefetchBehavior( diff --git a/packages/remix-react/data.ts b/packages/remix-react/data.ts index 4991ec8720d..a065b65c407 100644 --- a/packages/remix-react/data.ts +++ b/packages/remix-react/data.ts @@ -10,32 +10,20 @@ import { */ export type AppData = any; -export function isCatchResponse(response: any): boolean { - return ( - response instanceof Response && - response.headers.get("X-Remix-Catch") != null - ); +export function isCatchResponse(response: Response): boolean { + return response.headers.get("X-Remix-Catch") != null; } -export function isErrorResponse(response: any): boolean { - return ( - response instanceof Response && - response.headers.get("X-Remix-Error") != null - ); +export function isErrorResponse(response: Response): boolean { + return response.headers.get("X-Remix-Error") != null; } -export function isRedirectResponse(response: any): boolean { - return ( - response instanceof Response && - response.headers.get("X-Remix-Redirect") != null - ); +export function isRedirectResponse(response: Response): boolean { + return response.headers.get("X-Remix-Redirect") != null; } -export function isDeferredResponse(response: any): boolean { - return ( - response instanceof Response && - !!response.headers.get("Content-Type")?.match(/text\/remix-deferred/) - ); +export function isDeferredResponse(response: Response): boolean { + return !!response.headers.get("Content-Type")?.match(/text\/remix-deferred/); } export async function fetchData( @@ -106,7 +94,7 @@ export async function parseDeferredReadableStream( deferredData = deferredData || {}; - deferredData[eventKey] = new Promise((resolve, reject) => { + deferredData[eventKey] = new Promise((resolve, reject) => { deferredResolvers[eventKey] = { resolve: (value: unknown) => { resolve(value); From 08aedc4e37b9f8ec86472e6de571fd34233ad628 Mon Sep 17 00:00:00 2001 From: Remix Run Bot Date: Wed, 31 May 2023 14:14:15 +0000 Subject: [PATCH 29/29] chore: format --- docs/guides/styling.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/guides/styling.md b/docs/guides/styling.md index 8a021efad76..3f95507db65 100644 --- a/docs/guides/styling.md +++ b/docs/guides/styling.md @@ -432,11 +432,11 @@ Now we can tell it which files to generate classes from: import type { Config } from "tailwindcss"; export default { - content: ["./app/**/*.{js,jsx,ts,tsx}"], - theme: { - extend: {}, - }, - plugins: [], + content: ["./app/**/*.{js,jsx,ts,tsx}"], + theme: { + extend: {}, + }, + plugins: [], } satisfies Config; ```