-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(remix-dev): relativize route modules to make builds deterministic
Fixes #2024 If virtual modules have non-deterministic paths or content (e.g. due to importing from other absolute paths), the input is technically different, and deterministic build output is not guaranteed. Depending on how you build/deploy (e.g. if you need to build and deploy your server separately from your browser build), this can result in a broken app, since the server and browser manifests may differ (i.e. due to different fingerprints). By using relative paths for route modules, we can ensure the same result no matter the absolute path. Possibly worth pointing out that this fix also affects file path comments in the server build, e.g. you'll now see stuff like: // app/root.tsx instead of: // /absolute/path/on/the/build/machine/to/app/root.tsx Testing notes: 1. Added integration test 2. Verified manually, i.e. 1. Create two remix projects (via npx create-remix@latest) 2. `npm run build` them both 3. `diff -r project1/build project2/build` has no differences 4. `diff -r project1/public/build project2/public/build` has no differences 5. `dev` and `start` still work as per usual
- Loading branch information
Showing
7 changed files
with
101 additions
and
24 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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])) | ||
| ); | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import type { | ||
| Plugin, | ||
| OnResolveOptions, | ||
| OnResolveArgs, | ||
| OnResolveResult, | ||
| PluginBuild, | ||
| } from "esbuild"; | ||
| import { pnpPlugin } from "@yarnpkg/esbuild-plugin-pnp"; | ||
|
|
||
| // esbuild-plugin-pnp doesn't correctly handle imports that happen within virtual modules | ||
| // with relative paths; it assumes that `importer` refers to a real path, and incorrectly | ||
| // resolves modules based on it. It should respect the resolveDir if present. | ||
| // See here: https://esbuild.github.io/plugins/#on-resolve-arguments | ||
| // | ||
| // Ideally we could just use an esbuild filter regex so that it only resolves module specifiers | ||
| // and their dependencies and *not* project relative imports, but we can't because there's | ||
| // no way to distinguish those from dependencies' relative imports which we *do* want yarn-pnp | ||
| // to handle. So anything not resolved by earlier plugins has to be resolved by yarn-pnp. | ||
| // | ||
| // This will be fixed by https://github.com/yarnpkg/berry/pull/4569 | ||
| // but in the meantime we need this hack 🙃 | ||
| export const yarnPnpPlugin = (): Plugin => { | ||
| return { | ||
| name: "yarn-pnp", | ||
| setup(build) { | ||
| return pnpPlugin().setup( | ||
| new Proxy(build, { | ||
| get(target, property) { | ||
| if (property === "onResolve") { | ||
| return ( | ||
| options: OnResolveOptions, | ||
| pluginOnResolveCallback: ( | ||
| args: OnResolveArgs | ||
| ) => OnResolveResult | ||
| ) => { | ||
| build.onResolve(options, (args: any) => { | ||
| // sneakily fix the importer before pnp onResolve callback runs | ||
| if (args.resolveDir) { | ||
| args.importer = args.resolveDir + "/"; | ||
| } | ||
| return pluginOnResolveCallback(args); | ||
| }); | ||
| }; | ||
| } else { | ||
| return target[property as keyof PluginBuild]; | ||
| } | ||
| }, | ||
| }) | ||
| ); | ||
| }, | ||
| }; | ||
| }; |