feat(dev): add serverNodeBuiltinsPolyfill option (#6814)

Co-authored-by: Luke Askew <720056+LukeAskew@users.noreply.github.com> Co-authored-by: Ivo Todorov <ivo.gtodorov@gmail.com> Co-authored-by: Jacob Ebey <jacob.ebey@live.com>
remix-run · Jul 13, 2023 · cdb1d32 · cdb1d32
1 parent 68877d1
commit cdb1d32
Show file tree

Hide file tree

Showing 9 changed files with 108 additions and 7 deletions.
diff --git a/.changeset/exclude-unimplemented-server-polyfills.md b/.changeset/exclude-unimplemented-server-polyfills.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/dev": patch
+---
+
+Exclude unimplemented polyfills from server build for non-Node.js server platforms
diff --git a/.changeset/server-node-builtins-polyfill.md b/.changeset/server-node-builtins-polyfill.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/dev": minor
+---
+
+Add `serverNodeBuiltinsPolyfill` config option. You can now disable polyfills of Node.js built-in modules for non-Node.js server platforms by setting `serverNodeBuiltinsPolyfill: false` in `remix.config.js`. You can also provide a list of server polyfills, e.g. `serverNodeBuiltinsPolyfill: ["crypto"]`.
diff --git a/docs/file-conventions/remix-config.md b/docs/file-conventions/remix-config.md
@@ -191,6 +191,18 @@ Whether to minify the server build in production or not. Defaults to `false`.
 The output format of the server build, which can either be `"cjs"` or `"esm"`.
 Defaults to `"cjs"`.
 
+## serverNodeBuiltinsPolyfill
+
+Whether to polyfill Node.js built-in modules in the server build, or a list of polyfills. Defaults to `true` for non-Node.js server platforms.
+
+```js filename=remix.config.js
+// disable all polyfills
+exports.serverNodeBuiltinsPolyfill = false;
+
+// enable specific polyfills
+exports.serverNodeBuiltinsPolyfill = ["crypto"];
+```
+
 ## serverPlatform
 
 The platform the server build is targeting, which can either be `"neutral"` or

diff --git a/docs/pages/v2.md b/docs/pages/v2.md
@@ -703,6 +703,12 @@ 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.
 
+## `serverNodeBuiltinsPolyfill`
+
+We will no longer polyfill Node.js built-in modules by default for non-Node.js server platforms.
+
+If you are targeting a non-Node.js server platform, in your `remix.config.js` you should specify either `serverNodeBuiltinsPolyfill: true` to retain existing behavior, or `serverNodeBuiltinsPolyfill: false` to opt into the future default behavior. Alternatively, you can provide a list of built-in modules, e.g. `serverNodeBuiltinsPolyfill: ["crypto"]`.
+
 ## Dev Server
 
 For configuration options, see the [`remix dev` docs][v2-dev-config].

diff --git a/packages/remix-dev/__tests__/readConfig-test.ts b/packages/remix-dev/__tests__/readConfig-test.ts
@@ -80,6 +80,7 @@ describe("readConfig", () => {
         "serverMinify": false,
         "serverMode": "production",
         "serverModuleFormat": "cjs",
+        "serverNodeBuiltinsPolyfill": false,
         "serverPlatform": "node",
         "tailwind": false,
         "tsconfigPath": Any<String>,

diff --git a/packages/remix-dev/compiler/server/compiler.ts b/packages/remix-dev/compiler/server/compiler.ts
@@ -1,3 +1,4 @@
+import { builtinModules } from "module";
 import * as esbuild from "esbuild";
 import { nodeModulesPolyfillPlugin } from "esbuild-plugins-node-modules-polyfill";
 
@@ -67,8 +68,38 @@ const createEsbuildConfig = (
     externalPlugin(/^node:.*/, { sideEffects: false }),
   ];
 
-  if (ctx.config.serverPlatform !== "node") {
-    plugins.unshift(nodeModulesPolyfillPlugin());
+  if (ctx.config.serverNodeBuiltinsPolyfill) {
+    // These unimplemented polyfills throw an error at runtime if they're used.
+    // It's also possible that they'll be provided by the host environment (e.g.
+    // Cloudflare provides an "async_hooks" polyfill) so it's better to avoid
+    // them by default when server polyfills are enabled. If consumers want an
+    // unimplemented polyfill for some reason, they can explicitly pass a list
+    // of desired polyfills instead. This list was manually populated by looking
+    // for unimplemented browser polyfills in the jspm-core repo:
+    // https://github.com/jspm/jspm-core/tree/main/nodelibs/browser
+    let unimplemented = [
+      "async_hooks", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/async_hooks.js
+      "child_process", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/child_process.js
+      "cluster", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/cluster.js
+      "dgram", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/dgram.js
+      "dns", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/dns.js
+      "dns/promises", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/dns/promises.js
+      "http2", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/http2.js
+      "net", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/net.js
+      "readline", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/readline.js
+      "repl", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/repl.js
+      "tls", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/tls.js
+      "v8", // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/v8.js
+    ];
+
+    plugins.unshift(
+      nodeModulesPolyfillPlugin({
+        modules:
+          ctx.config.serverNodeBuiltinsPolyfill === true
+            ? builtinModules.filter((mod) => !unimplemented.includes(mod))
+            : ctx.config.serverNodeBuiltinsPolyfill,
+      })
+    );
   }
 
   return {

diff --git a/packages/remix-dev/config.ts b/packages/remix-dev/config.ts
@@ -192,6 +192,12 @@ export interface AppConfig {
    */
   serverModuleFormat?: ServerModuleFormat;
 
+  /**
+   * Whether to polyfill Node.js built-in modules in the server build, or a
+   * list of polyfills. Defaults to `true` for non-Node.js server platforms.
+   */
+  serverNodeBuiltinsPolyfill?: boolean | string[];
+
   /**
    * The platform the server build is targeting. Defaults to "node".
    */
@@ -363,6 +369,12 @@ export interface RemixConfig {
    */
   serverModuleFormat: ServerModuleFormat;
 
+  /**
+   * Whether to polyfill Node.js built-in modules in the server build, or a
+   * list of polyfills. Defaults to `true` for non-Node.js server platforms.
+   */
+  serverNodeBuiltinsPolyfill: boolean | string[];
+
   /**
    * The platform the server build is targeting. Defaults to "node".
    */
@@ -489,6 +501,20 @@ export async function readConfig(
     serverModuleFormat === "esm" ? ["module", "main"] : ["main", "module"];
   serverMinify ??= false;
 
+  let serverNodeBuiltinsPolyfill: RemixConfig["serverNodeBuiltinsPolyfill"] =
+    serverPlatform !== "node";
+
+  if (appConfig.serverNodeBuiltinsPolyfill !== undefined) {
+    serverNodeBuiltinsPolyfill = appConfig.serverNodeBuiltinsPolyfill;
+  }
+
+  if (
+    serverPlatform !== "node" &&
+    appConfig.serverNodeBuiltinsPolyfill === undefined
+  ) {
+    serverNodeBuiltinsPolyfillWarning();
+  }
+
   if (appConfig.future) {
     if ("unstable_cssModules" in appConfig.future) {
       logger.warn(
@@ -820,6 +846,7 @@ export async function readConfig(
     serverMinify,
     serverMode,
     serverModuleFormat,
+    serverNodeBuiltinsPolyfill,
     serverPlatform,
     mdx,
     postcss,
@@ -977,6 +1004,20 @@ let serverModuleFormatWarning = () =>
     key: "serverModuleFormatWarning",
   });
 
+let serverNodeBuiltinsPolyfillWarning = () =>
+  logger.warn(
+    "The `serverNodeBuiltinsPolyfill` config default option will be changing in v2",
+    {
+      details: [
+        "The default value will change from `true` to `false` regardless of platform.",
+        "You can prepare for this change by explicitly specifying `serverNodeBuiltinsPolyfill: false` or",
+        "`serverNodeBuiltinsPolyfill: true` if you are currently relying on them.",
+        "-> https://remix.run/docs/en/v1.19.0/pages/v2#servernodebuiltinspolyfill",
+      ],
+      key: "serverNodeBuiltinsPolyfillWarning",
+    }
+  );
+
 let futureFlagWarning =
   (args: { message: string; flag: string; link: string }) => () => {
     logger.warn(args.message, {

diff --git a/packages/remix-dev/package.json b/packages/remix-dev/package.json
@@ -36,7 +36,7 @@
     "chokidar": "^3.5.1",
     "dotenv": "^16.0.0",
     "esbuild": "0.17.6",
-    "esbuild-plugins-node-modules-polyfill": "^1.1.0",
+    "esbuild-plugins-node-modules-polyfill": "^1.2.0",
     "execa": "5.1.1",
     "exit-hook": "2.2.1",
     "express": "^4.17.1",

diff --git a/yarn.lock b/yarn.lock
@@ -6132,10 +6132,10 @@ esbuild-openbsd-64@0.14.47:
   resolved "https://registry.npmjs.org/esbuild-openbsd-64/-/esbuild-openbsd-64-0.14.47.tgz#309af806db561aa886c445344d1aacab850dbdc5"
   integrity sha512-QpgN8ofL7B9z8g5zZqJE+eFvD1LehRlxr25PBkjyyasakm4599iroUpaj96rdqRlO2ShuyqwJdr+oNqWwTUmQw==
 
-esbuild-plugins-node-modules-polyfill@^1.1.0:
-  version "1.1.0"
-  resolved "https://registry.npmjs.org/esbuild-plugins-node-modules-polyfill/-/esbuild-plugins-node-modules-polyfill-1.1.0.tgz#9b701b0bff618ce24ac262fd49aa49dd3da58197"
-  integrity sha512-pfJAbt00Luc9uuYtXGlaUrcTzf4h95Cr9Lfw+7smTFmZWtbwbrN5Hsf+La4lfD6OygHvZeefZFILOGK1ZnuyjA==
+esbuild-plugins-node-modules-polyfill@^1.2.0:
+  version "1.2.0"
+  resolved "https://registry.npmjs.org/esbuild-plugins-node-modules-polyfill/-/esbuild-plugins-node-modules-polyfill-1.2.0.tgz#06de57620bc470e78999ba99664dca936cfbe7c4"
+  integrity sha512-jWyCoWQKRbxUUfWLO900IyBDHihavTMbQLCDRq8xqj6NhQ75eW7YRvdraSDzFqQ6/eLnhNZlvantQtYbZjqU0A==
   dependencies:
     "@jspm/core" "^2.0.1"
     local-pkg "^0.4.3"