diff --git a/.changeset/server-node-builtins-polyfill.md b/.changeset/server-node-builtins-polyfill.md index 017221ff14f..7d01aaf8409 100644 --- a/.changeset/server-node-builtins-polyfill.md +++ b/.changeset/server-node-builtins-polyfill.md @@ -6,7 +6,7 @@ Add `serverNodeBuiltinsPolyfill` config option. In `remix.config.js` you can now ```js // Disable all polyfills -exports.serverNodeBuiltinsPolyfill = false; +exports.serverNodeBuiltinsPolyfill = { modules: {} }; // Enable specific polyfills exports.serverNodeBuiltinsPolyfill = { diff --git a/.changeset/wise-drinks-juggle.md b/.changeset/wise-drinks-juggle.md new file mode 100644 index 00000000000..206cdaaf573 --- /dev/null +++ b/.changeset/wise-drinks-juggle.md @@ -0,0 +1,5 @@ +--- +"@remix-run/dev": minor +--- + +[REMOVE] Remove boolean option from serverNodeBuiltinsPolyfill, and revert to "empty" polyfill for `fs`/`fs/promises` and `crypto` modules diff --git a/docs/file-conventions/remix-config.md b/docs/file-conventions/remix-config.md index a4b7ff41e90..8d1b05c1dc6 100644 --- a/docs/file-conventions/remix-config.md +++ b/docs/file-conventions/remix-config.md @@ -193,21 +193,72 @@ Defaults to `"cjs"`. ## serverNodeBuiltinsPolyfill -Whether to polyfill Node.js built-in modules in the server build, or a configurable subset of polyfills. Polyfills are provided by [JSPM][jspm] and configured via [esbuild-plugins-node-modules-polyfill]. Defaults to `true` for non-Node.js server platforms. +The Node.js polyfills to include in the server build when targeting non-Node.js server platforms. Polyfills are provided by [JSPM][jspm] and configured via [esbuild-plugins-node-modules-polyfill]. ```js filename=remix.config.js -// Disable all polyfills -exports.serverNodeBuiltinsPolyfill = false; +exports.serverNodeBuiltinsPolyfill = { + modules: { + path: true, // Provide a JSPM polyfill + fs: "empty", // Provide an empty polyfill + }, +}; +``` + +If left unset, this config defaults to the following set of polyfills for non-Node.js server platforms: -// Enable specific polyfills +```js filename=remix.config.js exports.serverNodeBuiltinsPolyfill = { modules: { - crypto: true, // Provide a JSPM polyfill - fs: 'empty', // Provide an empty polyfill + _stream_duplex: true, + _stream_passthrough: true, + _stream_readable: true, + _stream_transform: true, + _stream_writable: true, + assert: true, + "assert/strict": true, + buffer: true, + console: true, + constants: true, + crypto: "empty", + diagnostics_channel: true, + domain: true, + events: true, + fs: "empty", + "fs/promises": "empty", + http: true, + https: true, + module: true, + os: true, + path: true, + "path/posix": true, + "path/win32": true, + perf_hooks: true, + process: true, + punycode: true, + querystring: true, + stream: true, + "stream/promises": true, + "stream/web": true, + string_decoder: true, + sys: true, + timers: true, + "timers/promises": true, + tty: true, + url: true, + util: true, + "util/types": true, + vm: true, + wasi: true, + worker_threads: true, + zlib: true, }, }; ``` + +This default behavior is changing in Remix v2 and will no longer polyfill any Node built-in modules on non-Node.js platforms by default. If your app requires polyfills, you will need to manually specify them via this setting. It's recommend to start manually specifying required polyfills for your app in v1 to ease your eventual migration to v2. + + ## 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 index f941b1d5433..42c19303bbb 100644 --- a/docs/pages/v2.md +++ b/docs/pages/v2.md @@ -705,9 +705,83 @@ In your `remix.config.js`, you should specify either `serverModuleFormat: "cjs"` ## `serverNodeBuiltinsPolyfill` -We will no longer polyfill Node.js built-in modules by default for non-Node.js server platforms. +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, 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"]`. +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`: + +```js filename=remix.config.js +module.exports = { + serverNodeBuiltinsPolyfill: { + modules: {}, + }, +}; +``` + +You can then reintroduce any polyfills (or blank polyfills) as required. + +```js filename=remix.config.js +module.exports = { + serverNodeBuiltinsPolyfill: { + modules: { + path: true, + fs: 'empty', + }, + }, +}; +``` + +For reference, the complete set of default polyfills from v1 can be manually specified as follows: + +```js filename=remix.config.js +module.exports = { + serverNodeBuiltinsPolyfill: { + modules: { + _stream_duplex: true, + _stream_passthrough: true, + _stream_readable: true, + _stream_transform: true, + _stream_writable: true, + assert: true, + "assert/strict": true, + buffer: true, + console: true, + constants: true, + crypto: "empty", + diagnostics_channel: true, + domain: true, + events: true, + fs: "empty", + "fs/promises": "empty", + http: true, + https: true, + module: true, + os: true, + path: true, + "path/posix": true, + "path/win32": true, + perf_hooks: true, + process: true, + punycode: true, + querystring: true, + stream: true, + "stream/promises": true, + "stream/web": true, + string_decoder: true, + sys: true, + timers: true, + "timers/promises": true, + tty: true, + url: true, + util: true, + "util/types": true, + vm: true, + wasi: true, + worker_threads: true, + zlib: true, + }, + }, +}; +``` ## Dev Server diff --git a/packages/remix-dev/__tests__/readConfig-test.ts b/packages/remix-dev/__tests__/readConfig-test.ts index fa3a50cd218..5eeb555d3b9 100644 --- a/packages/remix-dev/__tests__/readConfig-test.ts +++ b/packages/remix-dev/__tests__/readConfig-test.ts @@ -80,7 +80,7 @@ describe("readConfig", () => { "serverMinify": false, "serverMode": "production", "serverModuleFormat": "cjs", - "serverNodeBuiltinsPolyfill": false, + "serverNodeBuiltinsPolyfill": undefined, "serverPlatform": "node", "tailwind": false, "tsconfigPath": Any, diff --git a/packages/remix-dev/compiler/server/compiler.ts b/packages/remix-dev/compiler/server/compiler.ts index aef56cb958c..e96c5c20878 100644 --- a/packages/remix-dev/compiler/server/compiler.ts +++ b/packages/remix-dev/compiler/server/compiler.ts @@ -1,9 +1,5 @@ -import { builtinModules } from "module"; import * as esbuild from "esbuild"; -import { - type NodePolyfillsOptions, - nodeModulesPolyfillPlugin, -} from "esbuild-plugins-node-modules-polyfill"; +import { nodeModulesPolyfillPlugin } from "esbuild-plugins-node-modules-polyfill"; import { type Manifest } from "../../manifest"; import { loaders } from "../utils/loaders"; @@ -72,42 +68,11 @@ const createEsbuildConfig = ( ]; 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 - ]; - - let defaultPolyfillOptions: NodePolyfillsOptions = { - modules: builtinModules.filter((mod) => !unimplemented.includes(mod)), - }; - plugins.unshift( - nodeModulesPolyfillPlugin( - ctx.config.serverNodeBuiltinsPolyfill === true - ? defaultPolyfillOptions - : { - // Ensure only "modules" option is passed to the plugin - modules: ctx.config.serverNodeBuiltinsPolyfill.modules, - } - ) + nodeModulesPolyfillPlugin({ + // Ensure only "modules" option is passed to the plugin + modules: ctx.config.serverNodeBuiltinsPolyfill.modules, + }) ); } diff --git a/packages/remix-dev/config.ts b/packages/remix-dev/config.ts index 91aa215dff3..8c58149843e 100644 --- a/packages/remix-dev/config.ts +++ b/packages/remix-dev/config.ts @@ -5,7 +5,7 @@ import fse from "fs-extra"; import getPort from "get-port"; import NPMCliPackageJson from "@npmcli/package-json"; import { coerce } from "semver"; -import type { NodePolyfillsOptions } from "esbuild-plugins-node-modules-polyfill"; +import type { NodePolyfillsOptions as EsbuildPluginsNodeModulesPolyfillOptions } from "esbuild-plugins-node-modules-polyfill"; import type { RouteManifest, DefineRoutesFunction } from "./config/routes"; import { defineRoutes } from "./config/routes"; @@ -65,6 +65,11 @@ interface FutureConfig { v2_routeConvention: boolean; } +type ServerNodeBuiltinsPolyfillOptions = Pick< + EsbuildPluginsNodeModulesPolyfillOptions, + "modules" +>; + /** * The user-provided config in `remix.config.js`. */ @@ -196,11 +201,10 @@ export interface AppConfig { serverModuleFormat?: ServerModuleFormat; /** - * Whether to polyfill Node.js built-in modules in the server build, or a - * configurable subset of polyfills. Defaults to `true` for non-Node.js server - * platforms. + * The Node.js polyfills to include in the server build when targeting + * non-Node.js server platforms. */ - serverNodeBuiltinsPolyfill?: boolean | Pick; + serverNodeBuiltinsPolyfill?: ServerNodeBuiltinsPolyfillOptions; /** * The platform the server build is targeting. Defaults to "node". @@ -374,11 +378,10 @@ export interface RemixConfig { serverModuleFormat: ServerModuleFormat; /** - * Whether to polyfill Node.js built-in modules in the server build, or a - * configurable subset of polyfills. Defaults to `true` for non-Node.js server - * platforms. + * The Node.js polyfills to include in the server build when targeting + * non-Node.js server platforms. */ - serverNodeBuiltinsPolyfill: boolean | Pick; + serverNodeBuiltinsPolyfill?: ServerNodeBuiltinsPolyfillOptions; /** * The platform the server build is targeting. Defaults to "node". @@ -506,18 +509,74 @@ export async function readConfig( serverModuleFormat === "esm" ? ["module", "main"] : ["main", "module"]; serverMinify ??= false; - let serverNodeBuiltinsPolyfill: RemixConfig["serverNodeBuiltinsPolyfill"] = - serverPlatform !== "node"; + let serverNodeBuiltinsPolyfill: RemixConfig["serverNodeBuiltinsPolyfill"]; - if (appConfig.serverNodeBuiltinsPolyfill !== undefined) { + if (appConfig.serverNodeBuiltinsPolyfill != null) { serverNodeBuiltinsPolyfill = appConfig.serverNodeBuiltinsPolyfill; - } - - if ( - serverPlatform !== "node" && - appConfig.serverNodeBuiltinsPolyfill === undefined - ) { + } else if (serverPlatform !== "node") { serverNodeBuiltinsPolyfillWarning(); + serverNodeBuiltinsPolyfill = { + modules: { + // Note: Remove this in Remix v2 + // All polyfills are ultimately sourced from JSPM: https://github.com/jspm/jspm-core/tree/main/nodelibs/browser + // Polyfills we choose to disable are explicitly configured here so we can note the reason for disabling them. + // Links are provided here to make it easier to review the source code for each polyfill. + _stream_duplex: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/_stream_duplex.js + _stream_passthrough: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/_stream_passthrough.js + _stream_readable: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/_stream_readable.js + _stream_transform: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/_stream_transform.js + _stream_writable: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/_stream_writable.js + assert: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/assert.js + "assert/strict": true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/assert/strict.js + async_hooks: false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/async_hooks.js. Also Cloudflare Workers provides an implementation: https://developers.cloudflare.com/workers/runtime-apis/nodejs/asynclocalstorage/ + buffer: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/buffer.js + child_process: false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/child_process.js + cluster: false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/cluster.js + console: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/console.js + constants: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/constants.js + crypto: "empty", // Polyfill exists (https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/crypto.js) but source code is over 2MB! Also, it was "empty" in esbuild-plugin-polyfill-node which we used previously as of Remix v1.17.0: https://github.com/cyco130/esbuild-plugin-polyfill-node/blob/9afcb6abaf9062a15daaffce9a14e478b365139c/src/index.ts#L144 + dgram: false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/dgram.js + diagnostics_channel: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/diagnostics_channel.js + dns: false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/dns.js + "dns/promises": false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/dns/promises.js + domain: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/domain.js + events: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/events.js + fs: "empty", // Polyfill was "empty" in esbuild-plugin-polyfill-node which we used previously as of Remix v1.17.0 (https://github.com/cyco130/esbuild-plugin-polyfill-node/blob/9afcb6abaf9062a15daaffce9a14e478b365139c/src/index.ts#L143C6-L143C6). Also, the polyfill immediately throws when importing in Cloudflare Workers due to top-level setTimeout usage which is not allowed outside of the request lifecycle: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/fs.js + "fs/promises": "empty", // See above + http: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/http.js + http2: false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/http2.js + https: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/https.js + module: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/module.js + net: false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/net.js + os: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/os.js + path: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/path.js + "path/posix": true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/path/posix.js + "path/win32": true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/path/win32.js + perf_hooks: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/perf_hooks.js + process: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/process.js + punycode: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/punycode.js + querystring: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/querystring.js + readline: false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/readline.js + repl: false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/repl.js + stream: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/stream.js + "stream/promises": true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/stream/promises.js + "stream/web": true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/stream/web.js + string_decoder: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/string_decoder.js + sys: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/sys.js + timers: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/timers.js + "timers/promises": true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/timers/promises.js + tls: false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/tls.js + tty: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/tty.js - Effectively not implemented, but provides `isatty` as `false` so consumers can check to avoid it + url: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/url.js + util: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/util.js + "util/types": true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/util/types.js + v8: false, // Unimplemented, throws on usage: https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/v8.js + vm: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/vm.js + wasi: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/wasi.js + worker_threads: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/worker_threads.js + zlib: true, // https://github.com/jspm/jspm-core/blob/main/nodelibs/browser/zlib.js + }, + }; } if (appConfig.future) { @@ -1014,9 +1073,8 @@ let serverNodeBuiltinsPolyfillWarning = () => "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.", + "Server polyfills will no longer be provided by default for non-Node.js platforms.", + "You can prepare for this change by specifying server polyfills, or opting out entirely.", "-> https://remix.run/docs/en/v1.19.0/pages/v2#servernodebuiltinspolyfill", ], key: "serverNodeBuiltinsPolyfillWarning",