Skip to content

Commit

Permalink
fix(remix-dev): make MDX builds deterministic
Browse files Browse the repository at this point in the history
This is a fix along the lines of #2027; because the MDX virtual modules
resolve to absolute paths, the manifest/hashes/etc. differ based on the
parent directory structure.

Testing notes:
1. Expanded existing integration test to cover this scenario (fails without
   the fix). Also add other virtual module scenarios that weren't already
   covered
2. Validated in a real MDX Remix app that has split client / server builds
   (applied fix via patch-package)
  • Loading branch information
jenseng committed Aug 8, 2022
1 parent 845c6c3 commit f9845a7
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 6 deletions.
30 changes: 27 additions & 3 deletions integration/deterministic-build-output-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,35 @@ import globby from "globby";
import fs from "fs";
import path from "path";

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

test("builds deterministically under different paths", async () => {
let dir1 = await createFixtureProject();
let dir2 = await createFixtureProject();
// This test validates various flavors of remix virtual modules to ensure
// we get identical builds regardless of the parent paths. If a virtual
// module resolves or imports from absolute paths (e.g. via `path.resolve`),
// the build hashes may change even though the output is identical. This
// can cause broken apps (i.e. manifest mismatch) if the server and client
// are built separately.

// Virtual modules tested:
// * browserRouteModulesPlugin (implicitly tested by root route)
// * emptyModulesPlugin (via app/routes/foo.tsx' server import)
// * mdx (via app/routes/index.mdx)
// * serverAssetsManifestPlugin (implicitly tested by build)
// * serverEntryModulePlugin (implicitly tested by build)
// * serverRouteModulesPlugin (implicitly tested by build)
let init = {
files: {
"app/routes/index.mdx": "# hello world",
"app/routes/foo.tsx": js`
export * from "~/foo/bar.server";
export default () => "YAY";
`,
"app/foo/bar.server.ts": "export const meta = () => []",
},
};
let dir1 = await createFixtureProject(init);
let dir2 = await createFixtureProject(init);

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

Expand Down
7 changes: 4 additions & 3 deletions packages/remix-dev/compiler/plugins/mdx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,15 @@ export function mdxPlugin(config: RemixConfig): esbuild.Plugin {
let resolved = path.resolve(args.resolveDir, resolvedPath);

return {
path: resolved,
path: path.relative(config.appDirectory, resolved),
namespace: "mdx",
};
});

build.onLoad({ filter: /\.mdx?$/ }, async (args) => {
let absolutePath = path.join(config.appDirectory, args.path);
try {
let fileContents = await fsp.readFile(args.path, "utf-8");
let fileContents = await fsp.readFile(absolutePath, "utf-8");

let rehypePlugins = [];
let remarkPlugins = [
Expand Down Expand Up @@ -115,7 +116,7 @@ ${remixExports}`;
errors: errors.length ? errors : undefined,
warnings: warnings.length ? warnings : undefined,
contents,
resolveDir: path.dirname(args.path),
resolveDir: path.dirname(absolutePath),
loader: getLoaderForFile(args.path),
};
} catch (err: any) {
Expand Down

0 comments on commit f9845a7

Please sign in to comment.