Migrate usePromise from react-use

[jpwallace22]

What new hook does?

Resolves promise only while the component is mounted.

I ran into a problem with testing. Maybe someone can help me out with it. More or less, I need to test that when unmounted, a promise is not returned. However, not returning a promise will break the test due to timeout. This is the test I was using:

test('should not return if unmounted', async () => {
    const { result, unmount } = renderHook(() => usePromise());
    unmount();
    await expect(result.current(resolves)).resolves.toBeUndefined();
  });

And I tried about a million variations. I AM new to writing tests, so I might be missing something simple. I figured instead of spinning wheels on this I would get a PR up and let yall chime in.

Cheers

Checklist

• Have you read contribution guideline?
• Does the code have comments in hard-to-understand areas?
• Is there an existing issue for this PR?
  • link issue here
• Have the files been linted and formatted?
• Have the docs been updated?
• Have the tests been added to cover new hook?
• Have you run the tests locally to confirm they pass?

[ArttuOll]

Thanks for the PR again!

Before reviewing this, I want to have a discussion in this repo about checking the isMounted status of a component and also the usage of the useSafeState hook. Personally I think checking the mounted state of a component is an anti-pattern and should be avoided. Some discussion happened already in #959 .

Before that, however, I have a couple of other things on my todo-list, so reviewing this might get delayed a bit (other maintainers are free to do it). Thank you for your patience.

[xobotyi]

To be honest - I'm not able to imagine worthy usecases of such hook

[ArttuOll]

To be honest - I'm not able to imagine worthy usecases of such hook

Speaking of which, I had a thought the other day: should we start requiring every pull request submitting a new hook to also include a viable use-case in the PR description? There are some pretty weird hooks in react-use that I can't imagine any possible use cases for. This requirement would ensure that there is at least one use-case and would also provide grounds for further discussion.

[xobotyi]

I've been planning to revisit PR and issues templates @ArttuOll in case you feel capable of making proposal - feel free to make a PR

[jpwallace22]

To be honest - I'm not able to imagine worthy usecases of such hook

I see the point there. I would also like to request that you go through the list of requested hooks to be moved over from react-use at #33 and purge the ones you don't want.

I just wanted to help out and saw the list requesting a bunch of hooks to be moved over with a 'Help Wanted" tag so I started picking some out from the list. It would really some a lot of time if yall just removed the ones you don't want before hand. Then people like me wont write code that nobody needs and it would probably cut down on the amount of hooks you don't want popping up as well.
@ArttuOll @ArttuOll

[ArttuOll]

To be honest - I'm not able to imagine worthy usecases of such hook

Speaking of which, I had a thought the other day: should we start requiring every pull request submitting a new hook to also include a viable use-case in the PR description? There are some pretty weird hooks in react-use that I can't imagine any possible use cases for. This requirement would ensure that there is at least one use-case and would also provide grounds for further discussion.

To be honest - I'm not able to imagine worthy usecases of such hook

I see the point there. I would also like to request that you go through the list of requested hooks to be moved over from react-use at #33 and purge the ones you don't want.

I just wanted to help out and saw the list requesting a bunch of hooks to be moved over with a 'Help Wanted" tag so I started picking some out from the list. It would really some a lot of time if yall just removed the ones you don't want before hand. Then people like me wont write code that nobody needs and it would probably cut down on the amount of hooks you don't want popping up as well. @ArttuOll

Agreed. Sorry for the trouble. The list has been filtered already a couple of times. We should do it again.

Also, I will redo the PR template today and add a mention that it would be good to ask if a hook is needed before writing it. I will also add this to the contribution guide.

[jpwallace22]

@ArttuOll If you read #33 and the migrating-from-react-use.story.mdx it still looks like usePromise would be wanted. Could you cross it off at #33 or add something to the storybook about not wanting such hooks? It is very confusing.

[ArttuOll]

@ArttuOll If you read #33 and the migrating-from-react-use.story.mdx it still looks like usePromise would be wanted. Could you cross it off at #33 or add something to the storybook about not wanting such hooks? It is very confusing.

Absolutely. I'm planning to go through the list again soon when I have time. After all, we are not getting paid to maintain this library.