starred/vinext

Fork 0

mirror of https://github.com/cloudflare/vinext.git synced 2026-05-09 08:25:34 +02:00

[PR #920] [MERGED] fix(fonts): google fonts building perfectly (it was just a regex 😭) #948

New issue

Closed

opened 2026-05-06 13:11:01 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:11:01 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/920
Author: @MrIago
Created: 4/27/2026
Status: ✅ Merged
Merged: 4/27/2026
Merged by: @james-elicx

Base: main ← Head: fix/font-google-working-fine

📝 Commits (3)

3096114 fix(fonts): google fonts building perfectly (it was just a regex 😭)
82d80fc fix(fonts): allow newlines inside bracket clauses
dff1643 fix(fonts): apply formatter to test file

📊 Changes

2 files changed (+68 additions, -1 deletions)

View changed files

📝 packages/vinext/src/plugins/fonts.ts (+11 -1)
📝 tests/font-google.test.ts (+57 -0)

📄 Description

"I" finally fixed google fonts 😭🙏🙌🙌 (it was just a regex!)

What this is, in one breath

If you copy the exact sample from the official Next.js font docs into a vinext project, the build dies.

Not because your code is wrong. Not because vinext doesn't support it. Because of how your code formatter ends each line.

Wait, what?

Modern formatters — Prettier, Biome, oxfmt — all ship without trailing semicolons by default. That's how import { Inter } from 'next/font/google' shows up in 90%+ of new Next.js projects today.

In vinext, the absence of those semicolons made the font plugin's regex misfire. The rewrite was silently skipped. Rolldown then complained about a missing export, and the build was dead.

   With trailing ";"            Default style (no ";")
   ─────────────────            ──────────────────────
        ✅ builds                    ❌ build dies
                                       MISSING_EXPORT "Inter"

Same code. Same import. Different format. One worked, one didn't.

Why this matters

This wasn't a niche edge case. It was the canonical path:

The exact line from the Next.js font docs.
The output of every modern formatter on its default settings.
The first thing a new vinext user types after vinext init.

Every one of them hit a wall.

How small the actual fix was

Two characters. \n added to a single regex (twice — once for imports, once for exports).

The regex was written to stop at ;. Now it also stops at line breaks. That's it. The rest of the font pipeline was already correct.

Proof it works

A real Cloudflare Workers project that wouldn't deploy now deploys cleanly:

BEFORE   →   vinext deploy   →   ❌  MISSING_EXPORT "Inter"
AFTER    →   vinext deploy   →   ✅  Deployed and healthy
                                   ✅  Inter <link> emitted
                                   ✅  --font-inter applied to CSS

Tested with Inter, Geist, and Architects_Daughter — swapping the font name is now a no-op, exactly as the docs promise.

For reviewers (the technical bit)

Click to expand — root cause walk-through

The plugin scans source files for import … from 'next/font/google' with a regex. The clause portion was [^;]+? — "any character that isn't a ;, lazy".

When the previous line in the file lacks a ;, that lazy match still has to eventually hit next/font/google. So it walks across the \n and consumes the previous import line on its way down. The captured clause looks like:

type { Metadata } from 'next'
import { Inter }

That mangled clause goes into parseGoogleFontImportClause(), returns no fontImports, and the rewrite branch is skipped. hasChanges stays false, the transform returns null, and Inter survives untouched into rolldown — which then can't find a named export on the Proxy-default shim.

The fix:

- /^[ \t]*import\s+([^;]+?)\s+from\s*(["'])next\/font\/google\2\s*;?/gm
+ /^[ \t]*import\s+([^;\n]+?)\s+from\s*(["'])next\/font\/google\2\s*;?/gm

Same change on exportRe for parity ({ … } clause).

A full ESM parser would have been overkill — the transform's filter already opts in only when code: "next/font/google" is present, and the regex sweep is what keeps the hot path Rust-side. The minimal regex change preserves that contract.

Why your existing tests didn't catch it

Every fixture in tests/font-google.test.ts writes imports with trailing ;. Reasonable style, just not the modern default.

Added a regression test that mirrors a real Prettier-default 3-line layout. Asserts (a) the font import gets rewritten, (b) the unrelated preceding import type line is left untouched.

Validation

✅ pnpm test run tests/font-google.test.ts — 100 passed (1 added)
✅ pnpm run check — format, lint, typecheck green
✅ End-to-end deploy on a Cloudflare Workers project (vinext@0.0.44, @vitejs/plugin-rsc@0.5.24, @cloudflare/vite-plugin@1.32.3)

Refs

Next.js's own next-font-loader parser is line-bounded by construction because SWC emits one font import per call into the resourceQuery: https://github.com/vercel/next.js/blob/canary/packages/next/src/build/webpack/loaders/next-font-loader/index.ts#L25-L31
AGENTS.md asks contributors to bring "here's what the agent found" — done above.

🧠 Thought by a Human (@MrIago) · 🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/920 **Author:** [@MrIago](https://github.com/MrIago) **Created:** 4/27/2026 **Status:** ✅ Merged **Merged:** 4/27/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `fix/font-google-working-fine` --- ### 📝 Commits (3) - [`3096114`](https://github.com/cloudflare/vinext/commit/30961149c36d6a311533456204bfe0e5dbb8bd8a) fix(fonts): google fonts building perfectly (it was just a regex 😭) - [`82d80fc`](https://github.com/cloudflare/vinext/commit/82d80fc088f14216195417759e6e8b9067d9111e) fix(fonts): allow newlines inside bracket clauses - [`dff1643`](https://github.com/cloudflare/vinext/commit/dff1643cb0cd1a219448d8bfabc20516931c6326) fix(fonts): apply formatter to test file ### 📊 Changes **2 files changed** (+68 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/plugins/fonts.ts` (+11 -1) 📝 `tests/font-google.test.ts` (+57 -0) </details> ### 📄 Description > ## "I" finally fixed google fonts 😭🙏🙌🙌 (it was just a regex!) > > ![Me writing great code](https://media.licdn.com/dms/image/v2/D5622AQHGOah3ssafvw/feedshare-shrink_1280/B56ZwZHPtQIwAg-/0/1769947856126?e=2147483647&v=beta&t=2ww9oTeaergW2NJbTP_pzGK82tVK2DgwRsCDTwRJYf0) ## What this is, in one breath If you copy the **exact** sample from the official Next.js font docs into a vinext project, the build dies. Not because your code is wrong. Not because vinext doesn't support it. Because of how your code formatter ends each line. ## Wait, what? Modern formatters — **Prettier**, **Biome**, **oxfmt** — all ship without trailing semicolons by default. That's how `import { Inter } from 'next/font/google'` shows up in 90%+ of new Next.js projects today. In vinext, the *absence* of those semicolons made the font plugin's regex misfire. The rewrite was silently skipped. Rolldown then complained about a missing export, and the build was dead. ``` With trailing ";" Default style (no ";") ───────────────── ────────────────────── ✅ builds ❌ build dies MISSING_EXPORT "Inter" ``` Same code. Same import. Different format. One worked, one didn't. ## Why this matters This wasn't a niche edge case. It was the **canonical** path: - The exact line from the [Next.js font docs](https://nextjs.org/docs/app/api-reference/components/font#google-fonts). - The output of every modern formatter on its default settings. - The first thing a new vinext user types after `vinext init`. Every one of them hit a wall. ## How small the actual fix was Two characters. `\n` added to a single regex (twice — once for imports, once for exports). The regex was written to stop at `;`. Now it also stops at line breaks. That's it. The rest of the font pipeline was already correct. ## Proof it works A real Cloudflare Workers project that wouldn't deploy now deploys cleanly: ``` BEFORE → vinext deploy → ❌ MISSING_EXPORT "Inter" AFTER → vinext deploy → ✅ Deployed and healthy ✅ Inter <link> emitted ✅ --font-inter applied to CSS ``` Tested with Inter, Geist, and Architects_Daughter — swapping the font name is now a no-op, exactly as the docs promise. --- ## For reviewers (the technical bit) <details> <summary>Click to expand — root cause walk-through</summary> The plugin scans source files for `import … from 'next/font/google'` with a regex. The clause portion was `[^;]+?` — "any character that isn't a `;`, lazy". When the previous line in the file lacks a `;`, that lazy match still has to eventually hit `next/font/google`. So it walks **across the `\n`** and consumes the previous import line on its way down. The captured clause looks like: ``` type { Metadata } from 'next' import { Inter } ``` That mangled clause goes into `parseGoogleFontImportClause()`, returns no `fontImports`, and the rewrite branch is skipped. `hasChanges` stays `false`, the transform returns `null`, and `Inter` survives untouched into rolldown — which then can't find a named export on the Proxy-default shim. The fix: ```diff - /^[ \t]*import\s+([^;]+?)\s+from\s*(["'])next\/font\/google\2\s*;?/gm + /^[ \t]*import\s+([^;\n]+?)\s+from\s*(["'])next\/font\/google\2\s*;?/gm ``` Same change on `exportRe` for parity (`{ … }` clause). A full ESM parser would have been overkill — the transform's `filter` already opts in only when `code: "next/font/google"` is present, and the regex sweep is what keeps the hot path Rust-side. The minimal regex change preserves that contract. </details> <details> <summary>Why your existing tests didn't catch it</summary> Every fixture in `tests/font-google.test.ts` writes imports with trailing `;`. Reasonable style, just not the modern default. Added a regression test that mirrors a real Prettier-default 3-line layout. Asserts (a) the font import gets rewritten, (b) the unrelated preceding `import type` line is left untouched. </details> <details> <summary>Validation</summary> - ✅ `pnpm test run tests/font-google.test.ts` — 100 passed (1 added) - ✅ `pnpm run check` — format, lint, typecheck green - ✅ End-to-end deploy on a Cloudflare Workers project (`vinext@0.0.44`, `@vitejs/plugin-rsc@0.5.24`, `@cloudflare/vite-plugin@1.32.3`) </details> <details> <summary>Refs</summary> - Next.js's own `next-font-loader` parser is line-bounded by construction because SWC emits one font import per call into the resourceQuery: https://github.com/vercel/next.js/blob/canary/packages/next/src/build/webpack/loaders/next-font-loader/index.ts#L25-L31 - AGENTS.md asks contributors to bring "here's what the agent found" — done above. </details> --- 🧠 Thought by a Human ([@MrIago](https://github.com/MrIago)) · 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>