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

Add ReactFiberErrorDialog from React + tests #25671

Closed
wants to merge 1 commit into from

Conversation

motiz88
Copy link
Contributor

@motiz88 motiz88 commented Jul 16, 2019

Summary:
Moves the RN-specific ReactFiberErrorDialog implementation from React to RN for easier iteration. Also adds new unit tests.

This current change is additive, so we're compatible with the current React renderer which still uses ExceptionsManager and not the file added here. After the corresponding React update we can remove ExceptionsManager from the RN private interface entirely.

Differential Revision: D16278938

@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 Jul 16, 2019
@pull-bot
Copy link

pull-bot commented Jul 16, 2019

Messages
📖 📋 Missing Summary - Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.
📖 📋 Missing Test Plan - Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.
📖

📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against e22e50a

Summary:
Pull Request resolved: facebook#25671

Moves the RN-specific `ReactFiberErrorDialog` implementation from React to RN for easier iteration. Also adds new unit tests.

This current change is additive, so we're compatible with the current React renderer which still uses `ExceptionsManager` and not the file added here. After the corresponding React update we can remove `ExceptionsManager` from the RN private interface entirely.

Reviewed By: cpojer

Differential Revision: D16278938

fbshipit-source-id: d2e44ada12f959b8d483a5ca1387c3891a10acbf
@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @motiz88 in a9cab21.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Jul 16, 2019
facebook-github-bot pushed a commit that referenced this pull request Jul 18, 2019
Summary:
#25671 added a shallow unit test for `ReactFiberErrorDialog`, mocking out the (JS) `ExceptionsManager` module. This rewrites that test in terms of `NativeExceptionsManager` instead, so the integration with `ExceptionsManager` is also tested.

Also adds tests for the behaviour of the `framesToPop` and `jsEngine` extended error fields, and for passing a frozen error object (seeing as we potentially mutate the error).

Reviewed By: rickhanlonii

Differential Revision: D16330341

fbshipit-source-id: 0b514d1c8f193a114748739ec31ddb4e06e4d2fd
aleclarson pushed a commit to alloc/react-native-macos that referenced this pull request Nov 3, 2019
Summary:
Pull Request resolved: facebook/react-native#25671

Moves the RN-specific `ReactFiberErrorDialog` implementation from React to RN for easier iteration. Also adds new unit tests.

This current change is additive, so we're compatible with the current React renderer which still uses `ExceptionsManager` and not the file added here. After the corresponding React update we can remove `ExceptionsManager` from the RN private interface entirely.

Reviewed By: cpojer

Differential Revision: D16278938

fbshipit-source-id: 0c2c0c3e65e524e079730ae3b0cc23e0c0bdc5fd
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. Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants