Skip to content

Commit

Permalink
fix: inline optional dependencies when bundling the server (#325)
Browse files Browse the repository at this point in the history
Co-authored-by: James Anderson <james@eli.cx>
  • Loading branch information
vicb and james-elicx authored Feb 3, 2025
1 parent da7f8d8 commit 0892679
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 217 deletions.
5 changes: 5 additions & 0 deletions .changeset/wicked-parents-switch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@opennextjs/cloudflare": patch
---

fix: inline optional dependencies when bundling the server
2 changes: 1 addition & 1 deletion examples/e2e/app-router/e2e/ssr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 7 additions & 11 deletions packages/cloudflare/src/cli/build/bundle-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -71,8 +70,9 @@ export async function bundleServer(buildOpts: BuildOptions): Promise<void> {
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");
Expand Down Expand Up @@ -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 {
Expand Down
153 changes: 0 additions & 153 deletions packages/cloudflare/src/cli/build/patches/ast/optional-deps.spec.ts

This file was deleted.

48 changes: 0 additions & 48 deletions packages/cloudflare/src/cli/build/patches/ast/optional-deps.ts

This file was deleted.

63 changes: 63 additions & 0 deletions packages/cloudflare/src/cli/build/patches/plugins/optional-deps.ts
Original file line number Diff line number Diff line change
@@ -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<OnResolveResult> => {
// 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}"')`,
};
});
},
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -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",

Expand Down
4 changes: 2 additions & 2 deletions packages/cloudflare/src/cli/build/patches/plugins/require.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",

Expand Down

0 comments on commit 0892679

Please sign in to comment.