From 5b9eb2917682e13389f4b91152cb97e665999092 Mon Sep 17 00:00:00 2001 From: Leo Singer Date: Tue, 15 Aug 2023 20:33:08 -0400 Subject: [PATCH] fix(remix-dev): fix server builds where serverBuildPath ends in .cjs Fix a bug that caused the server build file to be emitted into the assets directory if the value of `serverBuildPath` ended in `.cjs`. The bug was due to the server-side portion of the Remix compiler only iterating over files ending in `.js` and `.mjs`, but not `.cjs`. Note that this diff is smaller than it appears: the largest number of lines affected are in integration/compiler-mjs-cjs-output-test.ts, in which I increased the indentation of the test fixture code in order to parametrize it over file extension. --- .changeset/wild-steaks-bathe.md | 7 ++ integration/compiler-mjs-cjs-output-test.ts | 83 +++++++++++++++++++++ integration/compiler-mjs-output-test.ts | 76 ------------------- packages/remix-dev/compiler/server/write.ts | 2 +- 4 files changed, 91 insertions(+), 77 deletions(-) create mode 100644 .changeset/wild-steaks-bathe.md create mode 100644 integration/compiler-mjs-cjs-output-test.ts delete mode 100644 integration/compiler-mjs-output-test.ts diff --git a/.changeset/wild-steaks-bathe.md b/.changeset/wild-steaks-bathe.md new file mode 100644 index 00000000000..25a35dce28b --- /dev/null +++ b/.changeset/wild-steaks-bathe.md @@ -0,0 +1,7 @@ +--- +"@remix-run/dev": patch +--- + +Fix server builds where serverBuildPath extension is `.cjs`. + +Fix a bug that caused the server build file to be emitted into the assets directory if the value of `serverBuildPath` ended in `.cjs`. diff --git a/integration/compiler-mjs-cjs-output-test.ts b/integration/compiler-mjs-cjs-output-test.ts new file mode 100644 index 00000000000..e84c1dce451 --- /dev/null +++ b/integration/compiler-mjs-cjs-output-test.ts @@ -0,0 +1,83 @@ +import { test, expect } from "@playwright/test"; +import * as fs from "node:fs"; +import * as path from "node:path"; + +import { createFixtureProject, js } from "./helpers/create-fixture.js"; + +test.describe("", () => { + for (let [serverModuleExt, serverModuleFormat, exportStatement] of [ + ["mjs", "esm", "export {"], + ["cjs", "cjs", "module.exports ="], + ]) { + test(`can write .${serverModuleExt} server output module`, async () => { + let projectDir = await createFixtureProject({ + files: { + // Ensure the config is valid ESM + "remix.config.js": js` + export default { + serverModuleFormat: "${serverModuleFormat}", + serverBuildPath: "build/index.${serverModuleExt}", + }; + `, + "package.json": js` + { + "name": "remix-template-remix", + "private": true, + "sideEffects": false, + "type": "module", + "scripts": { + "build": "node ../../../build/node_modules/@remix-run/dev/dist/cli.js build", + "dev": "node ../../../build/node_modules/@remix-run/dev/dist/cli.js dev", + "start": "node ../../../build/node_modules/@remix-run/serve/dist/cli.js build" + }, + "dependencies": { + "@remix-run/node": "0.0.0-local-version", + "@remix-run/react": "0.0.0-local-version", + "@remix-run/serve": "0.0.0-local-version", + "isbot": "0.0.0-local-version", + "react": "0.0.0-local-version", + "react-dom": "0.0.0-local-version" + }, + "devDependencies": { + "@remix-run/dev": "0.0.0-local-version", + "@types/react": "0.0.0-local-version", + "@types/react-dom": "0.0.0-local-version", + "typescript": "0.0.0-local-version" + }, + "engines": { + "node": ">=18.0.0" + } + } + `, + "app/routes/_index.tsx": js` + import { json } from "@remix-run/node"; + import { useLoaderData, Link } from "@remix-run/react"; + + export function loader() { + return json("pizza"); + } + + export default function Index() { + let data = useLoaderData(); + return ( +
+ {data} + Other Route +
+ ) + } + `, + }, + }); + + let buildPath = path.resolve( + projectDir, + "build", + `index.${serverModuleExt}` + ); + expect(fs.existsSync(buildPath), "doesn't exist").toBe(true); + let contents = fs.readFileSync(buildPath, "utf8"); + expect(contents, "no export statement").toContain(exportStatement); + }); + } +}); diff --git a/integration/compiler-mjs-output-test.ts b/integration/compiler-mjs-output-test.ts deleted file mode 100644 index 5f0012a93f7..00000000000 --- a/integration/compiler-mjs-output-test.ts +++ /dev/null @@ -1,76 +0,0 @@ -import { test, expect } from "@playwright/test"; -import * as fs from "node:fs"; -import * as path from "node:path"; - -import { createFixtureProject, js } from "./helpers/create-fixture.js"; - -let projectDir: string; - -test.beforeAll(async () => { - projectDir = await createFixtureProject({ - files: { - // Ensure the config is valid ESM - "remix.config.js": js` - export default { - serverModuleFormat: "esm", - serverBuildPath: "build/index.mjs", - }; - `, - "package.json": js` - { - "name": "remix-template-remix", - "private": true, - "sideEffects": false, - "type": "module", - "scripts": { - "build": "node ../../../build/node_modules/@remix-run/dev/dist/cli.js build", - "dev": "node ../../../build/node_modules/@remix-run/dev/dist/cli.js dev", - "start": "node ../../../build/node_modules/@remix-run/serve/dist/cli.js build" - }, - "dependencies": { - "@remix-run/node": "0.0.0-local-version", - "@remix-run/react": "0.0.0-local-version", - "@remix-run/serve": "0.0.0-local-version", - "isbot": "0.0.0-local-version", - "react": "0.0.0-local-version", - "react-dom": "0.0.0-local-version" - }, - "devDependencies": { - "@remix-run/dev": "0.0.0-local-version", - "@types/react": "0.0.0-local-version", - "@types/react-dom": "0.0.0-local-version", - "typescript": "0.0.0-local-version" - }, - "engines": { - "node": ">=18.0.0" - } - } - `, - "app/routes/_index.tsx": js` - import { json } from "@remix-run/node"; - import { useLoaderData, Link } from "@remix-run/react"; - - export function loader() { - return json("pizza"); - } - - export default function Index() { - let data = useLoaderData(); - return ( -
- {data} - Other Route -
- ) - } - `, - }, - }); -}); - -test("can write .mjs server output module", () => { - let buildPath = path.resolve(projectDir, "build", "index.mjs"); - expect(fs.existsSync(buildPath), "doesn't exist").toBe(true); - let contents = fs.readFileSync(buildPath, "utf8"); - expect(contents, "no export statement").toContain("export {"); -}); diff --git a/packages/remix-dev/compiler/server/write.ts b/packages/remix-dev/compiler/server/write.ts index 948d1a0b5a4..d5300452d1b 100644 --- a/packages/remix-dev/compiler/server/write.ts +++ b/packages/remix-dev/compiler/server/write.ts @@ -11,7 +11,7 @@ export async function write( await fse.ensureDir(path.dirname(config.serverBuildPath)); for (let file of outputFiles) { - if (file.path.endsWith(".js") || file.path.endsWith(".mjs")) { + if ([".js", ".cjs", ".mjs"].some((ext) => file.path.endsWith(ext))) { // fix sourceMappingURL to be relative to current path instead of /build let filename = file.path.substring(file.path.lastIndexOf(path.sep) + 1); let escapedFilename = filename.replace(/\./g, "\\.");