From 5665baa8735b04daf41cf8160c21575984525349 Mon Sep 17 00:00:00 2001
From: William Candillon <wcandillon@gmail.com>
Date: Fri, 5 Jul 2024 14:45:03 +0200
Subject: [PATCH] =?UTF-8?q?fix(=F0=9F=90=9B):=20Fix=20parameter=20handling?=
 =?UTF-8?q?=20in=20SkTypefaceFontProvider.matchFamilyStyle()=20(#2518)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 docs/docs/text/text.md                        |  7 +++
 package/cpp/api/JsiSkTypefaceFontProvider.h   | 18 +++++--
 .../renderer/__tests__/e2e/FontMgr.spec.tsx   | 49 +++++++++++++++++--
 package/src/skia/types/Font/FontMgr.ts        |  2 +-
 4 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/docs/docs/text/text.md b/docs/docs/text/text.md
index 8ef19c6f70..d7a9fc143d 100644
--- a/docs/docs/text/text.md
+++ b/docs/docs/text/text.md
@@ -43,6 +43,13 @@ export const HelloWorld = () => {
 
 Once the fonts are loaded, we provide a `matchFont` function that given a font style will return a font object that you can use directly.
 
+:::info
+
+For font matching we recommend using the [Paragraph API](/docs/text/paragraph/) instead.
+The APIs belows were made available before the Paragraph API was released.
+
+:::
+
 ```tsx twoslash
 import {useFonts, Text, matchFont} from "@shopify/react-native-skia";
 
diff --git a/package/cpp/api/JsiSkTypefaceFontProvider.h b/package/cpp/api/JsiSkTypefaceFontProvider.h
index e2ef47a9db..e939021aa4 100644
--- a/package/cpp/api/JsiSkTypefaceFontProvider.h
+++ b/package/cpp/api/JsiSkTypefaceFontProvider.h
@@ -45,10 +45,20 @@ class JsiSkTypefaceFontProvider
   }
 
   JSI_HOST_FUNCTION(matchFamilyStyle) {
-    auto name = arguments[0].asString(runtime).utf8(runtime);
-    auto fontStyle = JsiSkFontStyle::fromValue(runtime, arguments[1]);
-    sk_sp<SkFontStyleSet> set(getObject()->onMatchFamily(name.c_str()));
-    sk_sp<SkTypeface> typeface(set->matchStyle(*fontStyle));
+    auto name = count > 0 ? arguments[0].asString(runtime).utf8(runtime) : "";
+    auto fontStyle =
+        count > 1 ? JsiSkFontStyle::fromValue(runtime, arguments[1]) : nullptr;
+    if (name == "" || fontStyle == nullptr) {
+      throw std::runtime_error("matchFamilyStyle requires a name and a style");
+    }
+    auto set = getObject()->onMatchFamily(name.c_str());
+    if (!set) {
+      throw std::runtime_error("Could not find font family " + name);
+    }
+    auto typeface = set->matchStyle(*fontStyle);
+    if (!typeface) {
+      throw std::runtime_error("Could not find font style for " + name);
+    }
     return jsi::Object::createFromHostObject(
         runtime, std::make_shared<JsiSkTypeface>(getContext(), typeface));
   }
diff --git a/package/src/renderer/__tests__/e2e/FontMgr.spec.tsx b/package/src/renderer/__tests__/e2e/FontMgr.spec.tsx
index 57a21622af..27c51f97ce 100644
--- a/package/src/renderer/__tests__/e2e/FontMgr.spec.tsx
+++ b/package/src/renderer/__tests__/e2e/FontMgr.spec.tsx
@@ -101,7 +101,50 @@ describe("FontMgr", () => {
       expect(width).not.toEqual([0, 0]);
     }
   });
-  // Add test
-  //     *  Passing |nullptr| as the parameter for |familyName| will return the
-  // *  default system font.
+  itRunsE2eOnly("Shouldn't crash the font cannot be resolved", async () => {
+    const result = await surface.eval(
+      (Skia, { fonts }) => {
+        const fontMgr = Skia.TypefaceFontProvider.Make();
+        (Object.keys(fonts) as (keyof typeof fonts)[]).flatMap((familyName) => {
+          const typefaces = fonts[familyName];
+          typefaces.forEach((typeface) => {
+            const data = Skia.Data.fromBytes(new Uint8Array(typeface));
+            fontMgr.registerFont(
+              Skia.Typeface.MakeFreeTypeFaceFromData(data)!,
+              familyName
+            );
+          });
+        });
+        let exists1 = true;
+        let exists2 = true;
+        let exists3 = true;
+        try {
+          fontMgr.matchFamilyStyle("Robot", {
+            weight: 400,
+          });
+        } catch {
+          exists1 = false;
+        }
+        try {
+          fontMgr.matchFamilyStyle("Roboto", {
+            weight: 100,
+          });
+        } catch {
+          exists2 = false;
+        }
+        try {
+          // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+          // @ts-expect-error
+          fontMgr.matchFamilyStyle();
+        } catch {
+          exists3 = false;
+        }
+        return { exists1, exists2, exists3 };
+      },
+      { fonts: testingFonts }
+    );
+    expect(result.exists1).toBe(false);
+    expect(result.exists2).toBe(true);
+    expect(result.exists3).toBe(false);
+  });
 });
diff --git a/package/src/skia/types/Font/FontMgr.ts b/package/src/skia/types/Font/FontMgr.ts
index e4bda405ef..7dce039b1a 100644
--- a/package/src/skia/types/Font/FontMgr.ts
+++ b/package/src/skia/types/Font/FontMgr.ts
@@ -6,5 +6,5 @@ import type { FontStyle } from "./Font";
 export interface SkFontMgr extends SkJSIInstance<"FontMgr"> {
   countFamilies(): number;
   getFamilyName(index: number): string;
-  matchFamilyStyle(name?: string, style?: FontStyle): SkTypeface;
+  matchFamilyStyle(name: string, style: FontStyle): SkTypeface;
 }