Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 40 additions & 13 deletions packages/vinext/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,12 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
// Shim alias map — populated in config(), used by resolveId() for .js variants
let nextShimMap: Record<string, string> = {};

// Build-only cache for og-inline-fetch-assets to avoid repeated file reads
// during a single production build. Dev mode skips the cache so asset edits
// are picked up without restarting the Vite server.
const _ogInlineCache = new Map<string, string>();
Copy link
Contributor

Choose a reason for hiding this comment

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

This cache lives for the entire lifetime of the plugin instance, which in dev mode means the entire dev server session. If a developer changes a font file on disk while the dev server is running, the stale cached base64 will be served until restart.

Two options:

  1. Clear the cache on buildStart (making it truly per-build as the comment says)
  2. Skip caching when command === 'serve' (dev mode), and only cache during builds

Option 1 is simpler and still captures the main win (fonts are read once per build, not once per module):

Suggested change
const _ogInlineCache = new Map<string, string>();
// Cache for og-inline-fetch-assets plugin to avoid repeated file reads within a build.
// Cleared on each buildStart so dev-server HMR picks up changed assets.
const _ogInlineCache = new Map<string, string>();

Then add a buildStart hook to the vinext:og-inline-fetch-assets plugin object:

buildStart() {
  _ogInlineCache.clear();
},

let _ogInlineIsBuild = false;

/**
* Generate the virtual SSR server entry module.
* This is the entry point for `vite build --ssr`.
Expand Down Expand Up @@ -2835,12 +2841,21 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
{
name: "vinext:og-inline-fetch-assets",
enforce: "pre",
transform(code, id) {
configResolved(config) {
_ogInlineIsBuild = config.command === "build";
},
buildStart() {
if (_ogInlineIsBuild) {
_ogInlineCache.clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: In Vite's multi-environment build pipeline (RSC → SSR → Client), buildStart is called once per environment. This means the cache gets cleared between environments, so the same font file will be read from disk once per environment rather than once total for the entire build.

This is fine in practice — the main win is avoiding repeated reads within a single environment's module graph (e.g., 10 OG route modules all importing the same font). Just noting it for awareness since the comment says "per-build" which could be read as "once for the entire build".

}
},
async transform(code, id) {
// Quick bail-out: only process modules that use new URL(..., import.meta.url)
if (!code.includes("import.meta.url")) {
return null;
}

const useCache = _ogInlineIsBuild;
const moduleDir = path.dirname(id);
let newCode = code;
let didReplace = false;
Expand All @@ -2856,12 +2871,18 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
const relPath = match[2]; // e.g. "./noto-sans-v27-latin-regular.ttf"
const absPath = path.resolve(moduleDir, relPath);

let fileBase64: string;
try {
fileBase64 = fs.readFileSync(absPath).toString("base64");
} catch {
// File not found on disk — skip (may be a runtime-only asset)
continue;
let fileBase64 = useCache ? _ogInlineCache.get(absPath) : undefined;
if (fileBase64 === undefined) {
try {
const buf = await fs.promises.readFile(absPath);
fileBase64 = buf.toString("base64");
if (useCache) {
_ogInlineCache.set(absPath, fileBase64);
}
} catch {
// File not found on disk — skip (may be a runtime-only asset)
continue;
}
}

// Replace fetch(...).then(...) with an inline IIFE that returns Promise<ArrayBuffer>.
Expand Down Expand Up @@ -2892,12 +2913,18 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
const relPath = match[2]; // e.g. "./noto-sans-v27-latin-regular.ttf"
const absPath = path.resolve(moduleDir, relPath);

let fileBase64: string;
try {
fileBase64 = fs.readFileSync(absPath).toString("base64");
} catch {
// File not found on disk — skip
continue;
let fileBase64 = useCache ? _ogInlineCache.get(absPath) : undefined;
if (fileBase64 === undefined) {
try {
const buf = await fs.promises.readFile(absPath);
fileBase64 = buf.toString("base64");
if (useCache) {
_ogInlineCache.set(absPath, fileBase64);
}
} catch {
// File not found on disk — skip
continue;
}
}

// Replace readFileSync(...) with Buffer.from("<base64>", "base64").
Expand Down
168 changes: 168 additions & 0 deletions tests/og-inline.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
import { describe, it, expect, vi, beforeAll, afterAll, afterEach } from "vitest";
import vinext from "../packages/vinext/src/index.js";
import type { Plugin } from "vite";
import fs from "node:fs";
import fsp from "node:fs/promises";
import os from "node:os";
import path from "node:path";

// ── Helpers ───────────────────────────────────────────────────

/** Unwrap a Vite plugin hook that may use the object-with-filter format */
function unwrapHook(hook: any): Function {
return typeof hook === "function" ? hook : hook?.handler;
}

/**
* Create a fresh vinext:og-inline-fetch-assets plugin instance.
* Each call gets an independent cache so tests do not share state.
*/
function createOgInlinePlugin(command: "serve" | "build" = "serve"): Plugin {
const plugins = vinext() as Plugin[];
const plugin = plugins.find((p) => p.name === "vinext:og-inline-fetch-assets");
if (!plugin) throw new Error("vinext:og-inline-fetch-assets plugin not found");
const configResolved = unwrapHook(plugin.configResolved);
configResolved?.call(plugin, { command });
return plugin;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: createOgInlinePlugin calls configResolved but never calls buildStart. For the "build" command path, this means the buildStart hook — which clears the cache — is never exercised in tests.

This doesn't cause test failures today because each vinext() call creates a fresh cache, but it means there's no test validating that the buildStart cache-clearing logic actually works. Consider adding a small test that creates a plugin, does a transform (populating the cache), calls buildStart, then does the same transform again and asserts readFile is called twice (proving the cache was cleared).

}

// ── Test fixture setup ────────────────────────────────────────

let tmpDir: string;
const fontContent = Buffer.from("fake-font-data-for-testing");
const fontBase64 = fontContent.toString("base64");

beforeAll(async () => {
tmpDir = await fsp.mkdtemp(path.join(os.tmpdir(), "og-inline-test-"));
await fsp.writeFile(path.join(tmpDir, "noto-sans.ttf"), fontContent);
});

afterAll(async () => {
await fsp.rm(tmpDir, { recursive: true, force: true });
});

// ── Tests ─────────────────────────────────────────────────────

describe("vinext:og-inline-fetch-assets plugin", () => {
afterEach(() => vi.restoreAllMocks());

it("exists in the plugin array", () => {
const plugin = createOgInlinePlugin();
expect(plugin.name).toBe("vinext:og-inline-fetch-assets");
expect(plugin.enforce).toBe("pre");
});

// ── Guard clause ──────────────────────────────────────────

it("returns null when code has no import.meta.url", async () => {
const plugin = createOgInlinePlugin();
const transform = unwrapHook(plugin.transform);
const code = `import fs from 'node:fs';\nconst x = 1;`;
const result = await transform.call(plugin, code, "/app/og.tsx");
expect(result).toBeNull();
});

// ── Pattern 1: fetch ─────────────────────────────────────

it("transforms fetch(new URL(..., import.meta.url)).then(r => r.arrayBuffer())", async () => {
const plugin = createOgInlinePlugin();
const transform = unwrapHook(plugin.transform);
const code = `const data = fetch(new URL("./noto-sans.ttf", import.meta.url)).then((res) => res.arrayBuffer());`;
const moduleId = path.join(tmpDir, "og.tsx");

const result = await transform.call(plugin, code, moduleId);
expect(result).not.toBeNull();
expect(result.code).toContain(JSON.stringify(fontBase64));
expect(result.code).toContain("Promise.resolve(a.buffer)");
expect(result.code).not.toContain("fetch(");
});

// ── Pattern 2: readFileSync ──────────────────────────────

it("transforms fs.readFileSync(fileURLToPath(new URL(..., import.meta.url)))", async () => {
const plugin = createOgInlinePlugin();
const transform = unwrapHook(plugin.transform);
const code = `const buf = fs.readFileSync(fileURLToPath(new URL("./noto-sans.ttf", import.meta.url)));`;
const moduleId = path.join(tmpDir, "og.tsx");

const result = await transform.call(plugin, code, moduleId);
expect(result).not.toBeNull();
expect(result.code).toContain(`Buffer.from(${JSON.stringify(fontBase64)},"base64")`);
expect(result.code).not.toContain("readFileSync");
});

// ── File not found ───────────────────────────────────────

it("silently skips when the referenced file does not exist", async () => {
const plugin = createOgInlinePlugin();
const transform = unwrapHook(plugin.transform);
const code = `const data = fetch(new URL("./nonexistent.ttf", import.meta.url)).then((res) => res.arrayBuffer());`;
const moduleId = path.join(tmpDir, "og.tsx");

const result = await transform.call(plugin, code, moduleId);
// No file found → no replacement → returns null
expect(result).toBeNull();
});

// ── Async assertion ──────────────────────────────────────

it("returns a Promise (hook is async)", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test doesn't await the result, so it's an unhandled promise. Vitest won't flag this as a failure today, but the dangling promise can cause flaky behavior (e.g., it might resolve after the test ends and interfere with afterEach cleanup). Since this test just asserts the return type is a Promise, that's fine, but you should still clean it up:

Suggested change
it("returns a Promise (hook is async)", () => {
it("returns a Promise (hook is async)", () => {
const plugin = getOgInlinePlugin();
const transform = unwrapHook(plugin.transform);
const code = `const data = fetch(new URL("./noto-sans.ttf", import.meta.url)).then((res) => res.arrayBuffer());`;
const moduleId = path.join(tmpDir, "og.tsx");
const result = transform.call(plugin, code, moduleId);
expect(result).toBeInstanceOf(Promise);
return result; // Return the promise so Vitest awaits it

const plugin = createOgInlinePlugin();
const transform = unwrapHook(plugin.transform);
const code = `const data = fetch(new URL("./noto-sans.ttf", import.meta.url)).then((res) => res.arrayBuffer());`;
const moduleId = path.join(tmpDir, "og.tsx");

const result = transform.call(plugin, code, moduleId);
expect(result).toBeInstanceOf(Promise);
return result;
});

// ── Build cache hit ──────────────────────────────────────

it("reads the file only once for repeated build transforms (cache hit)", async () => {
const readFileSpy = vi.spyOn(fs.promises, "readFile");

const plugin = createOgInlinePlugin("build");
const transform = unwrapHook(plugin.transform);
const code = `const buf = fs.readFileSync(fileURLToPath(new URL("./noto-sans.ttf", import.meta.url)));`;
const moduleId = path.join(tmpDir, "og.tsx");

// First call — should read from disk
await transform.call(plugin, code, moduleId);

// Second call — should use cache
await transform.call(plugin, code, moduleId);

// Exactly once: first call reads from disk, second call hits the build cache.
const calls = readFileSpy.mock.calls.filter(
(call) => call[0] === path.join(tmpDir, "noto-sans.ttf"),
);
expect(calls.length).toBe(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment says "at most once" but the assertion is toBe(1), which is correct — it should be exactly 1 (first call is a cache miss that reads from disk, second call hits the cache). The comment should match:

Suggested change
expect(calls.length).toBe(1);
expect(calls.length).toBe(1); // Exactly once: first call reads from disk, second hits cache

Also worth noting: this test works because getOgInlinePlugin() creates a fresh vinext() instance (with an empty cache). If the test ever shares a plugin instance with earlier tests, the first call here might also be a cache hit. Consider adding a comment to getOgInlinePlugin() documenting that each call creates an independent plugin instance with its own cache.

});

// ── Dev mode stays fresh ─────────────────────────────────

it("does not cache asset contents across serve transforms", async () => {
const devFontPath = path.join(tmpDir, "dev-font.ttf");
const initialFontBase64 = Buffer.from("dev-font-v1").toString("base64");
const updatedFontBase64 = Buffer.from("dev-font-v2").toString("base64");
await fsp.writeFile(devFontPath, Buffer.from("dev-font-v1"));

const plugin = createOgInlinePlugin("serve");
const transform = unwrapHook(plugin.transform);
const code = `const buf = fs.readFileSync(fileURLToPath(new URL("./dev-font.ttf", import.meta.url)));`;
const moduleId = path.join(tmpDir, "og.tsx");

const firstResult = await transform.call(plugin, code, moduleId);
expect(firstResult.code).toContain(
`Buffer.from(${JSON.stringify(initialFontBase64)},"base64")`,
);

await fsp.writeFile(devFontPath, Buffer.from("dev-font-v2"));

const secondResult = await transform.call(plugin, code, moduleId);
expect(secondResult.code).toContain(
`Buffer.from(${JSON.stringify(updatedFontBase64)},"base64")`,
);
});
});
Loading