diff --git a/integration/deterministic-build-output-test.ts b/integration/deterministic-build-output-test.ts new file mode 100644 index 00000000000..f713717491d --- /dev/null +++ b/integration/deterministic-build-output-test.ts @@ -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])) + ); + }); +}); diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts index 94af6ab020c..911fb62ff9e 100644 --- a/integration/helpers/create-fixture.ts +++ b/integration/helpers/create-fixture.ts @@ -17,7 +17,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"; } @@ -143,7 +143,9 @@ export async function createAppFixture(fixture: Fixture) { } //////////////////////////////////////////////////////////////////////////////// -export async function createFixtureProject(init: FixtureInit): Promise { +export async function createFixtureProject( + init: FixtureInit = {} +): Promise { let template = init.template ?? "node-template"; let integrationTemplateDir = path.join(__dirname, template); let projectName = `remix-${template}-${Math.random().toString(32).slice(2)}`; @@ -186,7 +188,7 @@ 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])); diff --git a/packages/remix-dev/compiler.ts b/packages/remix-dev/compiler.ts index 6ddc2fa68e8..72968c36a76 100644 --- a/packages/remix-dev/compiler.ts +++ b/packages/remix-dev/compiler.ts @@ -5,7 +5,6 @@ import * as fse from "fs-extra"; import debounce from "lodash.debounce"; import chokidar from "chokidar"; import { NodeModulesPolyfillPlugin } from "@esbuild-plugins/node-modules-polyfill"; -import { pnpPlugin as yarnPnpPlugin } from "@yarnpkg/esbuild-plugin-pnp"; import { BuildMode, BuildTarget } from "./build"; import type { RemixConfig } from "./config"; @@ -25,6 +24,7 @@ import { serverEntryModulePlugin } from "./compiler/plugins/serverEntryModulePlu import { serverRouteModulesPlugin } from "./compiler/plugins/serverRouteModulesPlugin"; import { writeFileSafe } from "./compiler/utils/fs"; import { urlImportsPlugin } from "./compiler/plugins/urlImportsPlugin"; +import { yarnPnpPlugin } from "./compiler/plugins/yarnPnpPlugin"; // When we build Remix, this shim file is copied directly into the output // directory in the same place relative to this file. It is eventually injected @@ -344,8 +344,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 = [ diff --git a/packages/remix-dev/compiler/assets.ts b/packages/remix-dev/compiler/assets.ts index 44185bcbc61..678a2f93f7d 100644 --- a/packages/remix-dev/compiler/assets.ts +++ b/packages/remix-dev/compiler/assets.ts @@ -59,7 +59,7 @@ export async function createAssetsManifest( let routesByFile: Map = 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() @@ -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); diff --git a/packages/remix-dev/compiler/plugins/browserRouteModulesPlugin.ts b/packages/remix-dev/compiler/plugins/browserRouteModulesPlugin.ts index fb2ed3ff695..6e66da4aef1 100644 --- a/packages/remix-dev/compiler/plugins/browserRouteModulesPlugin.ts +++ b/packages/remix-dev/compiler/plugins/browserRouteModulesPlugin.ts @@ -1,4 +1,3 @@ -import * as path from "path"; import type esbuild from "esbuild"; import type { RemixConfig } from "../../config"; @@ -31,7 +30,7 @@ export function browserRouteModulesPlugin( let routesByFile: Map = 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() @@ -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", }; } diff --git a/packages/remix-dev/compiler/plugins/serverEntryModulePlugin.ts b/packages/remix-dev/compiler/plugins/serverEntryModulePlugin.ts index 79e20f4f419..662cfe769df 100644 --- a/packages/remix-dev/compiler/plugins/serverEntryModulePlugin.ts +++ b/packages/remix-dev/compiler/plugins/serverEntryModulePlugin.ts @@ -1,4 +1,3 @@ -import * as path from "path"; import type { Plugin } from "esbuild"; import type { RemixConfig } from "../../config"; @@ -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")} diff --git a/packages/remix-dev/compiler/plugins/yarnPnpPlugin.ts b/packages/remix-dev/compiler/plugins/yarnPnpPlugin.ts new file mode 100644 index 00000000000..37b46d1d2ed --- /dev/null +++ b/packages/remix-dev/compiler/plugins/yarnPnpPlugin.ts @@ -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]; + } + }, + }) + ); + }, + }; +};