fix(next/font): update axes error logic (#72442)

## Why? The current error message when you have have a set `weight: ['400', '700']` and set `axes: ['wdth']` for a variable font is confusing—it should mention to correct your weight property (e.g., `weight: 'variable'` or nothing at all for `weight`). - Fixes #72413
vercel · Nov 7, 2024 · a422054 · a422054
1 parent 49ac101
commit a422054
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 28 deletions.
diff --git a/crates/next-core/src/next_font/google/options.rs b/crates/next-core/src/next_font/google/options.rs
@@ -99,8 +99,9 @@ pub(super) fn options_from_request(
         })
         .unwrap_or_default();
 
+    let supports_variable_weight = font_data.weights.iter().any(|el| el == "variable");
     let weights = if requested_weights.is_empty() {
-        if !font_data.weights.contains(&"variable".into()) {
+        if !supports_variable_weight {
             return Err(anyhow!(
                 "Missing weight for {}. Available weights: {}",
                 font_family,
@@ -170,8 +171,17 @@ pub(super) fn options_from_request(
     }
 
     if let Some(axes) = argument.axes.as_ref() {
-        if !axes.is_empty() && !matches!(weights, FontWeights::Variable) {
-            return Err(anyhow!("Axes can only be defined for variable fonts"));
+        if !axes.is_empty() {
+            if !supports_variable_weight {
+                return Err(anyhow!("Axes can only be defined for variable fonts."));
+            }
+
+            if weights != FontWeights::Variable {
+                return Err(anyhow!(
+                    "Axes can only be defined for variable fonts when the weight property is \
+                     nonexistent or set to `variable`."
+                ));
+            }
         }
     }
 
@@ -208,7 +218,7 @@ mod tests {
                     "styles": ["normal", "italic"]
                 }
             }
-  "#,
+            "#,
         )?;
 
         let request: NextFontRequest = parse_json_with_source_context(
@@ -219,7 +229,7 @@ mod tests {
                 "variableName": "inter",
                 "arguments": [{}]
             }
-        "#,
+            "#,
         )?;
 
         match options_from_request(&request, &data) {
@@ -241,7 +251,7 @@ mod tests {
                     "styles": ["normal", "italic"]
                 }
             }
-  "#,
+            "#,
         )?;
 
         let request: NextFontRequest = parse_json_with_source_context(
@@ -252,7 +262,7 @@ mod tests {
                 "variableName": "abeezee",
                 "arguments": []
             }
-        "#,
+            "#,
         )?;
 
         assert_eq!(
@@ -284,7 +294,7 @@ mod tests {
                     "styles": ["normal", "italic"]
                 }
             }
-  "#,
+            "#,
         )?;
 
         let request: NextFontRequest = parse_json_with_source_context(
@@ -295,7 +305,7 @@ mod tests {
                 "variableName": "abeezee",
                 "arguments": [{}]
             }
-        "#,
+            "#,
         )?;
 
         match options_from_request(&request, &data) {
@@ -320,7 +330,7 @@ mod tests {
                     "styles": ["normal", "italic"]
                 }
             }
-  "#,
+            "#,
         )?;
 
         let request: NextFontRequest = parse_json_with_source_context(
@@ -333,7 +343,7 @@ mod tests {
                     "weight": ["400", "variable"]
                 }]
             }
-        "#,
+            "#,
         )?;
 
         match options_from_request(&request, &data) {
@@ -359,7 +369,7 @@ mod tests {
                     "styles": ["normal", "italic"]
                 }
             }
-  "#,
+            "#,
         )?;
 
         let request: NextFontRequest = parse_json_with_source_context(
@@ -372,7 +382,7 @@ mod tests {
                     "weight": ["200"]
                 }]
             }
-        "#,
+            "#,
         )?;
 
         match options_from_request(&request, &data) {
@@ -397,7 +407,7 @@ mod tests {
                     "styles": ["italic"]
                 }
             }
-  "#,
+            "#,
         )?;
 
         let request: NextFontRequest = parse_json_with_source_context(
@@ -410,7 +420,7 @@ mod tests {
                     "weight": ["400"]
                 }]
             }
-        "#,
+            "#,
         )?;
 
         let options = options_from_request(&request, &data)?;
@@ -429,7 +439,7 @@ mod tests {
                     "styles": ["normal", "italic"]
                 }
             }
-  "#,
+            "#,
         )?;
 
         let request: NextFontRequest = parse_json_with_source_context(
@@ -442,7 +452,7 @@ mod tests {
                     "weight": ["400"]
                 }]
             }
-        "#,
+            "#,
         )?;
 
         let options = options_from_request(&request, &data)?;
@@ -461,7 +471,7 @@ mod tests {
                     "styles": ["normal", "italic"]
                 }
             }
-  "#,
+            "#,
         )?;
 
         let request: NextFontRequest = parse_json_with_source_context(
@@ -475,7 +485,7 @@ mod tests {
                     "style": ["foo"]
                 }]
             }
-        "#,
+            "#,
         )?;
 
         match options_from_request(&request, &data) {
@@ -501,7 +511,7 @@ mod tests {
                     "styles": ["normal", "italic"]
                 }
             }
-  "#,
+            "#,
         )?;
 
         let request: NextFontRequest = parse_json_with_source_context(
@@ -515,7 +525,7 @@ mod tests {
                     "display": "foo"
                 }]
             }
-        "#,
+            "#,
         )?;
 
         match options_from_request(&request, &data) {
@@ -533,7 +543,7 @@ mod tests {
     }
 
     #[test]
-    fn test_errors_on_axes_without_variable() -> Result<()> {
+    fn test_errors_on_axes_without_variable_weight() -> Result<()> {
         let data: FxIndexMap<RcStr, FontDataEntry> = parse_json_with_source_context(
             r#"
             {
@@ -542,7 +552,7 @@ mod tests {
                     "styles": ["normal", "italic"]
                 }
             }
-  "#,
+            "#,
         )?;
 
         let request: NextFontRequest = parse_json_with_source_context(
@@ -556,15 +566,56 @@ mod tests {
                     "axes": ["wght"]
                 }]
             }
-        "#,
+            "#,
+        )?;
+
+        match options_from_request(&request, &data) {
+            Ok(_) => panic!(),
+            Err(err) => {
+                assert_eq!(
+                    err.to_string(),
+                    "Axes can only be defined for variable fonts when the weight property is \
+                     nonexistent or set to `variable`."
+                )
+            }
+        }
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_errors_on_axes_without_variable_font() -> Result<()> {
+        let data: FxIndexMap<RcStr, FontDataEntry> = parse_json_with_source_context(
+            r#"
+            {
+                "ABeeZee": {
+                    "weights": ["400", "700"],
+                    "styles": ["normal", "italic"]
+                }
+            }
+            "#,
+        )?;
+
+        let request: NextFontRequest = parse_json_with_source_context(
+            r#"
+            {
+                "import": "ABeeZee",
+                "path": "index.js",
+                "variableName": "abeezee",
+                "arguments": [{
+                    "weight": ["400", "700"],
+                    "axes": ["wght"]
+                }]
+            }
+            "#,
         )?;
 
         match options_from_request(&request, &data) {
             Ok(_) => panic!(),
             Err(err) => {
                 assert_eq!(
                     err.to_string(),
-                    "Axes can only be defined for variable fonts"
+                    "Axes can only be defined for variable fonts."
                 )
             }
         }

diff --git a/packages/font/src/google/validate-google-font-function-call.test.ts b/packages/font/src/google/validate-google-font-function-call.test.ts
@@ -101,7 +101,19 @@ describe('validateFontFunctionCall errors', () => {
         subsets: ['latin'],
       })
     ).toThrowErrorMatchingInlineSnapshot(
-      `"Axes can only be defined for variable fonts"`
+      `"Axes can only be defined for variable fonts."`
+    )
+  })
+
+  test('Setting axes on variable font with incorrect weight', async () => {
+    expect(() =>
+      validateGoogleFontFunctionCall('Roboto_Flex', {
+        weight: ['400', '700'],
+        axes: ['wght'],
+        subsets: ['latin'],
+      })
+    ).toThrowErrorMatchingInlineSnapshot(
+      '"Axes can only be defined for variable fonts when the weight property is nonexistent or set to `variable`."'
     )
   })
 })
diff --git a/packages/font/src/google/validate-google-font-function-call.ts b/packages/font/src/google/validate-google-font-function-call.ts
@@ -135,8 +135,16 @@ export function validateGoogleFontFunctionCall(
     )
   }
 
-  if (weights[0] !== 'variable' && axes) {
-    nextFontError('Axes can only be defined for variable fonts')
+  if (axes) {
+    if (!fontWeights.includes('variable')) {
+      nextFontError('Axes can only be defined for variable fonts.')
+    }
+
+    if (weights[0] !== 'variable') {
+      nextFontError(
+        'Axes can only be defined for variable fonts when the weight property is nonexistent or set to `variable`.'
+      )
+    }
   }
 
   return {