Skip to content

Commit

Permalink
#1284 - Fixed issue with dependency-manager's update (#1345)
Browse files Browse the repository at this point in the history
* 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 (
    <Canvas style={{ flex: 1 }}>
      <Circle cx={100} cy={100} r={50} color="red" />
      <Mounter />
    </Canvas>
  );
};

function Mounter() {
  const [showRect, setShowRect] = useState(false);

  useEffect(() => {
    const t = setInterval(() => {
      setShowRect((p) => !p);
    }, 1000);
    return () => clearInterval(t);
  }, []);

  if (!showRect) {
    return null;
  }

  return <RectComp />;
}

function RectComp() {
  const loop = useLoop();
  const height = useComputedValue(() => mix(loop.current, 100, 200), [loop]);
  return <Rect x={0} y={0} width={100} height={height} color="green" />;
}

// eslint-disable-next-line import/no-default-export
export default App;

```

* Removed superflous call to dependency manager's update function

* Fix CI

* Kept hostDebug
  • Loading branch information
chrfalch authored Feb 10, 2023
1 parent 44bea66 commit cb5bbf1
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 4 deletions.
6 changes: 3 additions & 3 deletions package/src/renderer/Canvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ export const Canvas = forwardRef<SkiaDomView, CanvasProps>(

const registerValues = useCallback(
(values: Array<SkiaValue<unknown>>) => {
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]
);
Expand Down
1 change: 1 addition & 0 deletions package/src/renderer/HostConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ export const skHostConfig: SkiaHostConfig = {

resetAfterCommit(container) {
debug("resetAfterCommit");
container.depMgr.update();
container.redraw();
},

Expand Down
1 change: 0 additions & 1 deletion package/src/renderer/Reconciler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export class SkiaRoot {
render(element: ReactNode) {
skiaReconciler.updateContainer(element, this.root, null, () => {
hostDebug("updateContainer");
this.container.depMgr.update();
});
}

Expand Down

0 comments on commit cb5bbf1

Please sign in to comment.