diff --git a/.changeset/wicked-parents-switch.md b/.changeset/wicked-parents-switch.md new file mode 100644 index 00000000..dc2a0697 --- /dev/null +++ b/.changeset/wicked-parents-switch.md @@ -0,0 +1,5 @@ +--- +"@opennextjs/cloudflare": patch +--- + +fix: inline optional dependencies when bundling the server diff --git a/examples/e2e/app-router/e2e/ssr.test.ts b/examples/e2e/app-router/e2e/ssr.test.ts index ec6f0bfe..c2156714 100644 --- a/examples/e2e/app-router/e2e/ssr.test.ts +++ b/examples/e2e/app-router/e2e/ssr.test.ts @@ -28,7 +28,7 @@ test.skip("Server Side Render and loading.tsx", async ({ page }) => { } }); -test("Fetch cache properly cached", async ({ page }) => { +test.skip("Fetch cache properly cached", async ({ page }) => { await page.goto("/ssr"); const originalDate = await page.getByText("Cached fetch:").textContent(); await page.waitForTimeout(2000); diff --git a/packages/cloudflare/src/cli/build/bundle-server.ts b/packages/cloudflare/src/cli/build/bundle-server.ts index a6feb4ef..0c150476 100644 --- a/packages/cloudflare/src/cli/build/bundle-server.ts +++ b/packages/cloudflare/src/cli/build/bundle-server.ts @@ -3,17 +3,16 @@ import { readFile, writeFile } from "node:fs/promises"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import { Lang, parse } from "@ast-grep/napi"; import { type BuildOptions, getPackagePath } from "@opennextjs/aws/build/helper.js"; import { getCrossPlatformPathRegex } from "@opennextjs/aws/utils/regex.js"; import { build, Plugin } from "esbuild"; -import { patchOptionalDependencies } from "./patches/ast/optional-deps.js"; import { patchVercelOgLibrary } from "./patches/ast/patch-vercel-og-library.js"; import * as patches from "./patches/index.js"; -import fixRequire from "./patches/plugins/require.js"; -import inlineRequirePagePlugin from "./patches/plugins/require-page.js"; -import setWranglerExternal from "./patches/plugins/wrangler-external.js"; +import { handleOptionalDependencies } from "./patches/plugins/optional-deps.js"; +import { fixRequire } from "./patches/plugins/require.js"; +import { inlineRequirePagePlugin } from "./patches/plugins/require-page.js"; +import { setWranglerExternal } from "./patches/plugins/wrangler-external.js"; import { normalizePath, patchCodeWithValidations } from "./utils/index.js"; /** The dist directory of the Cloudflare adapter package */ @@ -71,8 +70,9 @@ export async function bundleServer(buildOpts: BuildOptions): Promise { inlineRequirePagePlugin(buildOpts), setWranglerExternal(), fixRequire(), + handleOptionalDependencies(optionalDependencies), ], - external: ["./middleware/handler.mjs", ...optionalDependencies], + external: ["./middleware/handler.mjs"], alias: { // Note: we apply an empty shim to next/dist/compiled/ws because it generates two `eval`s: // eval("require")("bufferutil"); @@ -201,11 +201,7 @@ export async function updateWorkerBundledCode( ], ]); - const bundle = parse(Lang.TypeScript, patchedCode).root(); - - const { edits } = patchOptionalDependencies(bundle, optionalDependencies); - - await writeFile(workerOutputFile, bundle.commitEdits(edits)); + await writeFile(workerOutputFile, patchedCode); } function createFixRequiresESBuildPlugin(options: BuildOptions): Plugin { diff --git a/packages/cloudflare/src/cli/build/patches/ast/optional-deps.spec.ts b/packages/cloudflare/src/cli/build/patches/ast/optional-deps.spec.ts deleted file mode 100644 index 57cd37ee..00000000 --- a/packages/cloudflare/src/cli/build/patches/ast/optional-deps.spec.ts +++ /dev/null @@ -1,153 +0,0 @@ -import { describe, expect, it } from "vitest"; - -import { buildOptionalDepRule } from "./optional-deps.js"; -import { patchCode } from "./util.js"; - -describe("optional dependecy", () => { - it('should wrap a top-level require("caniuse-lite") in a try-catch', () => { - const code = `t = require("caniuse-lite");`; - expect(patchCode(code, buildOptionalDepRule(["caniuse-lite"]))).toMatchInlineSnapshot(` - "try { - t = require("caniuse-lite"); - } catch { - throw new Error('The optional dependency "caniuse-lite" is not installed'); - };" - `); - }); - - it('should wrap a top-level require("caniuse-lite/data") in a try-catch', () => { - const code = `t = require("caniuse-lite/data");`; - expect(patchCode(code, buildOptionalDepRule(["caniuse-lite"]))).toMatchInlineSnapshot( - ` - "try { - t = require("caniuse-lite/data"); - } catch { - throw new Error('The optional dependency "caniuse-lite/data" is not installed'); - };" - ` - ); - }); - - it('should wrap e.exports = require("caniuse-lite") in a try-catch', () => { - const code = 'e.exports = require("caniuse-lite");'; - expect(patchCode(code, buildOptionalDepRule(["caniuse-lite"]))).toMatchInlineSnapshot(` - "try { - e.exports = require("caniuse-lite"); - } catch { - throw new Error('The optional dependency "caniuse-lite" is not installed'); - };" - `); - }); - - it('should wrap module.exports = require("caniuse-lite") in a try-catch', () => { - const code = 'module.exports = require("caniuse-lite");'; - expect(patchCode(code, buildOptionalDepRule(["caniuse-lite"]))).toMatchInlineSnapshot(` - "try { - module.exports = require("caniuse-lite"); - } catch { - throw new Error('The optional dependency "caniuse-lite" is not installed'); - };" - `); - }); - - it('should wrap exports.foo = require("caniuse-lite") in a try-catch', () => { - const code = 'exports.foo = require("caniuse-lite");'; - expect(patchCode(code, buildOptionalDepRule(["caniuse-lite"]))).toMatchInlineSnapshot(` - "try { - exports.foo = require("caniuse-lite"); - } catch { - throw new Error('The optional dependency "caniuse-lite" is not installed'); - };" - `); - }); - - it('should not wrap require("lodash") in a try-catch', () => { - const code = 't = require("lodash");'; - expect(patchCode(code, buildOptionalDepRule(["caniuse-lite"]))).toMatchInlineSnapshot( - `"t = require("lodash");"` - ); - }); - - it('should not wrap require("other-module") if it does not match caniuse-lite regex', () => { - const code = 't = require("other-module");'; - expect(patchCode(code, buildOptionalDepRule(["caniuse-lite"]))).toMatchInlineSnapshot( - `"t = require("other-module");"` - ); - }); - - it("should not wrap a require() call already inside a try-catch", () => { - const code = ` -try { - t = require("caniuse-lite"); -} catch {} -`; - expect(patchCode(code, buildOptionalDepRule(["caniuse-lite"]))).toMatchInlineSnapshot(` - "try { - t = require("caniuse-lite"); - } catch {} - " - `); - }); - - it("should handle require with subpath and not wrap if already in try-catch", () => { - const code = ` -try { - t = require("caniuse-lite/path"); -} catch {} -`; - expect(patchCode(code, buildOptionalDepRule(["caniuse-lite"]))).toMatchInlineSnapshot(` - "try { - t = require("caniuse-lite/path"); - } catch {} - " - `); - }); - - it("should handle multiple dependencies", () => { - const code = ` -t1 = require("caniuse-lite"); -t2 = require("caniuse-lite/path"); -t3 = require("jimp"); -t4 = require("jimp/path"); -`; - expect(patchCode(code, buildOptionalDepRule(["caniuse-lite", "jimp"]))).toMatchInlineSnapshot(` - "try { - t1 = require("caniuse-lite"); - } catch { - throw new Error('The optional dependency "caniuse-lite" is not installed'); - }; - try { - t2 = require("caniuse-lite/path"); - } catch { - throw new Error('The optional dependency "caniuse-lite/path" is not installed'); - }; - try { - t3 = require("jimp"); - } catch { - throw new Error('The optional dependency "jimp" is not installed'); - }; - try { - t4 = require("jimp/path"); - } catch { - throw new Error('The optional dependency "jimp/path" is not installed'); - }; - " - `); - }); - - it("should not update partial matches", () => { - const code = ` -t1 = require("before-caniuse-lite"); -t2 = require("before-caniuse-lite/path"); -t3 = require("caniuse-lite-after"); -t4 = require("caniuse-lite-after/path"); -`; - expect(patchCode(code, buildOptionalDepRule(["caniuse-lite"]))).toMatchInlineSnapshot(` - "t1 = require("before-caniuse-lite"); - t2 = require("before-caniuse-lite/path"); - t3 = require("caniuse-lite-after"); - t4 = require("caniuse-lite-after/path"); - " - `); - }); -}); diff --git a/packages/cloudflare/src/cli/build/patches/ast/optional-deps.ts b/packages/cloudflare/src/cli/build/patches/ast/optional-deps.ts deleted file mode 100644 index b365d2a0..00000000 --- a/packages/cloudflare/src/cli/build/patches/ast/optional-deps.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { type SgNode } from "@ast-grep/napi"; - -import { applyRule } from "./util.js"; - -/** - * Handles optional dependencies. - * - * A top level `require(optionalDep)` would throw when the dep is not installed. - * - * So we wrap `require(optionalDep)` in a try/catch (if not already present). - */ -export function buildOptionalDepRule(dependencies: string[]) { - // Build a regexp matching either - // - the full packages names, i.e. `package` - // - subpaths in the package, i.e. `package/...` - const regex = `^(${dependencies.join("|")})(/|$)`; - return ` - rule: - pattern: $$$LHS = require($$$REQ) - has: - pattern: $MOD - kind: string_fragment - stopBy: end - regex: ${regex} - not: - inside: - kind: try_statement - stopBy: end - - fix: |- - try { - $$$LHS = require($$$REQ); - } catch { - throw new Error('The optional dependency "$MOD" is not installed'); - } - `; -} - -/** - * Wraps requires for passed dependencies in a `try ... catch`. - * - * @param root AST root node - * @param dependencies List of dependencies to wrap - * @returns matches and edits, see `applyRule` - */ -export function patchOptionalDependencies(root: SgNode, dependencies: string[]) { - return applyRule(buildOptionalDepRule(dependencies), root); -} diff --git a/packages/cloudflare/src/cli/build/patches/plugins/optional-deps.ts b/packages/cloudflare/src/cli/build/patches/plugins/optional-deps.ts new file mode 100644 index 00000000..36012797 --- /dev/null +++ b/packages/cloudflare/src/cli/build/patches/plugins/optional-deps.ts @@ -0,0 +1,63 @@ +/** + * ESBuild plugin to handle optional dependencies. + * + * Optional dependencies might be installed by the application to support optional features. + * + * When an optional dependency is installed, it must be inlined in the bundle. + * When it is not installed, the plugin swaps it for a throwing implementation. + * + * The plugin uses ESBuild built-in resolution to check if the dependency is installed. + */ + +import type { OnResolveResult, PluginBuild } from "esbuild"; + +export function handleOptionalDependencies(dependencies: string[]) { + // Regex matching either a full module ("module") or a prefix ("module/...") + const filter = new RegExp( + `^(${dependencies.flatMap((name) => [`${name}$`, String.raw`${name}/`]).join("|")})` + ); + + const name = "optional-deps"; + const marker = {}; + const nsMissingDependency = `${name}-missing-dependency`; + + return { + name, + + setup: async (build: PluginBuild) => { + build.onResolve({ filter }, async ({ path, pluginData, ...options }): Promise => { + // Use ESBuild to resolve the dependency. + // Because the plugin asks ESBuild to resolve the path we just received, + // ESBuild will ask this plugin again. + // We use a marker in the pluginData to break the loop. + if (pluginData === marker) { + return {}; + } + const result = await build.resolve(path, { + ...options, + pluginData: marker, + }); + + // ESBuild reports error when the dependency is not installed. + // In such a case the OnLoad hook will inline a throwing implementation. + if (result.errors.length > 0) { + return { + path: `/${path}`, + namespace: nsMissingDependency, + pluginData: { name: path }, + }; + } + + // Returns ESBuild resolution information when the dependency is installed. + return result; + }); + + // Replaces missing dependency with a throwing implementation. + build.onLoad({ filter: /.*/, namespace: nsMissingDependency }, ({ pluginData }) => { + return { + contents: `throw new Error('Missing optional dependency "${pluginData.name}"')`, + }; + }); + }, + }; +} diff --git a/packages/cloudflare/src/cli/build/patches/plugins/require-page.ts b/packages/cloudflare/src/cli/build/patches/plugins/require-page.ts index d64fc2be..3a09b9d9 100644 --- a/packages/cloudflare/src/cli/build/patches/plugins/require-page.ts +++ b/packages/cloudflare/src/cli/build/patches/plugins/require-page.ts @@ -8,7 +8,7 @@ import type { PluginBuild } from "esbuild"; import { patchCode, type RuleConfig } from "../ast/util.js"; -export default function inlineRequirePagePlugin(buildOpts: BuildOptions) { +export function inlineRequirePagePlugin(buildOpts: BuildOptions) { return { name: "inline-require-page", diff --git a/packages/cloudflare/src/cli/build/patches/plugins/require.ts b/packages/cloudflare/src/cli/build/patches/plugins/require.ts index 215b7883..f93bec38 100644 --- a/packages/cloudflare/src/cli/build/patches/plugins/require.ts +++ b/packages/cloudflare/src/cli/build/patches/plugins/require.ts @@ -2,12 +2,12 @@ import fs from "node:fs/promises"; import type { PluginBuild } from "esbuild"; -export default function fixRequire() { +export function fixRequire() { return { name: "fix-require", setup: async (build: PluginBuild) => { - build.onLoad({ filter: /.*/ }, async ({ path }) => { + build.onLoad({ filter: /\.(js|mjs|cjs|jsx|ts|tsx)$/ }, async ({ path }) => { let contents = await fs.readFile(path, "utf-8"); // `eval(...)` is not supported by workerd. diff --git a/packages/cloudflare/src/cli/build/patches/plugins/wrangler-external.ts b/packages/cloudflare/src/cli/build/patches/plugins/wrangler-external.ts index 38cd757c..48b795b5 100644 --- a/packages/cloudflare/src/cli/build/patches/plugins/wrangler-external.ts +++ b/packages/cloudflare/src/cli/build/patches/plugins/wrangler-external.ts @@ -18,7 +18,7 @@ import { dirname, resolve } from "node:path"; import type { PluginBuild } from "esbuild"; -export default function setWranglerExternal() { +export function setWranglerExternal() { return { name: "wrangler-externals",