Skip to content

Commit

Permalink
fix(remix-dev): fix server builds where serverBuildPath ends in .cjs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lpsinger committed Aug 16, 2023
1 parent be5fa1f commit 5b9eb29
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 77 deletions.
7 changes: 7 additions & 0 deletions .changeset/wild-steaks-bathe.md
Original file line number Diff line number Diff line change
@@ -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`.
83 changes: 83 additions & 0 deletions integration/compiler-mjs-cjs-output-test.ts
Original file line number Diff line number Diff line change
@@ -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 (
<div>
{data}
<Link to="/burgers">Other Route</Link>
</div>
)
}
`,
},
});

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);
});
}
});
76 changes: 0 additions & 76 deletions integration/compiler-mjs-output-test.ts

This file was deleted.

2 changes: 1 addition & 1 deletion packages/remix-dev/compiler/server/write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, "\\.");
Expand Down

0 comments on commit 5b9eb29

Please sign in to comment.