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

Create HOC to load content asynchronously #16

Closed
jeanlescure opened this Issue Oct 25, 2018 · 2 comments

Comments

Projects
None yet
1 participant
@jeanlescure
Copy link
Member

jeanlescure commented Oct 25, 2018

I mentioned wanting to do this on #8 and now I'm bumping it up in priority since I notice that it will be used to enhance most typography components and I refuse to have to go in and refactor all of those components because of treating this awesome feature as an after-thought.

The gist of it all is that I want the ability to pass 3 distinct props to any component I want to be able to load content asynchronously into:

  • contentPromise (Promise)
  • onContentLoaded (Function)
  • onContentLoadError (Function)

The onContentLoaded and onContentLoadError props should have default behaviors so that only contentPromise is truly needed in order to make a component . Simplicity and out-of-the-box error handling are key to making this feature truly useful.

@jeanlescure jeanlescure self-assigned this Oct 25, 2018

@jeanlescure

This comment has been minimized.

Copy link
Member

jeanlescure commented Oct 25, 2018

The HOC will be children-centric since it should have as broad a spectrum as possible.

I'm going to name it withAsyncChildren and it will receive 4 arguments:

  • WrappedComponent
  • loadChildrenPromise
  • onChildrenLoaded
  • onChildrenLoadError

Doing it this way also has some cool implications, like the fact that we can also pass JSX markup within the asynchronous response.

Which now that I think about it, makes me wonder, what if I return an async component to an async component? 🤯

@jeanlescure

This comment has been minimized.

Copy link
Member

jeanlescure commented Oct 25, 2018

As I was testing I encountered a warning about memory leaks due to setting state after a component is unmounted.

Basically it had to do with re-rendering and promises not completing until after the re-render. These are orphan promises, very bad juju.

Now, ES6 promises do not have a native way to cancel that's thoroughly supported, but I found the following article which inspired how I ended up implementing a way to abort promises both manually as well as on the componentWillUnmount:

https://blog.bloomca.me/2017/12/04/how-to-cancel-your-promise.html

The final implementation is the following, just follow the aborted class property boolean to see how it is used:

const withAsyncChildren = (
  WrappedComponent: any,
  loadChildrenPromise: Promise<any>,
  onChildrenLoaded: Function = onChildrenLoadedDefault,
  onChildrenLoadError: Function = onChildrenLoadErrorDefault,
) => {
  return class extends Component<Props, State> {
    state = {
      asyncChildren: null,
      asyncError: null,
    };

    aborted: boolean = false;

    componentWillMount() {
      if (this.props.abort) {
        this.aborted = true;
      }
    }

    componentDidMount() {
      loadChildrenPromise
        .then((asyncChildren: any) => {
          if (!this.aborted) {
            this.setState({asyncChildren: onChildrenLoaded(asyncChildren)});
          }
        })
        .catch((error: Error) => {
          if (!this.aborted) {
            this.setState({asyncError: onChildrenLoadError(error)});
          }
        });
    }

    componentWillReceiveProps(nextProps: Props) {
      if (nextProps.abort) {
        this.aborted = true;
      }
    }

    componentWillUnmount() {
      this.aborted = true;
    }

    // ...

That was a close call...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment