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

Passing an instance of "ReactPage" should not be required (Windows) #943

Closed
pre10der89 opened this issue Jul 28, 2017 · 5 comments
Closed

Comments

@pre10der89
Copy link

Description

This issue is strictly related to the Windows implementation (UWP and Net46)

The CodePushReactPackage and CodePushNativeModule require access to an instance of Type "ReactPage" The instance is used to check the "UseDeveloperSupport" flag and to obtain a reference to the IReactInstanceManager, which is used to force a reload of the bundle.

According to @rozele the "ReactPage" was only meant to be a template for jump-starting application development on ReactNativeWindows. It was always intended that a developer be able to write their own ReactNativeWindow bootstrapping implementation to replace "ReactPage".

While, it is possible to write a custom "ReactPage" implementation, it is not possible to do so and use react-native-code-push because of the requirement to pass in type "ReactPage"

Additional Information

The ReactPage is used by the CodePushNativeModule to obtain a reference to internal objects of ReactPage that allow for a Bundle reload... This is done by using NonPublic reflection is to obtain the reference to IReactInstanceManager inside ReactPage, which is then cast to type "ReactInstanceManager", which is then used to get access to the NonPublic field _jsBundleFile for the purposes of replacing the bundle value. The use of NonPublic reflection and the assumption of the concrete type behind IReactInstanceManager is fragile and assumes knowledge of the ReactNativeWindows code.

@rozele
Copy link
Contributor

rozele commented Jul 28, 2017

@pre10der89 thanks for filing this, but I think we need to provide a reasonable alternative to grab the ReactInstanceManager from. We'll probably need an abstraction like ReactApplication in ReactAndroid (https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/ReactApplication.java)

@pre10der89
Copy link
Author

@rozele I agree 100%. I believe that the need for ReactApplication/ReactNativeHost is evident and hopefully, we can work on getting that implemented and getting the code push module updated. I believe there is also another short-coming with the code-push "reload" bundle requirement that could require steps beyond the ReactApplication implementation.

@max-mironov max-mironov added the UWP label Aug 2, 2017
@sergey-akhalkov
Copy link
Contributor

Hi @pre10der89, could you please confirm if your enhancement request has been implemented here #1051 ?

@max-mironov
Copy link
Contributor

@pre10der89, @rozele - hey guys, could you please take a look at this and let us know if the issue is still actual with the latest release of RN Windows 0.50+ or not?

@max-mironov
Copy link
Contributor

Hey guys, I believe we can close the issue as https://github.com/Microsoft/react-native-code-push/releases/tag/v5.2.0-beta has been released with #1051. Please let us know if any concerns.

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

No branches or pull requests

4 participants