[pull] main from Shopify:main

packages/skia/src/renderer/__tests__/e2e/ParagraphMethods.spec.tsx

@@ -10,6 +10,55 @@ const RobotoRegular = Array.from(
 );
 
 describe("Paragraph Methods", () => {
+  describe("paragraph style", () => {
+    it("should apply the default textStyle from the paragraph style", async () => {
+      const heights = await surface.eval(
+        (Skia, ctx) => {
+          const robotoRegular = Skia.Typeface.MakeFreeTypeFaceFromData(
+            Skia.Data.fromBytes(new Uint8Array(ctx.RobotoRegular))
+          )!;
+          const provider = Skia.TypefaceFontProvider.Make();
+          provider.registerFont(robotoRegular, "Roboto");
+
+          const textStyle = {
+            color: Skia.Color("black"),
+            fontFamilies: ["Roboto"],
+            fontSize: 24,
+          };
+
+          // The default textStyle set on the paragraph style should produce
+          // the same layout as the same style pushed explicitly.
+          const fromParagraphStyle = Skia.ParagraphBuilder.Make(
+            { textStyle },
+            provider
+          )
+            .addText("Hello")
+            .build();
+          fromParagraphStyle.layout(512);
+
+          const fromPushStyle = Skia.ParagraphBuilder.Make({}, provider)
+            .pushStyle(textStyle)
+            .addText("Hello")
+            .build();
+          fromPushStyle.layout(512);
+
+          return {
+            fromParagraphStyle: fromParagraphStyle.getLineMetrics()[0].height,
+            fromPushStyle: fromPushStyle.getLineMetrics()[0].height,
+          };
+        },
+        {
+          RobotoRegular,
+        }
+      );
+
+      expect(heights.fromParagraphStyle).toBeCloseTo(heights.fromPushStyle, 3);
+      // Roboto at fontSize 24 has a line height of ~28; the default fontSize
+      // (14) would yield ~16.4.
+      expect(heights.fromParagraphStyle).toBeGreaterThan(24);
+    });
+  });
+
   describe("getRectsForPlaceholders", () => {
     it("should handle multiple placeholders with different alignments", async () => {
       const placeholderRects = await surface.eval(

packages/skia/src/skia/web/JsiSkParagraphStyle.ts

@@ -1,16 +1,30 @@
-import type { CanvasKit, ParagraphStyle } from "canvaskit-wasm";
+import type { CanvasKit, ParagraphStyle, TextStyle } from "canvaskit-wasm";
 
 import { TextDirection } from "../types";
 import type { SkParagraphStyle } from "../types";
 
+import { JsiSkTextStyle } from "./JsiSkTextStyle";
+
 export class JsiSkParagraphStyle {
   static toParagraphStyle(
     ck: CanvasKit,
     value: SkParagraphStyle
   ): ParagraphStyle {
     // Seems like we need to provide the textStyle.color value, otherwise
     // the constructor crashes.
-    const ps = new ck.ParagraphStyle({ textStyle: { color: ck.BLACK } });
+    const textStyle: TextStyle = { color: ck.BLACK };
+    if (value.textStyle) {
+      // Only merge the properties that are set; toTextStyle() emits undefined
+      // for the others and the ParagraphStyle constructor requires a color.
+      Object.entries(JsiSkTextStyle.toTextStyle(value.textStyle)).forEach(
+        ([key, v]) => {
+          if (v !== undefined) {
+            (textStyle as Record<string, unknown>)[key] = v;
+          }
+        }
+      );
+    }
+    const ps = new ck.ParagraphStyle({ textStyle });
 
     ps.disableHinting = value.disableHinting ?? ps.disableHinting;
     ps.ellipsis = value.ellipsis ?? ps.ellipsis;

packages/skia/src/specs/NativeSkiaModule.web.ts

@@ -6,29 +6,45 @@ import type { SkiaPictureViewHandle } from "../views/SkiaPictureView.web";
 export type ISkiaViewApiWeb = ISkiaViewApi & {
   views: Record<string, SkiaPictureViewHandle>;
   deferedPictures: Record<string, SkPicture>;
+  unregisteredViews: Set<string>;
   registerView(nativeId: string, view: SkiaPictureViewHandle): void;
+  unregisterView(nativeId: string): void;
 };
 
 global.SkiaViewApi = {
   views: {},
   deferedPictures: {},
+  unregisteredViews: new Set<string>(),
   deferedOnSize: {},
   web: true,
   registerView(nativeId: string, view: SkiaPictureViewHandle) {
+    this.unregisteredViews.delete(nativeId);
     // Maybe a picture for this view was already set
     if (this.deferedPictures[nativeId]) {
       view.setPicture(this.deferedPictures[nativeId] as SkPicture);
+      delete this.deferedPictures[nativeId];
     }
     this.views[nativeId] = view;
   },
+  unregisterView(nativeId: string) {
+    // Views must be removed on unmount: the handle's closures capture the
+    // canvas element, so a stale entry retains the whole detached DOM tree.
+    this.unregisteredViews.add(nativeId);
+    delete this.views[nativeId];
+    delete this.deferedPictures[nativeId];
+  },
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   setJsiProperty(nativeId: number, name: string, value: any) {
     if (name === "picture") {
-      if (!this.views[`${nativeId}`]) {
-        this.deferedPictures[`${nativeId}`] = value;
-      } else {
-        this.views[`${nativeId}`].setPicture(value);
+      const id = `${nativeId}`;
+      if (this.views[id]) {
+        this.views[id].setPicture(value);
+      } else if (!this.unregisteredViews.has(id)) {
+        this.deferedPictures[id] = value;
       }
+      // Otherwise the view has unmounted (e.g. a trailing animation frame):
+      // drop the picture instead of deferring it for an id that will never
+      // register again, which would retain it forever.
     }
   },
   size(nativeId: number) {
@@ -39,14 +55,30 @@ global.SkiaViewApi = {
     }
   },
   requestRedraw(nativeId: number) {
-    this.views[`${nativeId}`].redraw();
+    // The view may already have unmounted (e.g. a trailing animation frame).
+    this.views[`${nativeId}`]?.redraw();
   },
   makeImageSnapshot(nativeId: number, rect?: SkRect) {
-    return this.views[`${nativeId}`].makeImageSnapshot(rect);
+    const view = this.views[`${nativeId}`];
+    if (!view) {
+      throw new Error(
+        `Cannot make image snapshot: view with nativeID ${nativeId} is not registered (it may have unmounted)`
+      );
+    }
+    return view.makeImageSnapshot(rect);
   },
   makeImageSnapshotAsync(nativeId: number, rect?: SkRect) {
     return new Promise((resolve, reject) => {
-      const result = this.views[`${nativeId}`].makeImageSnapshot(rect);
+      const view = this.views[`${nativeId}`];
+      if (!view) {
+        reject(
+          new Error(
+            `Cannot make image snapshot: view with nativeID ${nativeId} is not registered (it may have unmounted)`
+          )
+        );
+        return;
+      }
+      const result = view.makeImageSnapshot(rect);
       if (result) {
         resolve(result);
       } else {