feat(dev): make browser Node polyfills opt-in (#7269)

.changeset/global-polyfills.md

@@ -0,0 +1,22 @@
+---
+"@remix-run/dev": minor
+---
+
+The `serverNodeBuiltinsPolyfill` option (along with the newly added `browserNodeBuiltinsPolyfill`) now supports defining global polyfills in addition to module polyfills.
+
+For example, to polyfill Node's `Buffer` global:
+
+```js
+module.exports = {
+  serverNodeBuiltinsPolyfill: {
+    globals: {
+      Buffer: true,
+    },
+    // You'll probably need to polyfill the "buffer" module
+    // too since the global polyfill imports this:
+    modules: {
+      buffer: true,
+    },
+  },
+};
+```

.changeset/remove-default-node-polyfills.md

@@ -0,0 +1,33 @@
+---
+"@remix-run/dev": major
+---
+
+Remove default Node.js polyfills.
+
+Any Node.js polyfills (or empty polyfills) that are required for your browser code must be configured via the `browserNodeBuiltinsPolyfill` option in `remix.config.js`.
+
+```js
+exports.browserNodeBuiltinsPolyfill = {
+  modules: {
+    buffer: true,
+    fs: "empty",
+  },
+  globals: {
+    Buffer: true,
+  },
+};
+```
+
+If you're targeting a non-Node.js server platform, any Node.js polyfills (or empty polyfills) that are required for your server code must be configured via the `serverNodeBuiltinsPolyfill` option in `remix.config.js`.
+
+```js
+exports.serverNodeBuiltinsPolyfill = {
+  modules: {
+    buffer: true,
+    fs: "empty",
+  },
+  globals: {
+    Buffer: true,
+  },
+};
+```

.changeset/restart-dev-server-on-remix-config-change.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/dev": patch
+---
+
+Restart dev server when Remix config changes

docs/file-conventions/remix-config.md

@@ -40,6 +40,24 @@ exports.appDirectory = "./elsewhere";
 The path to the browser build, relative to remix.config.js. Defaults to
 "public/build". Should be deployed to static hosting.
 
+## browserNodeBuiltinsPolyfill
+
+The Node.js polyfills to include in the browser build. Polyfills are provided by [JSPM][jspm] and configured via [esbuild-plugins-node-modules-polyfill].
+
+```js filename=remix.config.js
+exports.browserNodeBuiltinsPolyfill = {
+  modules: {
+    buffer: true, // Provide a JSPM polyfill
+    fs: "empty", // Provide an empty polyfill
+  },
+  globals: {
+    Buffer: true,
+  },
+};
+```
+
+When using this option and targeting non-Node.js server platforms, you may also want to configure Node.js polyfills for the server via [`serverNodeBuiltinsPolyfill`][server-node-builtins-polyfill].
+
 ## cacheDirectory
 
 The path to a directory Remix can use for caching things in development,
@@ -168,12 +186,17 @@ The Node.js polyfills to include in the server build when targeting non-Node.js
 ```js filename=remix.config.js
 exports.serverNodeBuiltinsPolyfill = {
   modules: {
-    path: true, // Provide a JSPM polyfill
+    buffer: true, // Provide a JSPM polyfill
     fs: "empty", // Provide an empty polyfill
   },
+  globals: {
+    Buffer: true,
+  },
 };
 ```
 
+When using this option, you may also want to configure Node.js polyfills for the browser via [`browserNodeBuiltinsPolyfill`][browser-node-builtins-polyfill].
+
 ## serverPlatform
 
 The platform the server build is targeting, which can either be `"neutral"` or
@@ -232,3 +255,5 @@ There are a few conventions that Remix uses you should be aware of.
 [jspm]: https://github.com/jspm/jspm-core
 [esbuild-plugins-node-modules-polyfill]: https://www.npmjs.com/package/esbuild-plugins-node-modules-polyfill
 [port]: ../other-api/dev-v2#options-1
+[browser-node-builtins-polyfill]: #browsernodebuiltinspolyfill
+[server-node-builtins-polyfill]: #servernodebuiltinspolyfill

docs/guides/v2.md

@@ -719,11 +719,41 @@ The default server module output format will be changing from `cjs` to `esm`.
 
 In your `remix.config.js`, you should specify either `serverModuleFormat: "cjs"` to retain existing behavior, or `serverModuleFormat: "esm"`, to opt into the future behavior.
 
+## `browserNodeBuiltinsPolyfill`
+
+Polyfills for Node.js built-in modules will no longer be provided by default for the browser. In Remix v2 you'll need to explicitly reintroduce any polyfills (or blank polyfills) as required:
+
+```js filename=remix.config.js
+module.exports = {
+  browserNodeBuiltinsPolyfill: {
+    modules: {
+      buffer: true,
+      fs: "empty",
+    },
+    globals: {
+      Buffer: true,
+    },
+  },
+};
+```
+
+Even though we recommend being explicit about which polyfills are allowed in your browser bundle, especially since some polyfills can be quite large, you can quickly reinstate the full set of polyfills from Remix v1 with the following configuration:
+
+```js filename=remix.config.js
+const { builtinModules } = require("node:module");
+
+module.exports = {
+  browserNodeBuiltinsPolyfill: {
+    modules: builtinModules,
+  },
+};
+```
+
 ## `serverNodeBuiltinsPolyfill`
 
 Polyfills for Node.js built-in modules will no longer be provided by default for non-Node.js server platforms.
 
-If you are targeting a non-Node.js server platform and want to opt into the future default behavior, in `remix.config.js` you should first remove all server polyfills by providing an empty object for `serverNodeBuiltinsPolyfill.modules`:
+If you are targeting a non-Node.js server platform and want to opt into the future default behavior in v1, in `remix.config.js` you should first remove all server polyfills by explicitly providing an empty object for `serverNodeBuiltinsPolyfill.modules`:
 
 ```js filename=remix.config.js
 /** @type {import('@remix-run/dev').AppConfig} */
@@ -741,9 +771,12 @@ You can then reintroduce any polyfills (or blank polyfills) as required.
 module.exports = {
   serverNodeBuiltinsPolyfill: {
     modules: {
-      path: true,
+      buffer: true,
       fs: "empty",
     },
+    globals: {
+      Buffer: true,
+    },
   },
 };
 ```

integration/compiler-test.ts

@@ -29,6 +29,11 @@ test.describe("compiler", () => {
               "esm-only-single-export",
               ...getDependenciesToBundle("esm-only-exports-pkg"),
             ],
+            browserNodeBuiltinsPolyfill: {
+              modules: {
+                path: true,
+              },
+            },
           };
         `,
         "app/fake.server.ts": js`

integration/upload-test.ts

@@ -17,6 +17,13 @@ const __dirname = url.fileURLToPath(new URL(".", import.meta.url));
 
 test.beforeAll(async () => {
   fixture = await createFixture({
+    config: {
+      browserNodeBuiltinsPolyfill: {
+        modules: {
+          url: true,
+        },
+      },
+    },
     files: {
       "app/routes/file-upload-handler.tsx": js`
         import * as path from "node:path";

packages/remix-dev/__tests__/readConfig-test.ts

@@ -28,6 +28,7 @@ describe("readConfig", () => {
       Object {
         "appDirectory": Any<String>,
         "assetsBuildDirectory": Any<String>,
+        "browserNodeBuiltinsPolyfill": undefined,
         "cacheDirectory": Any<String>,
         "dev": Object {},
         "entryClientFile": "entry.client.tsx",

packages/remix-dev/compiler/compiler.ts

@@ -62,6 +62,7 @@ export let create = async (ctx: Context): Promise<Compiler> => {
 
     // keep track of manually written artifacts
     let writes: {
+      js?: Promise<void>;
       cssBundle?: Promise<void>;
       manifest?: Promise<void>;
       server?: Promise<void>;
@@ -100,7 +101,8 @@ export let create = async (ctx: Context): Promise<Compiler> => {
     // js compilation (implicitly writes artifacts/js)
     let js = await tasks.js;
     if (!js.ok) throw error ?? js.error;
-    let { metafile, hmr } = js.value;
+    let { metafile, outputFiles, hmr } = js.value;
+    writes.js = JS.write(ctx.config, outputFiles);
 
     // artifacts/manifest
     let manifest = await createManifest({

packages/remix-dev/compiler/js/compiler.ts

@@ -1,7 +1,6 @@
 import * as path from "node:path";
 import { builtinModules as nodeBuiltins } from "node:module";
 import * as esbuild from "esbuild";
-import { nodeModulesPolyfillPlugin } from "esbuild-plugins-node-modules-polyfill";
 
 import type { RemixConfig } from "../../config";
 import { type Manifest } from "../../manifest";
@@ -13,6 +12,7 @@ import { absoluteCssUrlsPlugin } from "../plugins/absoluteCssUrlsPlugin";
 import { emptyModulesPlugin } from "../plugins/emptyModules";
 import { mdxPlugin } from "../plugins/mdx";
 import { externalPlugin } from "../plugins/external";
+import { browserNodeBuiltinsPolyfillPlugin } from "./plugins/browserNodeBuiltinsPolyfill";
 import { cssBundlePlugin } from "../plugins/cssBundlePlugin";
 import { cssModulesPlugin } from "../plugins/cssModuleImports";
 import { cssSideEffectImportsPlugin } from "../plugins/cssSideEffectImports";
@@ -27,6 +27,7 @@ type Compiler = {
   // produce ./public/build/
   compile: () => Promise<{
     metafile: esbuild.Metafile;
+    outputFiles: esbuild.OutputFile[];
     hmr?: Manifest["hmr"];
   }>;
   cancel: () => Promise<void>;
@@ -94,15 +95,7 @@ const createEsbuildConfig = (
     emptyModulesPlugin(ctx, /^@remix-run\/(deno|cloudflare|node)(\/.*)?$/, {
       includeNodeModules: true,
     }),
-    nodeModulesPolyfillPlugin({
-      // For the browser build, we replace any Node built-ins that don't have a
-      // polyfill with an empty module. This ensures the build can pass without
-      // having to mark all Node built-ins as external which can result in other
-      // issues, e.g. https://github.com/remix-run/remix/issues/5521. We then
-      // rely on tree-shaking to remove all unused polyfills and fallbacks.
-      fallback: "empty",
-    }),
-    externalPlugin(/^node:.*/, { sideEffects: false }),
+    browserNodeBuiltinsPolyfillPlugin(ctx),
   ];
 
   if (ctx.options.mode === "development") {
@@ -156,11 +149,12 @@ export const create = async (
 ): Promise<Compiler> => {
   let compiler = await esbuild.context({
     ...createEsbuildConfig(ctx, refs),
+    write: false,
     metafile: true,
   });
 
   let compile = async () => {
-    let { metafile } = await compiler.rebuild();
+    let { metafile, outputFiles } = await compiler.rebuild();
     writeMetafile(ctx, "metafile.js.json", metafile);
 
     let hmr: Manifest["hmr"] | undefined = undefined;
@@ -181,7 +175,7 @@ export const create = async (
       };
     }
 
-    return { metafile, hmr };
+    return { metafile, hmr, outputFiles };
   };
 
   return {

packages/remix-dev/compiler/js/index.ts

@@ -1 +1,2 @@
 export { create as createCompiler } from "./compiler";
+export { write } from "./write";

packages/remix-dev/compiler/js/plugins/browserNodeBuiltinsPolyfill.ts

@@ -0,0 +1,37 @@
+import { nodeModulesPolyfillPlugin } from "esbuild-plugins-node-modules-polyfill";
+
+import type { Context } from "../../context";
+
+export const browserNodeBuiltinsPolyfillPlugin = (ctx: Context) =>
+  nodeModulesPolyfillPlugin({
+    // Rename plugin to improve error message attribution
+    name: "browser-node-builtins-polyfill-plugin",
+    // Only pass through the "modules" and "globals" options to ensure we
+    // don't leak the full plugin API to Remix consumers.
+    modules: ctx.config.browserNodeBuiltinsPolyfill?.modules ?? {},
+    globals: ctx.config.browserNodeBuiltinsPolyfill?.globals ?? {},
+    // Mark any unpolyfilled Node builtins in the build output as errors.
+    fallback: "error",
+    formatError({ moduleName, importer, polyfillExists }) {
+      let normalizedModuleName = moduleName.replace("node:", "");
+      let modulesConfigKey = /^[a-z_]+$/.test(normalizedModuleName)
+        ? normalizedModuleName
+        : JSON.stringify(normalizedModuleName);
+
+      return {
+        text: (polyfillExists
+          ? [
+              `Node builtin "${moduleName}" (imported by "${importer}") must be polyfilled for the browser. `,
+              `You can enable this polyfill in your Remix config, `,
+              `e.g. \`browserNodeBuiltinsPolyfill: { modules: { ${modulesConfigKey}: true } }\``,
+            ]
+          : [
+              `Node builtin "${moduleName}" (imported by "${importer}") doesn't have a browser polyfill available. `,
+              `You can stub it out with an empty object in your Remix config `,
+              `e.g. \`browserNodeBuiltinsPolyfill: { modules: { ${modulesConfigKey}: "empty" } }\` `,
+              "but note that this may cause runtime errors if the module is used in your browser code.",
+            ]
+        ).join(""),
+      };
+    },
+  });

packages/remix-dev/compiler/js/write.ts

@@ -0,0 +1,14 @@
+import * as path from "node:path";
+import type { OutputFile } from "esbuild";
+import fse from "fs-extra";
+
+import type { RemixConfig } from "../../config";
+
+export async function write(config: RemixConfig, outputFiles: OutputFile[]) {
+  await fse.ensureDir(path.dirname(config.assetsBuildDirectory));
+
+  for (let file of outputFiles) {
+    await fse.ensureDir(path.dirname(file.path));
+    await fse.writeFile(file.path, file.contents);
+  }
+}

packages/remix-dev/compiler/server/compiler.ts

@@ -1,5 +1,4 @@
 import * as esbuild from "esbuild";
-import { nodeModulesPolyfillPlugin } from "esbuild-plugins-node-modules-polyfill";
 
 import { type Manifest } from "../../manifest";
 import { loaders } from "../utils/loaders";
@@ -9,6 +8,7 @@ import { vanillaExtractPlugin } from "../plugins/vanillaExtract";
 import { cssFilePlugin } from "../plugins/cssImports";
 import { absoluteCssUrlsPlugin } from "../plugins/absoluteCssUrlsPlugin";
 import { emptyModulesPlugin } from "../plugins/emptyModules";
+import { serverNodeBuiltinsPolyfillPlugin } from "./plugins/serverNodeBuiltinsPolyfill";
 import { mdxPlugin } from "../plugins/mdx";
 import { serverAssetsManifestPlugin } from "./plugins/manifest";
 import { serverBareModulesPlugin } from "./plugins/bareImports";
@@ -66,12 +66,7 @@ const createEsbuildConfig = (
   ];
 
   if (ctx.config.serverNodeBuiltinsPolyfill) {
-    plugins.unshift(
-      nodeModulesPolyfillPlugin({
-        // Ensure only "modules" option is passed to the plugin
-        modules: ctx.config.serverNodeBuiltinsPolyfill.modules,
-      })
-    );
+    plugins.unshift(serverNodeBuiltinsPolyfillPlugin(ctx));
   }
 
   return {