From cb5bbf13c0120fd455d51291cd95f4e532786c83 Mon Sep 17 00:00:00 2001 From: Christian Falch <875252+chrfalch@users.noreply.github.com> Date: Fri, 10 Feb 2023 14:51:28 +0100 Subject: [PATCH] #1284 - Fixed issue with dependency-manager's update (#1345) * Fixed issue with dependency-manager's update If you mount / unmount / mount a component, the dependency manager's update method is not called correctly causing a missing detection of the precense of an animation value - making the animation stuck. This PR fixes this by updating the dependency manager after a commit phase in the reconciler - In addition it eases the error message given by the canvas component if you try to call the registerValues method when canvas is null. It is not possible to make a unit test for this problem - since it is related to an animation and some complex omount/unmount stuff. The following example shows the bug (a green rect is mounted/unmounted repeatedly, and the animation should pick up immediately after it is mounted.) ```ts import React, { useEffect, useState } from "react"; import { Canvas, Circle, mix, Rect, useComputedValue, useLoop, } from "@shopify/react-native-skia"; const App = () => { return ( ); }; function Mounter() { const [showRect, setShowRect] = useState(false); useEffect(() => { const t = setInterval(() => { setShowRect((p) => !p); }, 1000); return () => clearInterval(t); }, []); if (!showRect) { return null; } return ; } function RectComp() { const loop = useLoop(); const height = useComputedValue(() => mix(loop.current, 100, 200), [loop]); return ; } // eslint-disable-next-line import/no-default-export export default App; ``` * Removed superflous call to dependency manager's update function * Fix CI * Kept hostDebug --- package/src/renderer/Canvas.tsx | 6 +++--- package/src/renderer/HostConfig.ts | 1 + package/src/renderer/Reconciler.tsx | 1 - 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package/src/renderer/Canvas.tsx b/package/src/renderer/Canvas.tsx index 306f4731e0..165adeb594 100644 --- a/package/src/renderer/Canvas.tsx +++ b/package/src/renderer/Canvas.tsx @@ -40,10 +40,10 @@ export const Canvas = forwardRef( const registerValues = useCallback( (values: Array>) => { - if (ref.current === null) { - throw new Error("Canvas ref is not set"); + if (ref.current !== null) { + return ref.current.registerValues(values); } - return ref.current.registerValues(values); + return () => {}; }, [ref] ); diff --git a/package/src/renderer/HostConfig.ts b/package/src/renderer/HostConfig.ts index fc66964939..899f22db60 100644 --- a/package/src/renderer/HostConfig.ts +++ b/package/src/renderer/HostConfig.ts @@ -155,6 +155,7 @@ export const skHostConfig: SkiaHostConfig = { resetAfterCommit(container) { debug("resetAfterCommit"); + container.depMgr.update(); container.redraw(); }, diff --git a/package/src/renderer/Reconciler.tsx b/package/src/renderer/Reconciler.tsx index d2c9b1cfe7..54bbf187ee 100644 --- a/package/src/renderer/Reconciler.tsx +++ b/package/src/renderer/Reconciler.tsx @@ -50,7 +50,6 @@ export class SkiaRoot { render(element: ReactNode) { skiaReconciler.updateContainer(element, this.root, null, () => { hostDebug("updateContainer"); - this.container.depMgr.update(); }); }