Skip to content

Removing undocumented BaseComponent.onError handler.#3956

Merged
dzearing merged 3 commits into
microsoft:masterfrom
dzearing:fix-error-handling
Feb 14, 2018
Merged

Removing undocumented BaseComponent.onError handler.#3956
dzearing merged 3 commits into
microsoft:masterfrom
dzearing:fix-error-handling

Conversation

@dzearing

Copy link
Copy Markdown
Member

The onError handler in BaseComponent is not needed, and is taking up extra bytes. Removing in favor of letting exceptions flow to React.

This gives the caller the opportunity to take advantage of the official error boundary support in React 16.

(obj as any)[methodName] = function (): any {
let retVal;

try {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suppose this try-catch is the core meaning of this _makeSafe function. Now, after removing this try-catch logic, do we still need this _makeSafe function?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, though it might be more aptly named to something more clear. I was trying to minimize the change as much as possible.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That sounds good to me!

@dzearing dzearing merged commit 69854e9 into microsoft:master Feb 14, 2018
@dzearing dzearing deleted the fix-error-handling branch February 14, 2018 00:44
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants