Reduce local font loader options (vercel#41332)

Reduces the amount of options for local fonts. Adds additional regression tests. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
Kikobeats · Oct 24, 2022 · 2e54f66 · 2e54f66
1 parent e29d021
commit 2e54f66
Show file tree

Hide file tree

Showing 11 changed files with 172 additions and 85 deletions.
diff --git a/packages/font/src/local/index.ts b/packages/font/src/local/index.ts
@@ -7,20 +7,11 @@ type LocalFont = {
   display?: Display
   weight?: number
   style?: string
+  adjustFontFallback?: 'Arial' | 'Times New Roman' | false
   fallback?: string[]
   preload?: boolean
   variable?: CssVariable
-
-  fontStretch?: string
-  fontVariant?: string
-  fontFeatureSettings?: string
-  fontVariationSettings?: string
-  ascentOverride?: string
-  descentOverride?: string
-  lineGapOverride?: string
-  sizeAdjust?: string
-
-  adjustFontFallback?: 'Arial' | 'Times New Roman' | false
+  declarations?: Array<{ prop: string; value: string }>
 }
 
 export default function localFont(options: LocalFont): FontModule {

diff --git a/packages/font/src/local/loader.ts b/packages/font/src/local/loader.ts
@@ -26,13 +26,8 @@ const fetchFonts: FontLoader = async ({
     fallback,
     preload,
     variable,
-    ascentOverride,
-    descentOverride,
-    lineGapOverride,
-    fontStretch,
-    fontFeatureSettings,
-    sizeAdjust,
     adjustFontFallback,
+    declarations,
   } = validateData(functionName, data)
 
   const resolved = await resolve(src)
@@ -74,19 +69,14 @@ const fetchFonts: FontLoader = async ({
   }
 
   const fontFaceProperties = [
+    ...(declarations
+      ? declarations.map(({ prop, value }) => [prop, value])
+      : []),
     ['font-family', `'${fontMetadata?.familyName ?? family}'`],
     ['src', `url(${fontUrl}) format('${format}')`],
     ['font-display', display],
     ...(weight ? [['font-weight', weight]] : []),
     ...(style ? [['font-style', style]] : []),
-    ...(ascentOverride ? [['ascent-override', ascentOverride]] : []),
-    ...(descentOverride ? [['descent-override', descentOverride]] : []),
-    ...(lineGapOverride ? [['line-gap-override', lineGapOverride]] : []),
-    ...(fontStretch ? [['font-stretch', fontStretch]] : []),
-    ...(fontFeatureSettings
-      ? [['font-feature-settings', fontFeatureSettings]]
-      : []),
-    ...(sizeAdjust ? [['size-adjust', sizeAdjust]] : []),
   ]
 
   const css = `@font-face {

diff --git a/packages/font/src/local/utils.ts b/packages/font/src/local/utils.ts
@@ -22,15 +22,8 @@ type FontOptions = {
   fallback?: string[]
   preload: boolean
   variable?: string
-  ascentOverride?: string
-  descentOverride?: string
-  fontStretch?: string
-  fontVariant?: string
-  fontFeatureSettings?: string
-  fontVariationSettings?: string
-  lineGapOverride?: string
-  sizeAdjust?: string
   adjustFontFallback?: string | false
+  declarations?: Array<{ prop: string; value: string }>
 }
 export function validateData(functionName: string, data: any): FontOptions {
   if (functionName) {
@@ -44,15 +37,8 @@ export function validateData(functionName: string, data: any): FontOptions {
     fallback,
     preload = true,
     variable,
-    ascentOverride,
-    descentOverride,
-    fontStretch,
-    fontVariant,
-    fontFeatureSettings,
-    fontVariationSettings,
-    lineGapOverride,
-    sizeAdjust,
     adjustFontFallback,
+    declarations,
   } = data[0] || ({} as any)
 
   if (!allowedDisplayValues.includes(display)) {
@@ -74,6 +60,22 @@ export function validateData(functionName: string, data: any): FontOptions {
 
   const family = /.+\/(.+?)\./.exec(src)![1]
 
+  if (Array.isArray(declarations)) {
+    declarations.forEach((declaration) => {
+      if (
+        [
+          'font-family',
+          'src',
+          'font-display',
+          'font-weight',
+          'font-style',
+        ].includes(declaration?.prop)
+      ) {
+        throw new Error(`Invalid declaration prop: \`${declaration.prop}\``)
+      }
+    })
+  }
+
   return {
     family,
     src,
@@ -85,14 +87,7 @@ export function validateData(functionName: string, data: any): FontOptions {
     fallback,
     preload,
     variable,
-    ascentOverride,
-    descentOverride,
-    fontStretch,
-    fontVariant,
-    fontFeatureSettings,
-    fontVariationSettings,
-    lineGapOverride,
-    sizeAdjust,
     adjustFontFallback,
+    declarations,
   }
 }
diff --git a/test/e2e/app-dir/next-font.test.ts b/test/e2e/app-dir/next-font.test.ts
@@ -296,6 +296,58 @@ describe('app dir next-font', () => {
         type: 'font/woff2',
       })
     })
+
+    it('should preload correctly with layout using fonts', async () => {
+      const html = await renderViaHTTP(next.url, '/layout-with-fonts')
+      const $ = cheerio.load(html)
+
+      // Preconnect
+      expect($('link[rel="preconnect"]').length).toBe(0)
+
+      expect($('link[as="font"]').length).toBe(2)
+      // From root layout
+      expect($('link[as="font"]').get(0).attribs).toEqual({
+        as: 'font',
+        crossorigin: '',
+        href: '/_next/static/media/e9b9dc0d8ba35f48.p.woff2',
+        rel: 'preload',
+        type: 'font/woff2',
+      })
+
+      expect($('link[as="font"]').get(1).attribs).toEqual({
+        as: 'font',
+        crossorigin: '',
+        href: '/_next/static/media/75c5faeeb9c86969.p.woff2',
+        rel: 'preload',
+        type: 'font/woff2',
+      })
+    })
+
+    it('should preload correctly with page using fonts', async () => {
+      const html = await renderViaHTTP(next.url, '/page-with-fonts')
+      const $ = cheerio.load(html)
+
+      // Preconnect
+      expect($('link[rel="preconnect"]').length).toBe(0)
+
+      expect($('link[as="font"]').length).toBe(2)
+      // From root layout
+      expect($('link[as="font"]').get(0).attribs).toEqual({
+        as: 'font',
+        crossorigin: '',
+        href: '/_next/static/media/e9b9dc0d8ba35f48.p.woff2',
+        rel: 'preload',
+        type: 'font/woff2',
+      })
+
+      expect($('link[as="font"]').get(1).attribs).toEqual({
+        as: 'font',
+        crossorigin: '',
+        href: '/_next/static/media/568e4c6d8123c4d6.p.woff2',
+        rel: 'preload',
+        type: 'font/woff2',
+      })
+    })
   })
 
   if (isDev) {

diff --git a/test/e2e/app-dir/next-font/app/layout-with-fonts/layout-font.woff2 b/test/e2e/app-dir/next-font/app/layout-with-fonts/layout-font.woff2
@@ -0,0 +1 @@
+layout-font
diff --git a/test/e2e/app-dir/next-font/app/layout-with-fonts/layout.js b/test/e2e/app-dir/next-font/app/layout-with-fonts/layout.js
@@ -0,0 +1,14 @@
+import localFont from '@next/font/local'
+
+const layoutFont = localFont({ src: './layout-font.woff2' })
+
+export default function Layout({ children }) {
+  return (
+    <>
+      <p id="layout-with-fonts" className={layoutFont.className}>
+        {JSON.stringify(layoutFont)}
+      </p>
+      {children}
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/next-font/app/layout-with-fonts/page.js b/test/e2e/app-dir/next-font/app/layout-with-fonts/page.js
@@ -0,0 +1,3 @@
+export default function HomePage() {
+  return <p>Only layout</p>
+}
diff --git a/test/e2e/app-dir/next-font/app/page-with-fonts/layout.js b/test/e2e/app-dir/next-font/app/page-with-fonts/layout.js
@@ -0,0 +1,3 @@
+export default function Layout({ children }) {
+  return children
+}
diff --git a/test/e2e/app-dir/next-font/app/page-with-fonts/page-font.woff2 b/test/e2e/app-dir/next-font/app/page-with-fonts/page-font.woff2
@@ -0,0 +1 @@
+page-font
diff --git a/test/e2e/app-dir/next-font/app/page-with-fonts/page.js b/test/e2e/app-dir/next-font/app/page-with-fonts/page.js
@@ -0,0 +1,11 @@
+import localFont from '@next/font/local'
+
+const pageFont = localFont({ src: './page-font.woff2' })
+
+export default function HomePage() {
+  return (
+    <p id="page-with-fonts" className={pageFont.className}>
+      {JSON.stringify(pageFont)}
+    </p>
+  )
+}
diff --git a/test/unit/local-font-loader.test.ts b/test/unit/local-font-loader.test.ts
@@ -15,12 +15,12 @@ describe('@next/font/local', () => {
       })
 
       expect(css).toMatchInlineSnapshot(`
-"@font-face {
-font-family: 'my-font';
-src: url(/_next/static/media/my-font.woff2) format('woff2');
-font-display: optional;
-}"
-`)
+        "@font-face {
+        font-family: 'my-font';
+        src: url(/_next/static/media/my-font.woff2) format('woff2');
+        font-display: optional;
+        }"
+      `)
     })
 
     test('Weight and style', async () => {
@@ -36,14 +36,14 @@ font-display: optional;
       })
 
       expect(css).toMatchInlineSnapshot(`
-"@font-face {
-font-family: 'my-font';
-src: url(/_next/static/media/my-font.woff2) format('woff2');
-font-display: optional;
-font-weight: 100 900;
-font-style: italic;
-}"
-`)
+        "@font-face {
+        font-family: 'my-font';
+        src: url(/_next/static/media/my-font.woff2) format('woff2');
+        font-display: optional;
+        font-weight: 100 900;
+        font-style: italic;
+        }"
+      `)
     })
 
     test('Other properties', async () => {
@@ -52,14 +52,10 @@ font-style: italic;
         data: [
           {
             src: './my-font.woff2',
-            weight: '100 900',
-            style: 'italic',
-            ascentOverride: 'ascentOverride',
-            descentOverride: 'descentOverride',
-            lineGapOverride: 'lineGapOverride',
-            fontStretch: 'fontStretch',
-            fontFeatureSettings: 'fontFeatureSettings',
-            sizeAdjust: 'sizeAdjust',
+            declarations: [
+              { prop: 'font-feature-settings', value: '"smcp" on' },
+              { prop: 'ascent-override', value: '90%' },
+            ],
           },
         ],
         config: {},
@@ -71,20 +67,14 @@ font-style: italic;
       })
 
       expect(css).toMatchInlineSnapshot(`
-"@font-face {
-font-family: 'my-font';
-src: url(/_next/static/media/my-font.woff2) format('woff2');
-font-display: optional;
-font-weight: 100 900;
-font-style: italic;
-ascent-override: ascentOverride;
-descent-override: descentOverride;
-line-gap-override: lineGapOverride;
-font-stretch: fontStretch;
-font-feature-settings: fontFeatureSettings;
-size-adjust: sizeAdjust;
-}"
-`)
+        "@font-face {
+        font-feature-settings: \\"smcp\\" on;
+        ascent-override: 90%;
+        font-family: 'my-font';
+        src: url(/_next/static/media/my-font.woff2) format('woff2');
+        font-display: optional;
+        }"
+      `)
     })
   })
 
@@ -104,6 +94,21 @@ size-adjust: sizeAdjust;
       )
     })
 
+    test('Missing src', async () => {
+      await expect(
+        loader({
+          functionName: '',
+          data: [],
+          config: {},
+          emitFontFile: jest.fn(),
+          resolve: jest.fn(),
+          fs: {},
+        })
+      ).rejects.toThrowErrorMatchingInlineSnapshot(
+        `"Missing required \`src\` property"`
+      )
+    })
+
     test('Invalid file extension', async () => {
       await expect(
         loader({
@@ -134,5 +139,26 @@ size-adjust: sizeAdjust;
               Available display values: \`auto\`, \`block\`, \`swap\`, \`fallback\`, \`optional\`"
             `)
     })
+
+    test('Invalid declaration', async () => {
+      await expect(
+        loader({
+          functionName: '',
+          data: [
+            {
+              src: './font-file.woff2',
+              declarations: [{ prop: 'src', value: '/hello.woff2' }],
+            },
+          ],
+
+          config: {},
+          emitFontFile: jest.fn(),
+          resolve: jest.fn(),
+          fs: {},
+        })
+      ).rejects.toThrowErrorMatchingInlineSnapshot(
+        `"Invalid declaration prop: \`src\`"`
+      )
+    })
   })
 })