fix(dev): support polyfill package imports in deps

.changeset/polyfill-package-imports-in-deps.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/dev": patch
+---
+
+Support dependencies that import polyfill packages for Node built-ins via a trailing slash (e.g. importing the `buffer` package with `var Buffer = require('buffer/').Buffer` as recommended in their readme). These imports were previously marked as external. This meant that they were left as dynamic imports in the client bundle and would throw a runtime error in the browser (e.g. `Dynamic require of "buffer/" is not supported`).

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

@@ -1,8 +1,6 @@
 import { builtinModules as nodeBuiltins } from "node:module";
 import * as esbuild from "esbuild";
 
-import type { RemixConfig } from "../../config";
-import { getAppDependencies } from "../../dependencies";
 import { loaders } from "../utils/loaders";
 import { cssTarget } from "../utils/cssTarget";
 import { cssFilePlugin } from "../plugins/cssImports";
@@ -21,25 +19,6 @@ import type { Context } from "../context";
 import { groupCssBundleFiles } from "./bundle";
 import { writeMetafile } from "../analysis";
 
-const getExternals = (config: RemixConfig): string[] => {
-  // For the browser build, exclude node built-ins that don't have a
-  // browser-safe alternative installed in node_modules. Nothing should
-  // *actually* be external in the browser build (we want to bundle all deps) so
-  // this is really just making sure we don't accidentally have any dependencies
-  // on node built-ins in browser bundles.
-  let dependencies = Object.keys(getAppDependencies(config));
-  let fakeBuiltins = nodeBuiltins.filter((mod) => dependencies.includes(mod));
-
-  if (fakeBuiltins.length > 0) {
-    throw new Error(
-      `It appears you're using a module that is built in to node, but you installed it as a dependency which could cause problems. Please remove ${fakeBuiltins.join(
-        ", "
-      )} before continuing.`
-    );
-  }
-  return nodeBuiltins.filter((mod) => !dependencies.includes(mod));
-};
-
 const createCompilerEsbuildConfig = (ctx: Context): esbuild.BuildOptions => {
   return {
     entryPoints: {
@@ -48,7 +27,11 @@ const createCompilerEsbuildConfig = (ctx: Context): esbuild.BuildOptions => {
     outdir: ctx.config.assetsBuildDirectory,
     platform: "browser",
     format: "esm",
-    external: getExternals(ctx.config),
+    // Node built-ins (and any polyfills) are guaranteed to never contain CSS,
+    // and the JS from this build will never be executed, so we can safely skip
+    // bundling them and leave any imports of them as-is in the generated JS.
+    // Any issues with Node built-ins will be caught by the browser JS build.
+    external: nodeBuiltins,
     loader: loaders,
     bundle: true,
     logLevel: "silent",

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

@@ -33,23 +33,10 @@ type Compiler = {
   dispose: () => Promise<void>;
 };
 
-const getExternals = (remixConfig: RemixConfig): string[] => {
-  // For the browser build, exclude node built-ins that don't have a
-  // browser-safe alternative installed in node_modules. Nothing should
-  // *actually* be external in the browser build (we want to bundle all deps) so
-  // this is really just making sure we don't accidentally have any dependencies
-  // on node built-ins in browser bundles.
+const getFakeBuiltins = (remixConfig: RemixConfig): string[] => {
   let dependencies = Object.keys(getAppDependencies(remixConfig));
   let fakeBuiltins = nodeBuiltins.filter((mod) => dependencies.includes(mod));
-
-  if (fakeBuiltins.length > 0) {
-    throw new Error(
-      `It appears you're using a module that is built in to node, but you installed it as a dependency which could cause problems. Please remove ${fakeBuiltins.join(
-        ", "
-      )} before continuing.`
-    );
-  }
-  return nodeBuiltins.filter((mod) => !dependencies.includes(mod));
+  return fakeBuiltins;
 };
 
 const createEsbuildConfig = (
@@ -82,6 +69,15 @@ const createEsbuildConfig = (
     );
   }
 
+  let fakeBuiltins = getFakeBuiltins(ctx.config);
+  if (fakeBuiltins.length > 0) {
+    throw new Error(
+      `It appears you're using a module that is built in to Node, but you installed it as a dependency which could cause problems. Please remove ${fakeBuiltins.join(
+        ", "
+      )} before continuing.`
+    );
+  }
+
   let plugins: esbuild.Plugin[] = [
     browserRouteModulesPlugin(ctx, /\?browser$/),
     cssBundlePlugin(refs),
@@ -98,7 +94,14 @@ const createEsbuildConfig = (
     emptyModulesPlugin(ctx, /^@remix-run\/(deno|cloudflare|node)(\/.*)?$/, {
       includeNodeModules: true,
     }),
-    nodeModulesPolyfillPlugin(),
+    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 }),
   ];
 
@@ -111,7 +114,6 @@ const createEsbuildConfig = (
     outdir: ctx.config.assetsBuildDirectory,
     platform: "browser",
     format: "esm",
-    external: getExternals(ctx.config),
     loader: loaders,
     bundle: true,
     logLevel: "silent",

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.3.0",
+    "esbuild-plugins-node-modules-polyfill": "^1.4.0",
     "execa": "5.1.1",
     "exit-hook": "2.2.1",
     "express": "^4.17.1",

yarn.lock

@@ -5088,10 +5088,10 @@ es-to-primitive@^1.2.1:
     is-date-object "^1.0.1"
     is-symbol "^1.0.2"
 
-esbuild-plugins-node-modules-polyfill@^1.3.0:
-  version "1.3.0"
-  resolved "https://registry.npmjs.org/esbuild-plugins-node-modules-polyfill/-/esbuild-plugins-node-modules-polyfill-1.3.0.tgz#aa61ca6189d54b163acc503b9fcbbbc825f28226"
-  integrity sha512-r/aNOvAlIaIzqJwvFHWhDGrPF/Aj5qI1zKVeHbCFpKH+bnKW1BG2LGixMd3s6hyWcZHcfdl2QZRucVuOLzFRrA==
+esbuild-plugins-node-modules-polyfill@^1.4.0:
+  version "1.4.0"
+  resolved "https://registry.npmjs.org/esbuild-plugins-node-modules-polyfill/-/esbuild-plugins-node-modules-polyfill-1.4.0.tgz#2382a164fc07afc94ea4aef1d7ed46bfb4b02f1b"
+  integrity sha512-B+VD+hXhxbMCbIBsK9SEOrVIannCUefCNE1m4Fu0lrqK0NZKg+sMkmPZZJIg4cOXWt/RUv7AIB+VeVUlgVO8nw==
   dependencies:
     "@jspm/core" "^2.0.1"
     local-pkg "^0.4.3"