P5 instances not remove()'d on component unmount?

[fredrikelmegren]

Hi there!

Thanks for a nifty little wrapper! 🙏🏼

I'm having some issues in my Next.js setup however, the P5 instance keeps running when I navigate between pages. I may very well just have missed something in my implementation for the below issue - but I couldn't figure out what 🤷🏼.

NB. I import using next/dynamic ReactP5Wrapper to avoid SSR:

const ReactP5Wrapper = dynamic(
  () => {
    return import("react-p5-wrapper")
      .then((m) => m.ReactP5Wrapper)
      .catch(console.error);
  },
  { ssr: false }
);

Expected Behavior

When a component including a <P5Wrapper> is unmounted, I would expect the P5 instance to be cleaned up and removed (with remove()).

Actual Behavior

The P5 instance continues to live and run even though the component is unmounted.

Steps to Reproduce the Problem

Here's a codesandbox demonstrating the issue: https://codesandbox.io/s/prod-architecture-tf8l0

1. Navigate to "Page with ReactP5Wrapper", containing the P5 example.
2. Navigate back the landing page again and notice components on page B are unmounted but the P5 instance is still running (the console.log from within the sketch's draw() keeps running when using the browser's back button).

Workaround / suggested solution

I've worked around it by wrapping the wrapper (....🤪) and keeping tabs on the p5 instance with a ref, and removing it on unmount:

const SketchWrapper = ({ sketch }) => {
  const p5InstancRef = useRef();

  useEffect(() => {
    return () => {
      if (p5InstancRef.current) {
        p5InstancRef.current.remove();
     }
    };
  }, []);

  return (
    <ReactP5Wrapper
      sketch={(p5) => {
        p5InstancRef.current = p5;
        sketch(p5);
      }}
    />
  );
};

Package Version: 3.1.0

[jamesrweb]

Hi there @fredrikelmegren!

Thanks for the detailed issue you've raised. I believe I know what the issue is but I want to test my hypothesis locally first before I go into detail.

I'll do my best to come back to you by Thursday with my findings! If it is what I think it is, I'll be able to release the solution on the weekend but if it's not, we'll cross that bridge when we get there!

Thanks again for raising the issue and I'll come back to you asap! 🍀

[jamesrweb]

@fredrikelmegren please take a look at #145. I tested locally and I think this resolves the issue but would be good if you can verify / eye over it still! 😄

@yevdyko please wait for @fredrikelmegren to add a review to the PR or to comment back here with feedback before merging but feel free to add your own review in the meantime since there are other relevant changes worth your consideration nevertheless! 🙏🏻

[jhrtn]

Just noticed this issue in my app as logs continue to be called after the component has been unmounted.

Is it right that the fix in #145 has not yet been rolled out in a release (3.1.0 is the latest)? Is there a timeline when this could happen?

Many thanks

[jamesrweb]

@jhrtn I'll release the next version with the fix for this on Saturday 😊!

[jamesrweb]

@jhrtn and @bsaphier version 3.2.0 has been released and an announcement has been generated accordingly. Open a new issue if something looks off and otherwise, have fun with the new release 👍🏻!

Update: Version 3.2.1 had to be release to fix a release issue introduced in 3.2.0 where the dist files were not correctly published as expected.

[jhrtn]

Thanks for the update. Unfortunately the problem appears to persist, unless I'm missing something about how to correctly unmount and cleanup sketches?

I've created a minimal example on Codesandbox here:

https://codesandbox.io/s/sleepy-roman-7xeimf?file=/src/App.js

If you open the console and click the button to mount the sketch you'll see the setup is called with timestamp, then draw is called every 60 frames with that same timestamp.

If you press the button again to unmount the sketch the component is unmounted, but the logs continue. If you then mount the sketch again a new instance is created, the new sketch calls console.log as the old one continues to as well. It's very easy to crash the browser by doing this multiple times and having many sketches running concurrently, so be careful!

I've downloaded the source from github and can replicate the same behaviour when unmounting the sketch. Swapping the sketches does appear to free up the resources though.

[jamesrweb]

@jhrtn I'll take a look at this ASAP and come back to you. I have a feeling this is something with the use case but want to validate.