From 47c0330610a6c0f092a289ca28f165ad9f3c227c Mon Sep 17 00:00:00 2001 From: shadcn Date: Fri, 27 Feb 2026 18:35:12 +0400 Subject: [PATCH] fix: issue with font updates --- packages/shadcn/src/templates/next.ts | 12 +- packages/shadcn/src/utils/preset.ts | 11 +- .../src/utils/updaters/update-fonts.test.ts | 260 ++++++++++++++++-- .../shadcn/src/utils/updaters/update-fonts.ts | 128 +++++---- 4 files changed, 331 insertions(+), 80 deletions(-) diff --git a/packages/shadcn/src/templates/next.ts b/packages/shadcn/src/templates/next.ts index 947dbc057f..181c640b89 100644 --- a/packages/shadcn/src/templates/next.ts +++ b/packages/shadcn/src/templates/next.ts @@ -120,14 +120,10 @@ export default function Page() { themeCssVars[font.font.variable] = `var(${font.font.variable})` } - await updateCssVars( - { theme: themeCssVars }, - resolvedPackagesUiConfig, - { - silent: options.silent, - overwriteCssVars: false, - } - ) + await updateCssVars({ theme: themeCssVars }, resolvedPackagesUiConfig, { + silent: options.silent, + overwriteCssVars: false, + }) // Update layout.tsx in apps/web with the font import and className. await updateFonts(tree.fonts, resolvedAppsWebConfig, { diff --git a/packages/shadcn/src/utils/preset.ts b/packages/shadcn/src/utils/preset.ts index 29ade79b42..cceb9ee13e 100644 --- a/packages/shadcn/src/utils/preset.ts +++ b/packages/shadcn/src/utils/preset.ts @@ -12,7 +12,16 @@ // Value arrays — order matters for backward compat. Never reorder, only append. export const PRESET_BASES = ["radix", "base"] as const export const PRESET_STYLES = ["nova", "vega", "maia", "lyra", "mira"] as const -export const PRESET_BASE_COLORS = ["neutral", "stone", "zinc", "gray", "mauve", "olive", "mist", "taupe"] as const +export const PRESET_BASE_COLORS = [ + "neutral", + "stone", + "zinc", + "gray", + "mauve", + "olive", + "mist", + "taupe", +] as const export const PRESET_THEMES = [ "neutral", "stone", diff --git a/packages/shadcn/src/utils/updaters/update-fonts.test.ts b/packages/shadcn/src/utils/updaters/update-fonts.test.ts index eb048f5721..2a27db89be 100644 --- a/packages/shadcn/src/utils/updaters/update-fonts.test.ts +++ b/packages/shadcn/src/utils/updaters/update-fonts.test.ts @@ -1,6 +1,6 @@ -import { describe, expect, it } from "vitest" +import { describe, expect, it, vi } from "vitest" -import { transformLayoutFonts } from "./update-fonts" +import { massageTreeForFonts, transformLayoutFonts } from "./update-fonts" const mockConfig = { style: "new-york", @@ -74,6 +74,7 @@ export default function RootLayout({ import type { Metadata } from "next" import "./globals.css" import { Inter } from "next/font/google"; + import { cn } from "@/lib/utils"; const inter = Inter({subsets:['latin'],variable:'--font-sans'}); @@ -87,7 +88,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -158,7 +159,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -321,6 +322,7 @@ export default function RootLayout({ " import type { Metadata } from "next" import { Inter } from "next/font/google"; + import { cn } from "@/lib/utils"; const inter = Inter({subsets:['latin'],variable:'--font-sans'}); @@ -331,7 +333,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -377,6 +379,7 @@ export default function RootLayout({ expect(result).toMatchInlineSnapshot(` " import { Roboto, Inter } from "next/font/google" + import { cn } from "@/lib/utils"; const inter = Inter({subsets:['latin'],variable:'--font-sans'}) @@ -386,7 +389,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -443,7 +446,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -485,6 +488,7 @@ export default function RootLayout({ expect(result).toMatchInlineSnapshot(` "import { Inter } from "next/font/google"; + import { cn } from "@/lib/utils"; const inter = Inter({subsets:['latin'],weight:['400','500','600','700'],variable:'--font-sans'}); @@ -495,7 +499,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -504,7 +508,7 @@ export default function RootLayout({ `) }) - it("should skip font entirely if already imported", async () => { + it("should add already-imported font to html className", async () => { const input = ` import { Inter } from "next/font/google" @@ -538,10 +542,11 @@ export default function RootLayout({ const result = await transformLayoutFonts(input, fonts, mockConfig) - // Font is already imported, so the layout should remain unchanged. + // Font is already imported but not on , so it should be added. expect(result).toMatchInlineSnapshot(` " import { Inter } from "next/font/google" + import { cn } from "@/lib/utils"; const inter = Inter({subsets:['latin'],variable:'--font-sans'}) @@ -551,7 +556,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -655,6 +660,7 @@ export default function RootLayout({ expect(result).toMatchInlineSnapshot(` " import { Roboto, Inter } from "next/font/google" + import { cn } from "@/lib/utils"; const inter = Inter({subsets:['latin'],variable:'--font-sans'}); @@ -666,7 +672,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -707,6 +713,7 @@ export default function RootLayout({ expect(result).toMatchInlineSnapshot(` "import { Inter } from "next/font/google"; + import { cn } from "@/lib/utils"; const inter = Inter({subsets:['latin'],variable:'--font-sans'}); @@ -717,7 +724,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -764,6 +771,7 @@ export default function RootLayout({ import { GeistSans } from "geist/font/sans" import { GeistMono } from "geist/font/mono" import { Inter } from "next/font/google"; + import { cn } from "@/lib/utils"; const inter = Inter({subsets:['latin'],variable:'--font-sans'}); @@ -774,7 +782,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -869,6 +877,7 @@ export default function RootLayout({ import type { Metadata } from "next" import "./globals.css" import { Lora } from "next/font/google"; + import { cn } from "@/lib/utils"; const lora = Lora({subsets:['latin'],variable:'--font-serif'}); @@ -882,7 +891,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -953,7 +962,7 @@ export default function RootLayout({ children: React.ReactNode }) { return ( - + {children} ) @@ -1010,4 +1019,223 @@ export default function RootLayout({ // All runs should produce the same result. expect(secondRun).toBe(firstRun) }) + + it("should be idempotent when font is already imported and on html", async () => { + // Simulates a layout where the font was already added by a previous preset. + const input = ` +import { Merriweather } from "next/font/google"; +import { cn } from "@/lib/utils"; + +const merriweather = Merriweather({subsets:['latin'],weight:['400','700'],variable:'--font-serif'}); + +export default function RootLayout({ + children, +}: { + children: React.ReactNode +}) { + return ( + + {children} + + ) +} +` + const fonts = [ + { + name: "font-merriweather", + type: "registry:font" as const, + font: { + family: "'Merriweather Variable', serif", + provider: "google" as const, + import: "Merriweather", + variable: "--font-serif", + subsets: ["latin"], + weight: ["400", "700"], + }, + }, + ] + + const firstRun = await transformLayoutFonts(input, fonts, mockConfig) + const secondRun = await transformLayoutFonts(firstRun, fonts, mockConfig) + + // Should remain unchanged across all runs. + expect(firstRun).toBe(input) + expect(secondRun).toBe(input) + }) + + it("should be idempotent when adding font to pre-existing layout with other fonts", async () => { + // Layout already has Inter, and we're adding Merriweather. + const input = ` +import { Inter } from "next/font/google"; +import { cn } from "@/lib/utils"; + +const inter = Inter({subsets:['latin'],variable:'--font-sans'}); + +export default function RootLayout({ + children, +}: { + children: React.ReactNode +}) { + return ( + + {children} + + ) +} +` + const fonts = [ + { + name: "font-inter", + type: "registry:font" as const, + font: { + family: "'Inter Variable', sans-serif", + provider: "google" as const, + import: "Inter", + variable: "--font-sans", + subsets: ["latin"], + }, + }, + { + name: "font-merriweather", + type: "registry:font" as const, + font: { + family: "'Merriweather Variable', serif", + provider: "google" as const, + import: "Merriweather", + variable: "--font-serif", + subsets: ["latin"], + weight: ["400", "700"], + }, + }, + ] + + const firstRun = await transformLayoutFonts(input, fonts, mockConfig) + const secondRun = await transformLayoutFonts(firstRun, fonts, mockConfig) + + // Second run should be identical to first. + expect(secondRun).toBe(firstRun) + // Inter should still be there, Merriweather should be added. + expect(firstRun).toContain("font-sans") + expect(firstRun).toContain("font-serif") + expect(firstRun).toContain("inter.variable") + expect(firstRun).toContain("merriweather.variable") + }) +}) + +vi.mock("@/src/utils/get-project-info", () => ({ + getProjectInfo: vi.fn().mockResolvedValue({ + framework: { name: "vite" }, + isTsx: true, + isSrcDir: false, + }), +})) + +describe("massageTreeForFonts", () => { + it("should add font @apply to body when no existing css", async () => { + const tree = { + fonts: [ + { + name: "font-inter", + type: "registry:font" as const, + font: { + family: "'Inter Variable', sans-serif", + provider: "google" as const, + import: "Inter", + variable: "--font-sans", + subsets: ["latin"], + }, + }, + ], + } as any + + const result = await massageTreeForFonts(tree, { + resolvedPaths: { cwd: "/test" }, + } as any) + + expect(result.css["@layer base"].body).toEqual({ + "@apply font-sans": {}, + }) + }) + + it("should preserve existing body css rules when adding font classes", async () => { + const tree = { + fonts: [ + { + name: "font-inter", + type: "registry:font" as const, + font: { + family: "'Inter Variable', sans-serif", + provider: "google" as const, + import: "Inter", + variable: "--font-sans", + subsets: ["latin"], + }, + }, + ], + cssVars: { + theme: {}, + }, + css: { + "@layer base": { + body: { + "@apply bg-background text-foreground": {}, + }, + }, + }, + } as any + + const result = await massageTreeForFonts(tree, { + resolvedPaths: { cwd: "/test" }, + } as any) + + expect(result.css["@layer base"].body).toEqual({ + "@apply bg-background text-foreground": {}, + "@apply font-sans": {}, + }) + }) + + it("should combine multiple font classes into a single @apply", async () => { + const tree = { + fonts: [ + { + name: "font-inter", + type: "registry:font" as const, + font: { + family: "'Inter Variable', sans-serif", + provider: "google" as const, + import: "Inter", + variable: "--font-sans", + subsets: ["latin"], + }, + }, + { + name: "font-lora", + type: "registry:font" as const, + font: { + family: "'Lora Variable', serif", + provider: "google" as const, + import: "Lora", + variable: "--font-serif", + subsets: ["latin"], + }, + }, + ], + css: { + "@layer base": { + body: { + "@apply bg-background text-foreground": {}, + }, + }, + }, + } as any + + const result = await massageTreeForFonts(tree, { + resolvedPaths: { cwd: "/test" }, + } as any) + + expect(result.css["@layer base"].body).toEqual({ + "@apply bg-background text-foreground": {}, + "@apply font-sans font-serif": {}, + }) + }) }) diff --git a/packages/shadcn/src/utils/updaters/update-fonts.ts b/packages/shadcn/src/utils/updaters/update-fonts.ts index d91a0ed3c5..40ed07e257 100644 --- a/packages/shadcn/src/utils/updaters/update-fonts.ts +++ b/packages/shadcn/src/utils/updaters/update-fonts.ts @@ -36,7 +36,8 @@ export async function massageTreeForFonts( for (const font of tree.fonts) { if (isNext) { - tree.cssVars.theme[font.font.variable] = `var(${font.font.variable})` + // Next.js sets the CSS variable via next/font on the element. + // The font utility class is added to className in updateHtmlClassName. } else { // Other frameworks will use fontsource for now. const fontName = font.name.replace("font-", "") @@ -56,9 +57,8 @@ export async function massageTreeForFonts( .join(" ") tree.css ??= {} tree.css["@layer base"] ??= {} - tree.css["@layer base"].body = { - [`@apply ${fontClasses}`]: {}, - } + tree.css["@layer base"].body ??= {} + tree.css["@layer base"].body[`@apply ${fontClasses}`] = {} } return tree @@ -164,8 +164,9 @@ export async function transformLayoutFonts( // Only process Google fonts for now. const googleFonts = fonts.filter((f) => f.font.provider === "google") - // Track which font variables we're adding. + // Track which font variables and utility classes we're adding. const fontVariableNames: string[] = [] + const fontUtilityClasses: string[] = [] // Process Google fonts. for (const font of googleFonts) { @@ -186,8 +187,15 @@ export async function transformLayoutFonts( const hasImport = namedImports.some((imp) => imp.getName() === importName) if (hasImport) { - // Font is already imported - skip this font entirely. - // Assume the user already has it set up correctly. + // Font is already imported. Still track it for className update. + const existingVarDecl = findFontVariableDeclaration( + sourceFile, + font.font.variable + ) + if (existingVarDecl) { + fontVariableNames.push(existingVarDecl.getName()) + fontUtilityClasses.push(font.font.variable.replace("--", "")) + } continue } @@ -239,11 +247,17 @@ export async function transformLayoutFonts( } fontVariableNames.push(varName) + fontUtilityClasses.push(font.font.variable.replace("--", "")) } - // Update html className to include font variables. + // Update html className to include font variables and utility classes. if (fontVariableNames.length > 0) { - updateHtmlClassName(sourceFile, fontVariableNames, config) + updateHtmlClassName( + sourceFile, + fontVariableNames, + fontUtilityClasses, + config + ) } return sourceFile.getFullText() @@ -325,6 +339,7 @@ function findInsertPosition( function updateHtmlClassName( sourceFile: ReturnType, fontVariableNames: string[], + fontUtilityClasses: string[], config: Config ) { // Find the JSX element. @@ -336,26 +351,21 @@ function updateHtmlClassName( const tagName = element.getTagNameNode().getText() if (tagName !== "html") continue + // Build the new expressions: utility classes as strings, then .variable expressions. + const newUtilityClasses = fontUtilityClasses.map((cls) => `"${cls}"`) + const newVarExpressions = fontVariableNames.map( + (name) => `${name}.variable` + ) + const allNewArgs = [...newUtilityClasses, ...newVarExpressions] + const classNameAttr = element.getAttribute("className") if (!classNameAttr) { - // Add className attribute with font variables. - const variableExpressions = fontVariableNames - .map((name) => `${name}.variable`) - .join(", ") - - if (fontVariableNames.length === 1) { - element.addAttribute({ - name: "className", - initializer: `{${variableExpressions}}`, - }) - } else { - // Need to use cn() for multiple fonts. - ensureCnImport(sourceFile, config) - element.addAttribute({ - name: "className", - initializer: `{cn(${variableExpressions})}`, - }) - } + // Add className attribute with font utility classes and variables. + ensureCnImport(sourceFile, config) + element.addAttribute({ + name: "className", + initializer: `{cn(${allNewArgs.join(", ")})}`, + }) return } @@ -369,17 +379,12 @@ function updateHtmlClassName( if (!initializer) return - // Build the new variable expressions. - const newVarExpressions = fontVariableNames.map( - (name) => `${name}.variable` - ) - if (initializer.getKind() === SyntaxKind.StringLiteral) { - // className="some-class" -> className={cn("some-class", font.variable)} + // className="some-class" -> className={cn("some-class", "font-serif", font.variable)} const currentValue = initializer.getText().slice(1, -1) // Remove quotes. ensureCnImport(sourceFile, config) jsxAttr.setInitializer( - `{cn("${currentValue}", ${newVarExpressions.join(", ")})}` + `{cn("${currentValue}", ${allNewArgs.join(", ")})}` ) } else if (initializer.getKind() === SyntaxKind.JsxExpression) { // className={...} - need to analyze the expression. @@ -391,49 +396,49 @@ function updateHtmlClassName( // Check if it's already using cn(). if (exprText.startsWith("cn(")) { - // Check if cn() already has exactly our font variables. + // Check if cn() already has all our font variables and utility classes. const hasAllFontVars = newVarExpressions.every((v) => exprText.includes(v) ) - if (hasAllFontVars) { - // Already has our font variables, skip. + const hasAllUtilityClasses = fontUtilityClasses.every((cls) => + exprText.includes(`"${cls}"`) + ) + if (hasAllFontVars && hasAllUtilityClasses) { + // Already has everything, skip. continue } - // Remove existing font variables and add new ones. - const cleanedExpr = removeFontVariablesFromCn(exprText) - const newExpr = insertFontVariablesIntoCn( + // Remove existing font variables and utility classes, then add new ones. + let cleanedExpr = removeFontVariablesFromCn(exprText) + cleanedExpr = removeFontUtilityClassesFromCn( cleanedExpr, - newVarExpressions + fontUtilityClasses ) + const newExpr = insertFontVariablesIntoCn(cleanedExpr, allNewArgs) jsxExpr.replaceWithText(`{${newExpr}}`) } else if (/^\w+\.variable$/.test(exprText)) { // Single font variable like {inter.variable}. // Check if it's already one of our font variables. - if (newVarExpressions.includes(exprText)) { - // Already using our font variable, skip. + if ( + newVarExpressions.includes(exprText) && + fontUtilityClasses.length === 0 + ) { continue } - // Replace with our font variables. - if (newVarExpressions.length === 1) { - jsxExpr.replaceWithText(`{${newVarExpressions[0]}}`) - } else { - ensureCnImport(sourceFile, config) - jsxExpr.replaceWithText(`{cn(${newVarExpressions.join(", ")})}`) - } + // Replace with cn() including utility classes and font variables. + ensureCnImport(sourceFile, config) + jsxExpr.replaceWithText(`{cn(${allNewArgs.join(", ")})}`) } else if (exprText.startsWith("`") && exprText.endsWith("`")) { // Template literal - parse and convert to cn() arguments. const cnArgs = parseTemplateLiteralToCnArgs(exprText) ensureCnImport(sourceFile, config) jsxExpr.replaceWithText( - `{cn(${[...cnArgs, ...newVarExpressions].join(", ")})}` + `{cn(${[...cnArgs, ...allNewArgs].join(", ")})}` ) } else { // Some other expression (variable, etc.), wrap with cn(). ensureCnImport(sourceFile, config) - jsxExpr.replaceWithText( - `{cn(${exprText}, ${newVarExpressions.join(", ")})}` - ) + jsxExpr.replaceWithText(`{cn(${exprText}, ${allNewArgs.join(", ")})}`) } } } @@ -507,10 +512,23 @@ function parseTemplateLiteralToCnArgs(templateLiteral: string) { function removeFontVariablesFromCn(cnExpr: string) { // Remove patterns like "fontName.variable" from cn() call. - // This is a simple regex-based approach. return cnExpr.replace(/,?\s*\w+\.variable/g, "").replace(/cn\(\s*,/, "cn(") } +function removeFontUtilityClassesFromCn( + cnExpr: string, + utilityClasses: string[] +) { + // Remove font utility class strings like "font-sans", "font-serif" from cn() call. + let result = cnExpr + for (const cls of utilityClasses) { + result = result + .replace(new RegExp(`,?\\s*"${cls}"`, "g"), "") + .replace(/cn\(\s*,/, "cn(") + } + return result +} + function insertFontVariablesIntoCn(cnExpr: string, fontVars: string[]) { // Insert font variables at the end of cn() arguments. const varsStr = fontVars.join(", ")