Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: patch @vercel/og usage to use the edge runtime version #283

Merged
merged 13 commits into from
Jan 27, 2025
7 changes: 7 additions & 0 deletions .changeset/quick-timers-fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@opennextjs/cloudflare": patch
---

fix: @vercel/og failing due to using the node version.

Patches usage of the @vercel/og library to require the edge runtime version, and enables importing of the fallback font.
1 change: 0 additions & 1 deletion .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ on:
push:
branches: [main, experimental]
pull_request:
branches: [main, experimental]
vicb marked this conversation as resolved.
Show resolved Hide resolved

jobs:
checks:
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/playwright.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ on:
push:
branches: [main]
pull_request:
branches: [main]

jobs:
test:
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/prereleases.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ on:
push:
branches: [main, experimental]
pull_request:
branches: [main, experimental]

jobs:
release:
Expand Down
65 changes: 65 additions & 0 deletions examples/api/app/og/route.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { ImageResponse } from "next/og";

export const dynamic = "force-dynamic";

export async function GET() {
try {
return new ImageResponse(
(
<div
style={{
backgroundColor: "black",
backgroundSize: "150px 150px",
height: "100%",
width: "100%",
display: "flex",
textAlign: "center",
alignItems: "center",
justifyContent: "center",
flexDirection: "column",
flexWrap: "nowrap",
}}
>
<div
style={{
display: "flex",
alignItems: "center",
justifyContent: "center",
justifyItems: "center",
}}
>
<img
alt="Vercel"
height={200}
src="data:image/svg+xml,%3Csvg width='116' height='100' fill='white' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='M57.5 0L115 100H0L57.5 0z' /%3E%3C/svg%3E"
style={{ margin: "0 30px" }}
width={232}
/>
</div>
<div
style={{
fontSize: 60,
fontStyle: "normal",
letterSpacing: "-0.025em",
color: "white",
marginTop: 30,
padding: "0 120px",
lineHeight: 1.4,
whiteSpace: "pre-wrap",
}}
>
'next/og'
</div>
</div>
),
{
width: 1200,
height: 630,
}
);
} catch (e: any) {
return new Response("Failed to generate the image", {
status: 500,
});
}
}
19 changes: 19 additions & 0 deletions examples/api/e2e/base.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
import { test, expect } from "@playwright/test";
import type { BinaryLike } from "node:crypto";
import { createHash } from "node:crypto";

const OG_MD5 = "2f7b724d62d8c7739076da211aa62e7b";

export function validateMd5(data: Buffer, expectedHash: string) {
return (
createHash("md5")
.update(data as BinaryLike)
.digest("hex") === expectedHash
);
}

test("the application's noop index page is visible and it allows navigating to the hello-world api route", async ({
page,
Expand Down Expand Up @@ -42,3 +54,10 @@ test("returns correct information about the request from a route handler", async
const expectedURL = expect.stringMatching(/https?:\/\/localhost:(?!3000)\d+\/api\/request/);
await expect(res.json()).resolves.toEqual({ nextUrl: expectedURL, url: expectedURL });
});

test("generates an og image successfully", async ({ page }) => {
const res = await page.request.get("/og");
expect(res.status()).toEqual(200);
expect(res.headers()["content-type"]).toEqual("image/png");
james-elicx marked this conversation as resolved.
Show resolved Hide resolved
expect(validateMd5(await res.body(), OG_MD5)).toEqual(true);
});
5 changes: 3 additions & 2 deletions packages/cloudflare/src/cli/build/bundle-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ export async function bundleServer(buildOpts: BuildOptions): Promise<void> {
console.log(`\x1b[35m⚙️ Bundling the OpenNext server...\n\x1b[0m`);

patches.patchWranglerDeps(buildOpts);
patches.updateWebpackChunksFile(buildOpts);
await patches.updateWebpackChunksFile(buildOpts);
await patches.patchVercelOgLibrary(buildOpts);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to be able to move the changes to updateWorkerBundledCode()
It would probably require adding externals to the ESBuild config.
With that we could parse() (ast-grep) the code once and apply multiple edits.

(This again could be handled in a follow-up PR)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can do it there as this is patching a specific route's file whereas that function is patching handler.mjs, and the code for each of the routes doesn't appear to be bundled into the handler.mjs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code for each of the routes doesn't appear to be bundled into the handler.mjs

yet :)

As I mentioned, we should move patches that add dependencies (require) from after the bundle to before/during the bundling.

I do have an ast-grep patch solving the route issue. I need to rebase and submit the PR.


const outputPath = path.join(outputDir, "server-functions", "default");
const packagePath = getPackagePath(buildOpts);
Expand Down Expand Up @@ -176,7 +177,7 @@ async function updateWorkerBundledCode(workerOutputFile: string, buildOpts: Buil

const bundle = parse(Lang.TypeScript, patchedCode).root();

const edits = patchOptionalDependencies(bundle);
const { edits } = patchOptionalDependencies(bundle);

await writeFile(workerOutputFile, bundle.commitEdits(edits));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { type SgNode } from "@ast-grep/napi";

import { getRuleEdits } from "./util.js";
import { applyRule } from "./util.js";

/**
* Handle optional dependencies.
Expand Down Expand Up @@ -31,5 +31,5 @@ fix: |-
`;

export function patchOptionalDependencies(root: SgNode) {
return getRuleEdits(optionalDepRule, root);
return applyRule(optionalDepRule, root);
}
23 changes: 18 additions & 5 deletions packages/cloudflare/src/cli/build/patches/ast/util.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { readFileSync } from "node:fs";

import { type Edit, Lang, type NapiConfig, parse, type SgNode } from "@ast-grep/napi";
import yaml from "yaml";

Expand All @@ -8,7 +10,7 @@ import yaml from "yaml";
export type RuleConfig = NapiConfig & { fix?: string };

/**
* Returns the `Edit`s for an ast-grep rule in yaml format
* Returns the `Edit`s and `Match`es for an ast-grep rule in yaml format
*
* The rule must have a `fix` to rewrite the matched node.
*
Expand All @@ -17,9 +19,9 @@ export type RuleConfig = NapiConfig & { fix?: string };
* @param rule The rule. Either a yaml string or an instance of `RuleConfig`
* @param root The root node
* @param once only apply once
* @returns A list of edits.
* @returns A list of edits and a list of matches.
*/
export function getRuleEdits(rule: string | RuleConfig, root: SgNode, { once = false } = {}) {
export function applyRule(rule: string | RuleConfig, root: SgNode, { once = false } = {}) {
const ruleConfig: RuleConfig = typeof rule === "string" ? yaml.parse(rule) : rule;
if (ruleConfig.transform) {
throw new Error("transform is not supported");
Expand Down Expand Up @@ -50,7 +52,18 @@ export function getRuleEdits(rule: string | RuleConfig, root: SgNode, { once = f
);
});

return edits;
return { edits, matches };
}

/**
* Parse a file and obtain its root.
*
* @param path The file path
* @param lang The language to parse. Defaults to TypeScript.
* @returns The root for the file.
*/
export function parseFile(path: string, lang = Lang.TypeScript) {
return parse(lang, readFileSync(path, { encoding: "utf-8" })).root();
}

/**
Expand All @@ -71,6 +84,6 @@ export function patchCode(
{ lang = Lang.TypeScript, once = false } = {}
): string {
const node = parse(lang, code).root();
const edits = getRuleEdits(rule, node, { once });
const { edits } = applyRule(rule, node, { once });
return node.commitEdits(edits);
}
26 changes: 26 additions & 0 deletions packages/cloudflare/src/cli/build/patches/ast/vercel-og.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { describe, expect, it } from "vitest";

import { patchCode } from "./util";
import { vercelOgFallbackFontRule, vercelOgImportRule } from "./vercel-og";

describe("vercelOgImportRule", () => {
it("should rewrite a node import to an edge import", () => {
const code = `e.exports=import("next/dist/compiled/@vercel/og/index.node.js")`;
expect(patchCode(code, vercelOgImportRule)).toMatchInlineSnapshot(
`"e.exports=import("next/dist/compiled/@vercel/og/index.edge.js")"`
);
});
});

describe("vercelOgFallbackFontRule", () => {
it("should replace a fetch call for a font with an import", () => {
const code = `var fallbackFont = fetch(new URL("./noto-sans-v27-latin-regular.ttf", import.meta.url)).then((res) => res.arrayBuffer());`;
expect(patchCode(code, vercelOgFallbackFontRule)).toMatchInlineSnapshot(`
"async function getFallbackFont() {
return (await import("./noto-sans-v27-latin-regular.ttf.bin")).default
}

var fallbackFont = getFallbackFont()"
`);
});
});
63 changes: 63 additions & 0 deletions packages/cloudflare/src/cli/build/patches/ast/vercel-og.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { SgNode } from "@ast-grep/napi";

import { applyRule } from "./util.js";

export const vercelOgImportRule = `
rule:
pattern: $NODE
kind: string
regex: "next/dist/compiled/@vercel/og/index\\\\.node\\\\.js"
inside:
kind: arguments
inside:
kind: call_expression
stopBy: end
has:
field: function
regex: "import"

fix: |-
"next/dist/compiled/@vercel/og/index.edge.js"
`;

/**
* Patches Node.js imports for the library to be Edge imports.
*
* @param root Root node.
* @returns Results of applying the rule.
*/
export function patchVercelOgImport(root: SgNode) {
return applyRule(vercelOgImportRule, root);
}

export const vercelOgFallbackFontRule = `
rule:
kind: variable_declaration
all:
- has:
kind: variable_declarator
has:
kind: identifier
regex: ^fallbackFont$
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe it would be safer to detect the fetch(font_name) in case the variable name changes - but let's merge this, we can refine later if needed.

- has:
kind: call_expression
pattern: fetch(new URL("$PATH", $$$REST))
stopBy: end

fix: |-
async function getFallbackFont() {
return (await import("$PATH.bin")).default
james-elicx marked this conversation as resolved.
Show resolved Hide resolved
james-elicx marked this conversation as resolved.
Show resolved Hide resolved
}

var fallbackFont = getFallbackFont()
`;

/**
* Patches the default font fetching to use a .bin import.
*
* @param root Root node.
* @returns Results of applying the rule.
*/
export function patchVercelOgFallbackFont(root: SgNode) {
return applyRule(vercelOgFallbackFontRule, root);
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export * from "./copy-package-cli-files.js";
export * from "./patch-cache.js";
export * from "./patch-require.js";
export * from "./patch-vercel-og-library.js";
export * from "./update-webpack-chunks-file/index.js";
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { mkdirSync, readdirSync, readFileSync, writeFileSync } from "node:fs";
import path from "node:path";

import { BuildOptions } from "@opennextjs/aws/build/helper.js";
import mockFs from "mock-fs";
import { afterAll, beforeAll, describe, expect, it } from "vitest";

import { patchVercelOgLibrary } from "./patch-vercel-og-library";

const nodeModulesVercelOgDir = "node_modules/.pnpm/[email protected]/node_modules/next/dist/compiled/@vercel/og";
const nextServerOgNftPath = "examples/api/.next/server/app/og/route.js.nft.json";
const openNextFunctionDir = "examples/api/.open-next/server-functions/default/examples/api";
const openNextOgRoutePath = path.join(openNextFunctionDir, ".next/server/app/og/route.js");
const openNextVercelOgDir = path.join(openNextFunctionDir, "node_modules/next/dist/compiled/@vercel/og");

const buildOpts = {
appBuildOutputPath: "examples/api",
monorepoRoot: "",
outputDir: "examples/api/.open-next",
} as BuildOptions;

describe("patchVercelOgLibrary", () => {
beforeAll(() => {
mockFs();

mkdirSync(nodeModulesVercelOgDir, { recursive: true });
mkdirSync(path.dirname(nextServerOgNftPath), { recursive: true });
mkdirSync(path.dirname(openNextOgRoutePath), { recursive: true });
mkdirSync(openNextVercelOgDir, { recursive: true });

writeFileSync(
nextServerOgNftPath,
JSON.stringify({ version: 1, files: [`../../../../../../${nodeModulesVercelOgDir}/index.node.js`] })
);
writeFileSync(
path.join(nodeModulesVercelOgDir, "index.edge.js"),
`var fallbackFont = fetch(new URL("./noto-sans-v27-latin-regular.ttf", import.meta.url)).then((res) => res.arrayBuffer());`
);
writeFileSync(openNextOgRoutePath, `e.exports=import("next/dist/compiled/@vercel/og/index.node.js")`);
writeFileSync(path.join(openNextVercelOgDir, "index.node.js"), "");
writeFileSync(path.join(openNextVercelOgDir, "noto-sans-v27-latin-regular.ttf"), "");
});

afterAll(() => mockFs.restore());

it("should patch the open-next files correctly", () => {
patchVercelOgLibrary(buildOpts);

expect(readdirSync(openNextVercelOgDir)).toMatchInlineSnapshot(`
[
"index.edge.js",
"index.node.js",
"noto-sans-v27-latin-regular.ttf.bin",
]
`);

expect(readFileSync(path.join(openNextVercelOgDir, "index.edge.js"), { encoding: "utf-8" }))
.toMatchInlineSnapshot(`
"async function getFallbackFont() {
return (await import("./noto-sans-v27-latin-regular.ttf.bin")).default
}

var fallbackFont = getFallbackFont()"
`);

expect(readFileSync(openNextOgRoutePath, { encoding: "utf-8" })).toMatchInlineSnapshot(
`"e.exports=import("next/dist/compiled/@vercel/og/index.edge.js")"`
);
});
});
Loading
Loading