Expose ability for apps to provide their own RedBox implementation #4786
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dannyvv
reviewed
May 4, 2020
| } | ||
|
|
||
| [webhosthidden] | ||
| interface IRedBoxHandler |
Member
There was a problem hiding this comment.
The previous implementation, now DefaultRedBoxHandler has the comment:
This class is implemented such that the methods on IRedBoxHandler are thread safe
If this is a requirement from the interface should we document this, or is that implicit in winrt?
Contributor
Author
There was a problem hiding this comment.
It should probably be documented. We're pretty light on the documentation as a whole right now. @kikisaints - any idea the best way to track that?
asklar
suggested changes
May 4, 2020
| Native, | ||
| }; | ||
|
|
||
| [webhosthidden] interface IErrorFrameInfo { |
Member
There was a problem hiding this comment.
we could use the native WinRT types for exception/error management that are already understood by debuggers: ILanguageExceptionStackBackTrace and IRestrictedErrorInfo
Contributor
Author
There was a problem hiding this comment.
I dont think these types align with the Infomation that react-native is providing. RN is already doing some of the logic that debuggers would traditionally do, such as communicating with the bundle server and doing sourcemap lookups to determine the real stack info. The frame information is empty for native errors.
ghost
added
Needs: Author Feedback
vmoroz
reviewed
May 5, 2020
vmoroz
reviewed
May 5, 2020
Member
vmoroz
reviewed
May 5, 2020
vmoroz
reviewed
May 5, 2020
vmoroz
reviewed
May 5, 2020
vmoroz
reviewed
May 5, 2020
vmoroz
reviewed
May 5, 2020
vmoroz
reviewed
May 8, 2020
| String DebugBundlePath { get; set; }; | ||
| String BundleRootPath { get; set; }; | ||
| UInt16 DebuggerPort { get; set; }; | ||
| Microsoft.ReactNative.IRedBoxHandler RedBoxHandler { get; set; }; |
Member
There was a problem hiding this comment.
Microsoft.ReactNative. [](start = 4, length = 22)
Not needed
acoates-ms
added
Backport to 0.62
AutoMerge
|
Hello @acoates-ms! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
asklar
reviewed
May 8, 2020
asklar
reviewed
May 8, 2020
| CreateErrorInfo(info), static_cast<Mso::React::ErrorType>(static_cast<uint32_t>(type))); | ||
| } | ||
|
|
||
| bool IsDevSupportEnabled() noexcept { |
Member
There was a problem hiding this comment.
is it possible to express const method in MIDL? if so consider marking this one as such too
asklar
reviewed
May 8, 2020
| return; | ||
| m_showingRedBox = true; | ||
|
|
||
| Mso::DispatchQueue::MainUIQueue().Post( |
Member
There was a problem hiding this comment.
I'm curious how Mso determines which one is the MainUIQueue - i.e. how is this going to work in xaml islands?
It might make more sense to have the react host / instance expose the dispatch queue for that island
Member
There was a problem hiding this comment.
Yes, this is the plan. Currently we use the MainView CoreDispatcher and it fails for the XAML islands.
asklar
approved these changes
May 8, 2020
Member
asklar
left a comment
asklar
left a comment
There was a problem hiding this comment.
my only concern here is the use of Mso::DispatchQueue::MainUIQueue which doesn't allow us to direct to the right DispatcherQueue always? Signing off with the understanding that you've validated this will work fine :)
Contributor
Author
ghost
merged commit
NickGerleman
pushed a commit
to NickGerleman/react-native-windows
that referenced
this pull request
May 14, 2020
…icrosoft#4786) * Expose ability for apps to provide their own RedBox implementation * Change files * minor changes * Code review feedback * minor fix * format fixes * minor change * minor fix * minor fix * minor fix
NickGerleman
pushed a commit
to NickGerleman/react-native-windows
that referenced
this pull request
May 14, 2020
…icrosoft#4786) * Expose ability for apps to provide their own RedBox implementation * Change files * minor changes * Code review feedback * minor fix * format fixes * minor change * minor fix * minor fix * minor fix
NickGerleman
pushed a commit
that referenced
this pull request
May 14, 2020
…4786) * Expose ability for apps to provide their own RedBox implementation * Change files * minor changes * Code review feedback * minor fix * format fixes * minor change * minor fix * minor fix * minor fix
ghost
pushed a commit
that referenced
this pull request
May 14, 2020
* improve treedump capabilities * Reenable V8 for desktop projects (#4840) * Delay load ChakraCore.dll * Change files * clean up unneeded project references * Overwrite property after React.Cpp.props gets included * Change files * change file * Fix WinUI3 * Reverse the conditional logic * Revert some unnecessary changes. Consolidate the preprocessor defines in the props Co-authored-by: tudorm <[email protected]> * applying package updates ***NO_CI*** * update masters * Improve run_wdio to print out failed tests (#4843) * Improve run_wdio to print out failed tests * Fire onLoad event when a bitmap image is opened (#4750) * Fire onLoad event when a bitmap image is opeed * Change files * add imageFailed * remove firing of onload events in the createImageBrush block so that load events don't fire twice. * Expose ability for apps to provide their own RedBox implementation (#4786) * Expose ability for apps to provide their own RedBox implementation * Change files * minor changes * Code review feedback * minor fix * format fixes * minor change * minor fix * minor fix * minor fix * Bump fp-ts from 2.5.4 to 2.6.0 (#4871) Bumps [fp-ts](https://github.com/gcanti/fp-ts) from 2.5.4 to 2.6.0. - [Release notes](https://github.com/gcanti/fp-ts/releases) - [Changelog](https://github.com/gcanti/fp-ts/blob/master/CHANGELOG.md) - [Commits](gcanti/fp-ts@2.5.4...2.6.0) Signed-off-by: dependabot-preview[bot] <[email protected]> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * applying package updates ***NO_CI*** * Fixed ReactContext copy/move semantic (#4870) * Fixed ReactContext copy/move semantic * Change files * Fixed formatting * Test to show how to use a delegate in ReactPropertyBag * Add ReactNativeHost to Win32 C++/WinRT ABI (#4848) * create MS.ReactNative.IntegrationTests project * RNHost activation succeeds * update * update * Change files * add JS function call test * Change files * fix formatting * submit current state * ReactNativeHost activation test succeeds * Change files * account for property bag addition * sync, address code review feedback * Use spec file for DevSettings NativeModule (#4873) * Use spec file for DevSettings NativeModule * Change files * minor change * Handle HTTP errors in DevSupportManager. (#4880) * Handle HTTP connect exceptions in DevSupportManager * Change files * applying package updates ***NO_CI*** * Allow storing non-WinRT types in ReactPropertyBag (#4884) * Allow storing non-WinRT types in ReactPropertyBag * Change files * Addressed PR feedback * Use DispatcherQueue instead of CoreDispatcher for Mso::DispatchQueue (#4877) * Use DispatcherQueue instead of CoreDispatcher for Mso::DispatchQueue * Change files * Removed 'Thread' from some new API names. * Removed extra 'private' section in AppearanceModule * Temporary: use CoreDispatcher to check the E2E test * Avoid using DispatcherQueue::HasThreadAccess in earlier Windows versions. * RNW dependencies (#4876) * add rnw-dependencies.ps1 * Change files * --sln * run in CI * print * desktop appium is optional * use agent directory * 15GB is a ballpark estimate * optional space * . * support running from web (#4892) * Bump typescript from 3.8.3 to 3.9.2 (#4890) * Bump lerna from 3.20.2 to 3.21.0 (#4889) * BugFix: fix default tabindex stomping over acceptsKeyboardFocus (#4893) * Change files * revert dfc57fc * set default tab index to -1, the hope is to be in line with xbox assumption * xbox doesn't seem to rely on this, changing default back to match xaml default * Update e2e testing doc with CI debugging info (#4897) * add instructions for CI debugging for e2e test app * point e2e readme to e2e testing doc * Update rnw-dependencies (#4894) * don't exit the powershell session and pause for keyboard input if interactive * Change files * enable Switch (fixed 4596) * improve treedump capabilities * update masters * enable Switch (fixed 4596) * protect against exceptions in run_wdio * add another try block * update e2e testing and masters, fixes 4680 * publish wdio report and fix run_wdio typo bug * TreeDump should ignore collapsed object elements in an array * add info about run_wdio Co-authored-by: tudorms <[email protected]> Co-authored-by: tudorm <[email protected]> Co-authored-by: React-Native-Windows Bot <[email protected]> Co-authored-by: lamxdoan <[email protected]> Co-authored-by: Andrew Coates <[email protected]> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Vladimir Morozov <[email protected]> Co-authored-by: Andreas Eulitz <[email protected]> Co-authored-by: Julio César Rocha <[email protected]> Co-authored-by: kmelmon <[email protected]>
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Applications want to be able to tie their own error reporting mechanisms into React-Native.
This change provides a new property to ReactInstanceSettings that allows the app to provide their own implementation of RedBoxHandler, which will get notified of various error types from the react-native instance.
I also provided a ctor for the default RedBoxHandler, which allows apps to pass through to the default implementation as part of their implementation.
Microsoft Reviewers: Open in CodeFlow