Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(remix-dev): relativize route modules to make builds deterministic #2027

Merged
merged 1 commit into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/silent-parrots-matter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/dev": patch
---

Make build hashing deterministic
28 changes: 28 additions & 0 deletions integration/deterministic-build-output-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { test, expect } from "@playwright/test";
import globby from "globby";
import fs from "fs";
import path from "path";

import { createFixtureProject } from "./helpers/create-fixture";

test("builds deterministically under different paths", async () => {
let dir1 = await createFixtureProject();
let dir2 = await createFixtureProject();

expect(dir1).not.toEqual(dir2);

let files1 = await globby(["build/index.js", "public/build/**/*.js"], {
cwd: dir1,
});
let files2 = await globby(["build/index.js", "public/build/**/*.js"], {
cwd: dir2,
});

expect(files1.length).toBeGreaterThan(0);
expect(files1).toEqual(files2);
files1.forEach((file, i) => {
expect(fs.readFileSync(path.join(dir1, file))).toEqual(
fs.readFileSync(path.join(dir2, files2[i]))
);
});
});
10 changes: 6 additions & 4 deletions integration/helpers/create-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const TMP_DIR = path.join(process.cwd(), ".tmp", "integration");
interface FixtureInit {
buildStdio?: Writable;
sourcemap?: boolean;
files: { [filename: string]: string };
files?: { [filename: string]: string };
template?: "cf-template" | "deno-template" | "node-template";
setup?: "node" | "cloudflare";
}
Expand Down Expand Up @@ -133,7 +133,9 @@ export async function createAppFixture(fixture: Fixture) {
}

////////////////////////////////////////////////////////////////////////////////
export async function createFixtureProject(init: FixtureInit): Promise<string> {
export async function createFixtureProject(
init: FixtureInit = {}
): Promise<string> {
let template = init.template ?? "node-template";
let integrationTemplateDir = path.join(__dirname, template);
let projectName = `remix-${template}-${Math.random().toString(32).slice(2)}`;
Expand Down Expand Up @@ -202,10 +204,10 @@ function build(projectDir: string, buildStdio?: Writable, sourcemap?: boolean) {

async function writeTestFiles(init: FixtureInit, dir: string) {
await Promise.all(
Object.keys(init.files).map(async (filename) => {
Object.keys(init.files ?? {}).map(async (filename) => {
let filePath = path.join(dir, filename);
await fse.ensureDir(path.dirname(filePath));
await fse.writeFile(filePath, stripIndent(init.files[filename]));
await fse.writeFile(filePath, stripIndent(init.files![filename]));
})
);
}
3 changes: 1 addition & 2 deletions packages/remix-dev/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,7 @@ async function createBrowserBuild(
// 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] =
path.resolve(config.appDirectory, config.routes[id].file) + "?browser";
entryPoints[id] = config.routes[id].file + "?browser";
}

let plugins = [
Expand Down
18 changes: 9 additions & 9 deletions packages/remix-dev/compiler/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export async function createAssetsManifest(
let routesByFile: Map<string, Route> = Object.keys(config.routes).reduce(
(map, key) => {
let route = config.routes[key];
map.set(path.resolve(config.appDirectory, route.file), route);
map.set(route.file, route);
return map;
},
new Map()
Expand All @@ -72,19 +72,19 @@ export async function createAssetsManifest(
let output = metafile.outputs[key];
if (!output.entryPoint) continue;

let entryPointFile = path.resolve(
output.entryPoint.replace(
/(^browser-route-module:|^pnp:|\?browser$)/g,
""
)
);
if (entryPointFile === entryClientFile) {
// When using yarn-pnp, esbuild-plugin-pnp resolves files under the pnp namespace, even entry.client.tsx
let entryPointFile = output.entryPoint.replace(/^pnp:/, "");
if (path.resolve(entryPointFile) === entryClientFile) {
entry = {
module: resolveUrl(key),
imports: resolveImports(output.imports),
};
// Only parse routes otherwise dynamic imports can fall into here and fail the build
} else if (output.entryPoint.startsWith("browser-route-module:")) {
} else if (entryPointFile.startsWith("browser-route-module:")) {
entryPointFile = entryPointFile.replace(
/(^browser-route-module:|\?browser$)/g,
""
);
let route = routesByFile.get(entryPointFile);
invariant(route, `Cannot get route for entry point ${output.entryPoint}`);
let sourceExports = await getRouteModuleExportsCached(config, route.id);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as path from "path";
import type esbuild from "esbuild";

import type { RemixConfig } from "../../config";
Expand Down Expand Up @@ -31,7 +30,7 @@ export function browserRouteModulesPlugin(
let routesByFile: Map<string, Route> = Object.keys(config.routes).reduce(
(map, key) => {
let route = config.routes[key];
map.set(path.resolve(config.appDirectory, route.file), route);
map.set(route.file, route);
return map;
},
new Map()
Expand Down Expand Up @@ -71,12 +70,12 @@ export function browserRouteModulesPlugin(
let contents = "module.exports = {};";
if (theExports.length !== 0) {
let spec = `{ ${theExports.join(", ")} }`;
contents = `export ${spec} from ${JSON.stringify(file)};`;
contents = `export ${spec} from ${JSON.stringify(`./${file}`)};`;
}

return {
contents,
resolveDir: path.dirname(file),
resolveDir: config.appDirectory,
loader: "js",
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as path from "path";
import type { Plugin } from "esbuild";

import type { RemixConfig } from "../../config";
Expand Down Expand Up @@ -31,14 +30,12 @@ export function serverEntryModulePlugin(config: RemixConfig): Plugin {
resolveDir: config.appDirectory,
loader: "js",
contents: `
import * as entryServer from ${JSON.stringify(
path.resolve(config.appDirectory, config.entryServerFile)
)};
import * as entryServer from ${JSON.stringify(`./${config.entryServerFile}`)};
${Object.keys(config.routes)
.map((key, index) => {
let route = config.routes[key];
return `import * as route${index} from ${JSON.stringify(
path.resolve(config.appDirectory, route.file)
`./${route.file}`
)};`;
})
.join("\n")}
Expand Down