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

Promises with Hooks as proposed #14007

Closed
wesleycho opened this issue Oct 28, 2018 · 1 comment
Closed

Promises with Hooks as proposed #14007

wesleycho opened this issue Oct 28, 2018 · 1 comment

Comments

@wesleycho
Copy link

Do you want to request a feature or report a bug?

Feature request? This is more to kickstart a discussion.

What is the current behavior?

With the current proposal, with any promise-based function call, useEffect must be used something like so

function MyComponent(props) {
  const [state, setState] = useState(0);
  let unmounted = false;
  useEffect(() => {
    myXhrRequest()
      .then(data => {
        if (!unmounted) {
          setState(data);
        }
      });
    return () => {
      unmounted = true;
    };
  });
  ...
}

This is a fairly awkward pattern for promises, as one needs to add an extra check for mounted state potentially quite divorced from the unmount logic. It should be noted that for subscription-based patterns like with rxjs this is fine, but for promises this stinks and is a bit confusing - this would also get repetitive if there are multiple requests involved.

Ideally the useEffect should make it easier to clean up asynchronous code without imposing on the pattern a user would to carry out async behavior. One manner that could remove the need for the confusing return function is to provide a isMounted helper function as an argument for the function passed into useEffect that returns whether the current component is mounted. It does not solve the verbosity completely, which might be outside the scope of React, but it removes the need for extra boilerplate that arguably hurts the readability.

With this proposal, the useEffect part could be rewritten like this

  useEffect(({ isMounted }) => {
    myXhrRequest()
      .then(data => {
        if (isMounted()) {
          setState(data);
        }
      });
  });

Just some thoughts anyhow, I would understand if this proposal is rejected.

@gaearon
Copy link
Collaborator

gaearon commented Oct 28, 2018

Please consolidate feedback here: reactjs/rfcs#68
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants