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

Remove result.error and throw errors instead #646

Open
mpeyper opened this issue Jul 6, 2021 · 0 comments
Open

Remove result.error and throw errors instead #646

mpeyper opened this issue Jul 6, 2021 · 0 comments
Labels
BREAKING CHANGE This change will require a major version bump enhancement New feature or request request for comment Discussion about changes to the library

Comments

@mpeyper
Copy link
Member

mpeyper commented Jul 6, 2021

Describe the feature you'd like:

It's been discussed a lot in other issues, but never formally proposed, to remove the result.error property and instead simply allow rendering errors be throw out of the render calls and cause the async utils to reject. The current functionality is, IMO, not very intuitive and has resulted in issues being raised and some weird work arounds.

Suggested implementation:

Ideally, the implementation would remove the ErrorBoundary we use now to catch the errors, however I am concerned that errors thrown in useEffect will not be thrown as expected without it. Although I haven't tested this, it was required for the current implementation as previously, they were not catchable by wrapping render with a try/catch.

Describe alternatives you've considered:

The only alternative I've considered is leaving it as is. I'm happy to hear other's thoughts and ideas for alternative.

Teachability, Documentation, Adoption, Migration Strategy:

I think teachability will be easier than what we have now. We could update the docs to show examples using the jest error handling features.

The migration would need to be a breaking change (I think). I think the impact on most people will be minimal as they generally don't test error cases, but those that do would need to update their tests. We should provider migration docs at the very least, but we could consider a code mod as it should be a pretty straight forward conversion in most cases.

@mpeyper mpeyper added enhancement New feature or request request for comment Discussion about changes to the library BREAKING CHANGE This change will require a major version bump labels Jul 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE This change will require a major version bump enhancement New feature or request request for comment Discussion about changes to the library
Projects
None yet
Development

No branches or pull requests

1 participant