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 GetFacebookReactInstance #3782

Merged
90 commits merged into from
Jan 9, 2020
Merged

Add GetFacebookReactInstance #3782

90 commits merged into from
Jan 9, 2020

Conversation

ZihanChen-MSFT
Copy link
Contributor

@ZihanChen-MSFT ZihanChen-MSFT commented Dec 16, 2019

To get facebook::react::Instance from uwp::Instance

The reason not to do this directly in client application is, it requires the client application to include too many header files that are not designed to be included out side of ReactUWP, and it requires the client application to do too many configuration in its vcxproj file, which is not practical for consumers.

An uwp::Instance is needed for turbo module users for registering native module implementations.

Microsoft Reviewers: Open in CodeFlow

ZihanChen-MSFT and others added 30 commits December 9, 2019 16:39
* Update E2ETest to use ReactApplication

* Minor update

* Remove generating pch.pch

* Change files

* Shrink pch file size for Microsfot.ReactNative

* Revert "Remove generating pch.pch"

This reverts commit 39286c8.

* fix build

* Update Timeout
Documentation update based on #2852 completion
* Re-aligned SampleAppCPP project to match the others, #3728
* Updated all projects to 2.0.190730.2
update instructions on how to install submodules in the case where you started out working on vnext then switched to current.
…1.6 (#3729)

* unify hermes and v8jsi version

* Change files

* use HERMES_Package and V8_Package

* remove them from reactuwp project
* reduce build time

* changes

* fix pipeline failure

* use Add-AppxPackage

* E2E test still use windows-2019 image

* force install vs dependencies on vs2019 image for E2E test

* parameters.forceVSDependencies

* add ../../.ado/variables/vs2017.yml

* Revert "add ../../.ado/variables/vs2017.yml"

This reverts commit b829251.

* revert and force

* Fix pipeline error
* Add react-native-win32 package
* Miscellaneous fixes in ETW tracing and Systrace

* Miscellaneous fixes in ETW tracing and Systrace - Adding missing files

* Submitting the ETW schema resouce dll and the register script

* Change files
…Reader, JSValue, and IJSValueWriter (#3760)

* Merged implementation of strongly typed value serialization and deserialization using IJSValueReader, JSValue, and IJSValueWriter

* Change files

* Updated CLI template for C++ native modules
* Update to [email protected]

* Change files

* Change files
* Fix toggle debugger setting issue with ReactApplication
NickGerleman and others added 17 commits January 7, 2020 13:46
#3813)

* Export ability to query native module names

This is needed for testability (intenral CR using it out now). It's not ideal to add more exports, but we will always have to have some between instance interfaces.

* Change files

* Fix x86 mangeled name
* Changed Microsoft.ReactNative to be independent from ReactUWP

* Removed ReactUWP project from the ReactUWPTestApp to reduce compiled code size.

* Removed commented code from pch.h

* Moved WindowsBrushExample.windows.tsx to fix RNTester bundle building

* Updated TreeDumps to fix test cases.

* An attempt to fix E2ETest

* Changed ViewPanel naemspace in the E2ETest tree dumps

* Changed namespace for ViewPanel in other E2ETest tree dumps
* add InjectBundleContent target

* Change files

* format
…#3829)

* Call StartAnimatiom on m_scaleCombined for ScaleX / ScaleY animations

There was a copy-paste error previously that started m_translationCombined instead.

* Change files
* Remove remaining need for fork of RN for win32 JS

* Change files

* Build fix

* Change files

* Enable flow type checking in win32

* Fix build
@jonthysell jonthysell requested a review from vmoroz January 7, 2020 22:32
const winrt::react::uwp::Instance &uwpInstance) {
auto abiInstance = reinterpret_cast<ABI::react::uwp::Instance *>(winrt::get_abi(uwpInstance));
auto reactInstance = abiInstance->getInstance();
assert(reactInstance);
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I prefer if-else over assert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's fine, it can't fail, or it is a bug. So I prefer to crash.

@ZihanChen-MSFT ZihanChen-MSFT added the AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) label Jan 9, 2020
@ghost
Copy link

ghost commented Jan 9, 2020

Hello @ZihanChen-MSFT!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 1f8aea3 into microsoft:master Jan 9, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity)
Projects
None yet
Development

Successfully merging this pull request may close these issues.