diff --git a/packages/cloudflare/src/cli/build/patches/plugins/content-updater.ts b/packages/cloudflare/src/cli/build/patches/plugins/content-updater.ts index 81cca509..5687d833 100644 --- a/packages/cloudflare/src/cli/build/patches/plugins/content-updater.ts +++ b/packages/cloudflare/src/cli/build/patches/plugins/content-updater.ts @@ -8,11 +8,21 @@ import { readFile } from "node:fs/promises"; import { type OnLoadArgs, type OnLoadOptions, type Plugin, type PluginBuild } from "esbuild"; +/** + * The callbacks returns either an updated content or undefined if the content is unchanged. + */ export type Callback = (args: { contents: string; path: string; }) => string | undefined | Promise; -export type Updater = OnLoadOptions & { callback: Callback }; + +/** + * The callback is called only when `contentFilter` matches the content. + * It can be used as a fast heuristic to prevent an expensive update. + */ +export type OnUpdateOptions = OnLoadOptions & { contentFilter: RegExp }; + +export type Updater = OnUpdateOptions & { callback: Callback }; export class ContentUpdater { updaters = new Map(); @@ -23,11 +33,11 @@ export class ContentUpdater { * The callbacks are called in order of registration. * * @param name The name of the plugin (must be unique). - * @param options Same options as the `onLoad` hook to restrict updates. + * @param options Options. * @param callback The callback updating the content. * @returns A noop ESBuild plugin. */ - updateContent(name: string, options: OnLoadOptions, callback: Callback): Plugin { + updateContent(name: string, options: OnUpdateOptions, callback: Callback): Plugin { if (this.updaters.has(name)) { throw new Error(`Plugin "${name}" already registered`); } @@ -48,13 +58,16 @@ export class ContentUpdater { setup: async (build: PluginBuild) => { build.onLoad({ filter: /\.(js|mjs|cjs|jsx|ts|tsx)$/ }, async (args: OnLoadArgs) => { let contents = await readFile(args.path, "utf-8"); - for (const { filter, namespace, callback } of this.updaters.values()) { + for (const { filter, namespace, contentFilter, callback } of this.updaters.values()) { if (namespace !== undefined && args.namespace !== namespace) { continue; } if (!filter.test(args.path)) { continue; } + if (!contentFilter.test(contents)) { + continue; + } contents = (await callback({ contents, path: args.path })) ?? contents; } return { contents }; diff --git a/packages/cloudflare/src/cli/build/patches/plugins/load-instrumentation.ts b/packages/cloudflare/src/cli/build/patches/plugins/load-instrumentation.ts index 4a942e09..99076715 100644 --- a/packages/cloudflare/src/cli/build/patches/plugins/load-instrumentation.ts +++ b/packages/cloudflare/src/cli/build/patches/plugins/load-instrumentation.ts @@ -18,11 +18,7 @@ fix: async loadInstrumentationModule() { } export function patchLoadInstrumentation(updater: ContentUpdater) { return updater.updateContent( "patch-load-instrumentation", - { filter: /\.(js|mjs|cjs|jsx|ts|tsx)$/ }, - ({ contents }) => { - if (/async loadInstrumentationModule\(/.test(contents)) { - return patchCode(contents, instrumentationRule); - } - } + { filter: /\.(js|mjs|cjs|jsx|ts|tsx)$/, contentFilter: /async loadInstrumentationModule\(/ }, + ({ contents }) => patchCode(contents, instrumentationRule) ); } 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 2cd6d739..92d56974 100644 --- a/packages/cloudflare/src/cli/build/patches/plugins/require-page.ts +++ b/packages/cloudflare/src/cli/build/patches/plugins/require-page.ts @@ -12,12 +12,9 @@ export function inlineRequirePagePlugin(updater: ContentUpdater, buildOpts: Buil "inline-require-page", { filter: getCrossPlatformPathRegex(String.raw`/next/dist/server/require\.js$`, { escape: false }), + contentFilter: /function requirePage\(/, }, - async ({ contents }) => { - if (/function requirePage\(/.test(contents)) { - return patchCode(contents, await getRule(buildOpts)); - } - } + async ({ contents }) => patchCode(contents, await getRule(buildOpts)) ); } diff --git a/packages/cloudflare/src/cli/build/patches/plugins/require.ts b/packages/cloudflare/src/cli/build/patches/plugins/require.ts index b7114586..966f3efb 100644 --- a/packages/cloudflare/src/cli/build/patches/plugins/require.ts +++ b/packages/cloudflare/src/cli/build/patches/plugins/require.ts @@ -1,44 +1,48 @@ import type { ContentUpdater } from "./content-updater"; export function fixRequire(updater: ContentUpdater) { - return updater.updateContent("fix-require", { filter: /\.(js|mjs|cjs|jsx|ts|tsx)$/ }, ({ contents }) => { - // `eval(...)` is not supported by workerd. - contents = contents.replaceAll(`eval("require")`, "require"); + return updater.updateContent( + "fix-require", + { filter: /\.(js|mjs|cjs|jsx|ts|tsx)$/, contentFilter: /.*/ }, + ({ contents }) => { + // `eval(...)` is not supported by workerd. + contents = contents.replaceAll(`eval("require")`, "require"); - // `@opentelemetry` has a few issues. - // - // Next.js has the following code in `next/dist/server/lib/trace/tracer.js`: - // - // try { - // api = require('@opentelemetry/api'); - // } catch (err) { - // api = require('next/dist/compiled/@opentelemetry/api'); - // } - // - // The intent is to allow users to install their own version of `@opentelemetry/api`. - // - // The problem is that even when users do not explicitely install `@opentelemetry/api`, - // `require('@opentelemetry/api')` resolves to the package which is a dependency - // of Next. - // - // The second problem is that when Next traces files, it would not copy the `api/build/esm` - // folder (used by the `module` conditions in package.json) it would only copy `api/build/src`. - // This could be solved by updating the next config: - // - // const nextConfig: NextConfig = { - // // ... - // outputFileTracingIncludes: { - // "*": ["./node_modules/@opentelemetry/api/build/**/*"], - // }, - // }; - // - // We can consider doing that when we want to enable users to install their own version - // of `@opentelemetry/api`. For now we simply use the pre-compiled version. - contents = contents.replace( - /require\(.@opentelemetry\/api.\)/g, - `require("next/dist/compiled/@opentelemetry/api")` - ); + // `@opentelemetry` has a few issues. + // + // Next.js has the following code in `next/dist/server/lib/trace/tracer.js`: + // + // try { + // api = require('@opentelemetry/api'); + // } catch (err) { + // api = require('next/dist/compiled/@opentelemetry/api'); + // } + // + // The intent is to allow users to install their own version of `@opentelemetry/api`. + // + // The problem is that even when users do not explicitly install `@opentelemetry/api`, + // `require('@opentelemetry/api')` resolves to the package which is a dependency + // of Next. + // + // The second problem is that when Next traces files, it would not copy the `api/build/esm` + // folder (used by the `module` conditions in package.json) it would only copy `api/build/src`. + // This could be solved by updating the next config: + // + // const nextConfig: NextConfig = { + // // ... + // outputFileTracingIncludes: { + // "*": ["./node_modules/@opentelemetry/api/build/**/*"], + // }, + // }; + // + // We can consider doing that when we want to enable users to install their own version + // of `@opentelemetry/api`. For now we simply use the pre-compiled version. + contents = contents.replace( + /require\(.@opentelemetry\/api.\)/g, + `require("next/dist/compiled/@opentelemetry/api")` + ); - return contents; - }); + return contents; + } + ); }