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

Replace catchRender with catchError #1742

Merged
merged 11 commits into from Jul 6, 2019
Merged

Conversation

andrewiggins
Copy link
Member

This PR changes the behavior of the catchError option to enable composing of catchError option hooks (similar to middleware). The initial value of options.catchError is set to catchErrorInComponent. Any code that overrides options.catchError should store the previous value of options.catchError and invoke it if they want to rely on the default implementation of catchError. This pattern allows code to skip the default implementation of if an option hook chooses not to invoke the previous catchError behavior. It also gives options the ability to change what the default implementation is invoked with.

Two new features this pattern enables:

  1. Using catchError to replace catchRender (to implement Suspense)
  2. Adding debug error for "Missing Suspense"

In order to enable using catchError for Suspense, I had to change the behavior the of catchError to take in the VNode that threw error, instead of the first parent. An initial implementation caused some problems in how errors were caught that our tests didn't catch so I modified some tests to cover this scenario. Namely, if a direct child of an error boundary throws, the error boundary should catch that error. The initial implementation was skipping the first parent if a child threw in unmount and refs.

Change:
preact.js: -18 B
compat.js: -10 B

@coveralls
Copy link

coveralls commented Jun 27, 2019

Coverage Status

Coverage increased (+0.1%) to 99.679% when pulling 232d896 on compose-catchError into 984f18d on master.

Copy link
Member

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks amazing, have you actually ever added a byte? 💯 You seem to solve errors and add features while losing bytes that's a talent if you ask me

Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The master golfer has went it's course again 👍 Incredible work like usual 💯

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

Successfully merging this pull request may close these issues.

None yet

4 participants