handleTouchEvent called after Canvas is out of focus

[tlow92]

Description

Original error: TypeError: Cannot read properties of undefined (reading 'left')

https://github.com/Shopify/react-native-skia/blob/cc7f30da719e7cfcc3e6c49f84a730ecb9e7f499/package/src/views/SkiaBaseWebView.tsx#L153

Due to getClientRects() returning empty array when Canvas is hidden.

Version

At least 0.1.221 and 1.3.8 (latest) but most likely all versions

Steps to reproduce

1. onPress with navigation push event is wrapped around canvas
2. When clicking on the canvas react-navigation pushes the new screen to Stack, old screen is hidden (this is important) but not un-mounted
3. The mouseUp event from react-native-skia web is still called, but due to the screen being hidden getClientRects is empty and e.currentTarget.getClientRects()[0].left fails

Snack, code example, screenshot, or link to a repository

https://snack.expo.dev/@tlow92/6e407c

1. Open javascript console
2. Click on the Breathe canvas

https://github.com/user-attachments/assets/332bbbc3-28b3-42e8-9b40-2c1feed1ee26

[tlow92]

Temporary fix

Since we only use GestureDetector anyway with skia I disabled all handlers, since they are called unnecessarily (especially mouseMove)

diff --git a/node_modules/@shopify/react-native-skia/lib/module/views/SkiaBaseWebView.js b/node_modules/@shopify/react-native-skia/lib/module/views/SkiaBaseWebView.js
index 5675b03..282082a 100644
--- a/node_modules/@shopify/react-native-skia/lib/module/views/SkiaBaseWebView.js
+++ b/node_modules/@shopify/react-native-skia/lib/module/views/SkiaBaseWebView.js
@@ -22,10 +22,10 @@ export class SkiaBaseWebView extends React.Component {
     _defineProperty(this, "requestId", 0);
     _defineProperty(this, "width", 0);
     _defineProperty(this, "height", 0);
-    _defineProperty(this, "onStart", this.createTouchHandler(TouchType.Start));
-    _defineProperty(this, "onActive", this.createTouchHandler(TouchType.Active));
-    _defineProperty(this, "onCancel", this.createTouchHandler(TouchType.Cancelled));
-    _defineProperty(this, "onEnd", this.createTouchHandler(TouchType.End));
+    _defineProperty(this, "onStart", null);
+    _defineProperty(this, "onActive", null);
+    _defineProperty(this, "onCancel", null);
+    _defineProperty(this, "onEnd", null);
     _defineProperty(this, "onLayout", this.onLayoutEvent.bind(this));
     this._mode = (_props$mode = props.mode) !== null && _props$mode !== void 0 ? _props$mode : "default";
   }

Another way would be to guard the handleTouchEvent with if(!evt.currentTarget.getClientRects()[0] !== undefined) return

[wcandillon]

Nice. We are planning to remove this API completely (it already throws a warning at the moment if you use it) so it sounds like when finally removing, it will fix this issue? We will remove this API completely soon hopefully.

[tlow92]

Yes, removing it would fix it and I agree that it should be removed in favour of gesture-handler.

I have not benchmarked it, but I can imagine that the mousemove handler that is attached by default adds some performance penalty, so would be nice if we get rid of this.

[wcandillon]

absolutely. We first wanted to just throw a warning for a while to give people time to migrate.