Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

Unmount interactive blocks before removing them from the DOM #42

Closed
luisherranz opened this issue Jul 14, 2022 · 3 comments · Fixed by #47
Closed

Unmount interactive blocks before removing them from the DOM #42

luisherranz opened this issue Jul 14, 2022 · 3 comments · Fixed by #47
Labels
bug Something isn't working

Comments

@luisherranz
Copy link
Member

luisherranz commented Jul 14, 2022

While working on the React Context API, @c4rl0sbr4v0 discovered that with the current implementation, the unmount callbacks (mainly returns of useEffect) won't run when an interactive block is removed from the DOM because a parent interactive block stops showing children. This is normal because they are independent React apps, but can lead to memory leaks and inconsistencies.

I've taken a look at Astro, and it doesn't support this. I couldn't find any reference to this problem in their repository either. I haven't looked into other island-based frameworks.

I've been playing around with this, and the callback of useLayoutEffect runs before the DOM nodes are removed, so maybe we could use that to unmount the children interactive blocks before they are destroyed.

const Children = ({ value, providedContext }) => {
  const ref = useRef(null);

  useLayoutEffect(
    () => () => {
      ref.current
        .querySelectorAll("gutenberg-interactive-block")
        .map((node) => {
          // These interactive blocks are about to be destroyed. We can send them a
          // signal so they can switch to `null` internally and force React to run
          // their unmount callbacks.
          const setIsMounted = isMountedMap.get(node);
          setIsMounted(false);
        });
    },
    []
  );

  return (
    <gutenberg-inner-blocks
      ref={ref}
      suppressHydrationWarning={true}
      dangerouslySetInnerHTML={{ __html: value }}
    />
  );
};

The other part is to switch to null if a node receives that signal. We can use a similar approach to the one we used in the React Context.

const InteractiveBlock = () => {
  const [isMounted, setIsMounted] = useState(true);

  useEffect(() => {
    // Expose the setIsMounted so it can be accessed by the parent.
    isMountedMap.set(this, setIsMounted);
  })

  return isMounted ? <Comp ... /> : null;
};

hydrate(InteractiveBlock, this);

I'm not 100% confident that React will successfully run all the callbacks with this approach, but I don't consider this a priority, especially considering that even Astro doesn't have a solution for this. It'd be great to keep an eye on it, though.

EDIT: Client-side navigations could be another source of external unmounting. There could be more. Maybe the API should be part of the HTML nodes? Something like Bento Component APIs:

Bento:

const accordion = document.querySelector("#myAccordion");
await customElements.whenDefined("bento-accordion");
const api = await accordion.getApi();

// set up button actions
document.querySelector("#button1").onclick = () => {
  api.toggle();
};
document.querySelector("#button2").onclick = () => {
  api.toggle("section1");
};

Interactive Blocks:

ref.current.querySelectorAll("gutenberg-interactive-block").map((block) => {
  block.unmount();
});

It could be nice 🙂

EDIT 2: I'm not super familiar with the Mutation Observer, but if it can be used to execute code right before a <gutenberg-interactive-block> is about to be removed, it could be a more robust solution.

@DAreRodz
Copy link
Collaborator

DAreRodz commented Aug 2, 2022

I haven't tested this yet, but maybe we can simply use unmountComponentAtNode(this) inside disconnectedCallback(), the callback that runs when a Web Component is unmounted? 🤔

@DAreRodz
Copy link
Collaborator

DAreRodz commented Aug 2, 2022

Tested ― And it seems to work just fine. 👌 I've recorded a video showing it. I'll open a PR tomorrow.

TEST – block-hydration-experiments - 3 August 2022 - Watch Video

@luisherranz
Copy link
Member Author

OMG!! That is so cool 😄👏

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants