Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

forgo.unmount() not called after removing element #73

Closed
chronoDave opened this issue Aug 7, 2022 · 6 comments
Closed

forgo.unmount() not called after removing element #73

chronoDave opened this issue Aug 7, 2022 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@chronoDave
Copy link
Contributor

The unmount() function does not call when unmounting a component. Sandbox

import * as forgo from "forgo";

const App = () => {
  const component = new forgo.Component({
    render() {
      return <p>Tooltip</p>;
    }
  });

  component.unmount(() => alert("unmounted!"));

  return component;
};

forgo.mount(<App />, document.getElementById("root"));

// forgo.mount(null, document.getElementById("root"));
document.body.removeChild(document.getElementById("root"));

I'm not entirely sure if either method correctly unmount a Forgo component.

@spiffytech
Copy link
Member

Ah, you're bypassing Forgo's teardown logic by nuking the element with the DOM APIs. Forgo doesn't handle that right now. Supporting it has been on our long-term roadmap.

Let me think on what it would take to implement that.

@chronoDave
Copy link
Contributor Author

What would be the correct way to unmount a component?

@spiffytech
Copy link
Member

spiffytech commented Aug 7, 2022

Right now it's presumed that unmounts happen when a Forgo render detects that a previously-displayed component stops being displayed. That works fine inside the Forgo app, but isn't helpful if something outside Forgo needs to trigger an unmount of the whole app.

Forgo doesn't have a solution in place for that yet because you're the first person we've heard from who has integrated Forgo with other apps (vs greenfield work done entirely in Forgo).

For the moment, a hacky workaround would look like this:

import { rerenderElement } from "forgo-powertoys";

const App = () => {
  const component = new forgo.Component({
    render() {
      return <p>Tooltip</p>;
    }
  });

  component.unmount(() => alert("unmounted!"));

  return component;
};

const AppWrapper<{showTheApp: boolean}> = () => {
  return new forgo.Component<{showTheApp: boolean}>({
    render({showTheApp}) {
      if (!showTheApp) return null;
      return <App />;
    }
  });
};

forgo.mount(<AppWrapper showTheApp={true} />, document.getElementById("root"));

rerenderElement(document.getElementById("root"), {showTheApp: false);
document.body.removeChild(document.getElementById("root"));

Basically you put all the app logic that needs to care about the unmount lifecycle inside a component that you can rerender to unmount all of its decendants.

That's really ugly, but it'll at least get you moving today.


I'm looking into how practical it would be to expose a forgo.unmount(el) function for external logic to call, that would do all the teardown and remove the descendants of el. If that's straightforward, I can get that implemented and published promptly.

Longer term, we'll need to integrate with MutationObserver to allow arbitrary DOM operations on anything Forgo renders. But that'll be a lot more complicated to implement safely, so I don't expect to push that out soon.

@chronoDave
Copy link
Contributor Author

chronoDave commented Aug 7, 2022

Thank you for the quick response, I managed to adapt your example into a small helper. It works great 😄

export default (anchor: HTMLElement, element: forgo.ForgoComponent<any>) => {
  let unmounted = false;
  let unmount: () => void;

  const portal = () => {
    const component = new forgo.Component({
      render() {
        if (unmounted) return null;
        return element;
      }
    });

    unmount = () => {
      unmounted = true;
      component.update();
    };

    return component;
  };

  anchor.appendChild(forgo.render(forgo.createElement(portal, {})).node);
  return () => unmount();
};

@spiffytech spiffytech self-assigned this Aug 9, 2022
@spiffytech spiffytech added the bug Something isn't working label Aug 9, 2022
@spiffytech
Copy link
Member

I've got a PR up that adds a forgo.unmount() function.

I'll want to update the docs separately - I need to figure out what the right way to unmount on page exit is, and at a quick glance all I see are browser events labeled "deprecated" and "don't do this". I'll have to check what other frameworks do / recommend.

spiffytech added a commit that referenced this issue Aug 29, 2022
@spiffytech
Copy link
Member

This has been merged and will be released in v3.2.1.

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

No branches or pull requests

2 participants