feat(font): replace generated google font catalog with virtual import rewrites#656
Conversation
Reworks the font-catalog removal from #262 using per-import virtual modules so named imports keep normal ESM semantics, aliased/default calls still self-host in build mode, and dependency imports are transformed too. Credit to @yunus25jmi1 for the original PR and direction in #262.
commit: |
|
|
/bigbonk review please |
There was a problem hiding this comment.
Good change. Replacing the 2,251-line generated runtime catalog with per-import virtual modules is a clear win — smaller package, same ESM semantics, no TDZ regressions from the const-rewrite approach in #262.
What I verified
-
Virtual module lifecycle:
resolveId→load→generateGoogleFontsVirtualModulecorrectly handles the\0-prefix, root-prefix, and query-string patterns. The generated module re-exportsdefault/utilities from the real shim and creates per-fontcreateFontLoaderbindings. -
Import clause parsing:
parseGoogleFontImportClausehandles the main patterns correctly — named{ Inter, Roboto }, default, namespace* as fonts, mixeddefault, { Inter }, type-only. The"type "prefix detection for type-only imports is straightforward. -
Build-mode self-hosting:
fontLocalsmaps local names (including aliases likeinterfromInter as inter) back to the original font name for thenamedCallRescan.proxyObjectLocalstracks default/namespace locals formemberCallRe. TheoverwrittenRangesguard prevents MagicString double-overwrites. -
Proxy underscore fix in
font-google-base.ts: Adding.replace(/_/g, " ")before the PascalCase split meansRoboto_Mono→"Roboto Mono"works correctly. Previously only PascalCase splitting was applied, so underscored names wouldn't resolve to the right family. This is a real bug fix. -
Script changes:
generate-google-fonts.jsnow only generates the.d.ts(for IDE autocomplete) and the test fixture JSON, dropping the.tsruntime catalog. Clean separation. -
Filter broadening: Removing
exclude: /node_modules/from the transform filter means dependencies that re-export fromnext/font/googleget rewritten too. This is correct — third-party libraries shouldn't need to know about vinext internals.
Minor observations (non-blocking)
-
[^}]*innamedCallRe/memberCallRe: These regexes won't match options objects containing nested braces (e.g.,Inter({ weight: '400', axes: { wght: 400 } })). This was already the case before this PR andparseStaticObjectLiteralwould reject nested objects for self-hosting anyway, so no regression, but worth noting for future work if someone reports nested font options not self-hosting. -
Utility-only imports pass through untransformed:
import { createFontLoader } from 'next/font/google'with no font imports —fontImports.length === 0so neither virtual module nor namespace branch fires. The import resolves via the existing shim alias, which is correct. But it means the transform returnsnullfor this case (no changes). Just want to confirm this is intentional — I believe it is since the shim resolver already handlesnext/font/google→font-google.ts. -
URL encoding mismatch between plugin and runtime: The build plugin constructs font URLs with
family.replace(/\s+/g, "+")then passes throughURLSearchParams.set(), producingRoboto%2BMono(double-encoded). The runtimebuildGoogleFontsUrlpasses raw"Roboto Mono"toURLSearchParams.set(), producingRoboto+Mono. These are semantically equivalent to Google's API (both work), and they don't share a cache, so no functional issue. But if you ever unify the URL construction, this divergence would cause cache misses.
CI is green across all checks. LGTM.
|
I've approved PR #656 after a thorough review. Here's the summary: Verdict: Approved The PR cleanly replaces the 2,251-line generated runtime font catalog ( Key things I verified:
I noted three non-blocking observations: the |
Summary
Closes #200.
This reworks the core idea from #262 by @yunus25jmi1, but switches the implementation to per-import virtual modules instead of local
constrewrites.That keeps the maintenance win of deleting the generated
font-google.generated.tscatalog, while preserving normal ESM semantics for named imports and broadening compatibility for real-world call sites.What Changed
packages/vinext/src/shims/font-google.generated.tsnext/font/googleimports and re-exports to virtual modules that export only the requested fontscreateFontLoaderas a public shim export for direct runtime use and testsfonts.Roboto_Mono(...)next/font/google, not just app source files.d.tsfor IDE autocomplete and typesWhy This Shape
The earlier approach in #262 had a good goal, but converting imports into local
constdeclarations changed module evaluation semantics and dropped compatibility for some import patterns.Using virtual modules avoids the TDZ/hoisting regression for named imports while still removing the giant generated runtime catalog.
Testing
vp test run tests/font-google.test.tsvp test run tests/shims.test.ts -t "next/font/google shim"vp check packages/vinext/src/index.ts packages/vinext/src/shims/font-google.ts packages/vinext/src/shims/font-google-base.ts packages/vinext/src/shims/next-shims-font-google.generated.d.ts scripts/generate-google-fonts.js tests/font-google.test.ts tests/shims.test.tsCredit
Credit to @yunus25jmi1 for the original PR and direction in #262: #262