BUGFIX: Fix handleReject

[kentcdodds]

Description

By simply returning the error it changed the promise chain from rejected to resolved which was causing me issues.

Breaking changes

Does this include any (potentially) breaking API changes?

Only if people were relying on the bug.

Checklist

Make sure you check all the boxes. You can omit items that are not applicable.

• Implementation for both <Async> and useAsync()
• Added / updated the unit tests
• Added / updated the documentation N/A
• Updated the PropTypes N/A
• Updated the TypeScript type definitions N/A

[kentcdodds]

Sorry, I don't know what busted or have time ATM to fix it 😬

If you have a second to get this fix released (even if you treat this as a bug report rather than a PR and push the fix yourself that's fine) that would be wonderful.

[ghengeveld]

Thanks for your feedback and trying to provide a fix along with it! This was originally done explicitly for compatibility with React Final Form (which expects a resolved promise).

I'm on board with the change, as I don't want to break the Promise contract by not re-throwing. I do want to tread carefully when dealing with a breaking API change like this, so it won't be a quick fix. There's tests failing that I need to fix. Also the Async component API needs to be changed in tandem.

[kentcdodds]

Understood. I'll publish a temporary fork for my workshop then. Thanks for getting back to me so quickly!

Pretty weird that Final Form expects a resolved promise. Should be easy for people who are using that lib to change it from rejected to resolved themselves 👍

[ghengeveld]

Merged it to a separate branch. Will handle it from here 👍
It's going to be a major release as it's a breaking API change.

[kentcdodds]

Hey @ghengeveld, actually if you could do me another huge favor I would be super appreciative.

Could you publish this change as a beta release? For my workshop, I'd much rather have people import {useAsync} from 'react-async' than import {useAsync} from '@kentcdodds/react-async-6.2.1-temp-fix@6.2.1' 😅

Even if the dist tag is something weird, that'd just make things nicer for me and the attendees if that's ok :)

[ghengeveld]

Sure, I'll get you a pre-release shortly.

[kentcdodds]

You're the best! Thanks!

[ghengeveld]

Actually this involves a bit more work, as re-throwing will cause an uncaught promise rejection in the Fulfilled component.

Anyway, you can install react-async@kcd for now. It's got some tests skipped and you probably can't use <Fulfilled>.

[kentcdodds]

Ok, sounds good. Thank you so much for giving me some of your time today @ghengeveld! And I love react-async 💯

[ghengeveld]

No problem! Good luck with your workshop.

[Tomino2112]

Hi, can I help somehow to get this over the line?

[ghengeveld]

@Tomino2112 Yeah, please do! Here's what has to happen:

• Revert the change that propagates the rejection, because it breaks the Fulfilled component. Or even better, start fresh from the master branch.
• Expose a new promise property from useAsync and in the render props of <Async>, which rejects or resolves along with the internal promise. I would just create a new Promise(...) along with new window.AbortController() and keep it as a ref. It can be resolved/rejected from handleResolve and handleReject.
• The new prop must be tested and documented.

[ghengeveld]

@all-contributors please add @kentcdodds for code

[allcontributors]

@ghengeveld

I've put up a pull request to add @kentcdodds! 🎉