Skip to content

Refactor: JsErrorHandler: Rename JsErrorHandlingFunc -> OnJsError#43985

Closed
RSNara wants to merge 7 commits intofacebook:mainfrom
RSNara:export-D55563580
Closed

Refactor: JsErrorHandler: Rename JsErrorHandlingFunc -> OnJsError#43985
RSNara wants to merge 7 commits intofacebook:mainfrom
RSNara:export-D55563580

Conversation

@RSNara
Copy link
Contributor

@RSNara RSNara commented Apr 8, 2024

Summary:
This is just personal preference.

The name "OnJsError" makes the intent of the abstraction clear: an instance of OnJsError is a function that gets called when a js error is caught.

The name "JsErrorHandlingFunc" is not as good.

Changelog: [General][Breaking] - JsErrorHandler: Rename JsErrorHandlingFunc to OnJsError

Reviewed By: christophpurrer

Differential Revision: D55563580

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Apr 8, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55563580

RSNara added 7 commits April 8, 2024 12:32
…acebook#43952)

Summary:

getRuntimeScheduler() allows things to schedule work on the js thread by bypassing main bundle buffering.

This is unsafe: almost everything should be using the buffered runtime executor, unless it sets up bindings used in the main bundle.

I filed a task for the investigation to see if there's any problems. And added it to the code in this diff.

Changelog: [Internal]

Reviewed By: cipolleschi

Differential Revision: D55547899
…3953)

Summary:

RuntimeScheduler's ErrorUtils.h is redundant.

Let's just remove it.

Changelog: [Internal]

Reviewed By: cipolleschi

Differential Revision: D55547905
…dantly (facebook#43954)

Summary:

Now, all the defaulting is in RuntimeScheduler.h.

Changelog: [Internal]

Reviewed By: cipolleschi

Differential Revision: D55547900
…ed_ptr (facebook#43955)

Summary:

Just makes it easier to pass around JsErrorHandler.

We'll need this in D55547897, when we start storing the "has fataled" boolean inside the JsErrorHandler.

Changelog: [internal]

Reviewed By: cipolleschi

Differential Revision: D55547898
…orHandler (facebook#43956)

Summary:

I think we should try to centralize all things js error handling related inside JsErrorHandler. So, I moved this bool into JsErrorHandler.

This makes ReactInstance easier to understand: it removes one member variable from ReactInstance.

Changelog: [Internal]

Reviewed By: cipolleschi

Differential Revision: D55547897
…alError (facebook#43957)

Summary:

Right now, JsErrorHandler is only used to handle fatal exceptions.

So, let's just scope handleJsError down to handleFatalError.

Changelog: [General][Breaking] - JsErrorHandler: Rename handleJsError to handleFatalError

Reviewed By: cortinico

Differential Revision: D55547901
…cebook#43985)

Summary:

This is just personal preference.

The name "OnJsError" makes the intent of the abstraction clear: an instance of OnJsError is a function that gets called when a js error is caught.

The name "JsErrorHandlingFunc" is not as good.

Changelog: [General][Breaking] - JsErrorHandler: Rename JsErrorHandlingFunc to OnJsError

Reviewed By: christophpurrer

Differential Revision: D55563580
@RSNara RSNara force-pushed the export-D55563580 branch from e78edd7 to a4fc365 Compare April 8, 2024 19:33
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55563580

RSNara added a commit to RSNara/react-native that referenced this pull request Apr 8, 2024
…sError (facebook#43985)

Summary:

This is just personal preference.

The name "OnJsError" makes the intent of the abstraction clear: an instance of OnJsError is a function that gets called when a js error is caught.

The name "JsErrorHandlingFunc" is not as good.

Changelog: [General][Breaking] - JsErrorHandler: Rename JsErrorHandlingFunc to OnJsError

Reviewed By: christophpurrer

Differential Revision: D55563580
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 19,217,880 +3
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,594,618 -13
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: f77d028
Branch: main

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Apr 9, 2024
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 2e3f226.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants