-
Notifications
You must be signed in to change notification settings - Fork 24k
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
Ensure C++ default tmmd can access the CxxReactPackage #41673
Conversation
This pull request was exported from Phabricator. Differential Revision: D51550623 |
Summary: ## The Problem The cxxReactPackage property isn't initialized by the time initHybrid was executed. So, initHybrid was being called with null as the cxxReactPackage. Why; 1. The default turbomodule manager delegate receives cxxReactPackage as a constructor param. 2. Java then executes all the constructors of the class hierarchy. This executes initHybrid. 3. Java finally initializes the properties of the derived class: default tmmd.cxxReactPackage (i.e: the parameter to initHybrid). **This is too late** ## The Fix Refactor the code such that hybrid data creation doesn't depend on property initialization: 1. Create a static initHybrid method in default tmmd. 2. Call this static method with the cxxReactPackage, and assign the resultant HybridData to mHybridData (in tmmd). Changelog: [Internal] Reviewed By: javache Differential Revision: D51550623
fe35231
to
4ab1726
Compare
This pull request was exported from Phabricator. Differential Revision: D51550623 |
Summary: ## The Problem The cxxReactPackage property isn't initialized by the time initHybrid was executed. So, initHybrid was being called with null as the cxxReactPackage. Why; 1. The default turbomodule manager delegate receives cxxReactPackage as a constructor param. 2. Java then executes all the constructors of the class hierarchy. This executes initHybrid. 3. Java finally initializes the properties of the derived class: default tmmd.cxxReactPackage (i.e: the parameter to initHybrid). **This is too late** ## The Fix Refactor the code such that hybrid data creation doesn't depend on property initialization: 1. Create a static initHybrid method in default tmmd. 2. Call this static method with the cxxReactPackage, and assign the resultant HybridData to mHybridData (in tmmd). Changelog: [Internal] Reviewed By: javache Differential Revision: D51550623
4ab1726
to
6f27181
Compare
This pull request was exported from Phabricator. Differential Revision: D51550623 |
Summary: ## The Problem The cxxReactPackage property isn't initialized by the time initHybrid was executed. So, initHybrid was being called with null as the cxxReactPackage. Why; 1. The default turbomodule manager delegate receives cxxReactPackage as a constructor param. 2. Java then executes all the constructors of the class hierarchy. This executes initHybrid. 3. Java finally initializes the properties of the derived class: default tmmd.cxxReactPackage (i.e: the parameter to initHybrid). **This is too late** ## The Fix Refactor the code such that hybrid data creation doesn't depend on property initialization: 1. Create a static initHybrid method in default tmmd. 2. Call this static method with the cxxReactPackage, and assign the resultant HybridData to mHybridData (in tmmd). Changelog: [Internal] Reviewed By: javache Differential Revision: D51550623
6f27181
to
3c15365
Compare
This pull request was exported from Phabricator. Differential Revision: D51550623 |
Summary: ## The Problem The cxxReactPackage property isn't initialized by the time initHybrid was executed. So, initHybrid was being called with null as the cxxReactPackage. Why; 1. The default turbomodule manager delegate receives cxxReactPackage as a constructor param. 2. Java then executes all the constructors of the class hierarchy. This executes initHybrid. 3. Java finally initializes the properties of the derived class: default tmmd.cxxReactPackage (i.e: the parameter to initHybrid). **This is too late** ## The Fix Refactor the code such that hybrid data creation doesn't depend on property initialization: 1. Create a static initHybrid method in default tmmd. 2. Call this static method with the cxxReactPackage, and assign the resultant HybridData to mHybridData (in tmmd). Changelog: [Internal] Reviewed By: javache Differential Revision: D51550623
3c15365
to
21fe983
Compare
Summary: FBJni expects the HybridData object to exist on the mHybridData property of the java object. So, we have to call this propery mHybridData. Otherwise, this fbjni class just won't work. Changelog: [Internal] Reviewed By: javache Differential Revision: D51550621
Summary: The problem: The Java runtime couldn't find CatalystCxxReactPackage.initHybrid. Why: I think is because CxxReactPackage loads CatalystCxxReactPackage's so in its constructor. This might be too late to load the derived class's so. So, I just switched derived delegates to load the so in the static initializer (i.e: the recomended approch for so loading): https://www.internalfb.com/code/fbsource/[91c4e41c49ed191ac864250ccaec52c01ddaeccc]/fbandroid/libraries/soloader/java/com/facebook/soloader/SoLoader.java?lines=52-54%2C60-61%2C63-67 This way: 1. The So's are loaded plenty early (the method not found error went away). 2. We don't create our own so loading infra, which complicates this abstraction, and makes it harder to work with, even more. Changelog: [Internal] Reviewed By: javache Differential Revision: D51550622
Summary: ## The Problem The cxxReactPackage property isn't initialized by the time initHybrid was executed. So, initHybrid was being called with null as the cxxReactPackage. Why; 1. The default turbomodule manager delegate receives cxxReactPackage as a constructor param. 2. Java then executes all the constructors of the class hierarchy. This executes initHybrid. 3. Java finally initializes the properties of the derived class: default tmmd.cxxReactPackage (i.e: the parameter to initHybrid). **This is too late** ## The Fix Refactor the code such that hybrid data creation doesn't depend on property initialization: 1. Create a static initHybrid method in default tmmd. 2. Call this static method with the cxxReactPackage, and assign the resultant HybridData to mHybridData (in tmmd). Changelog: [Internal] Reviewed By: javache Differential Revision: D51550623
This pull request was exported from Phabricator. Differential Revision: D51550623 |
Summary: ## The Problem The cxxReactPackage property isn't initialized by the time initHybrid was executed. So, initHybrid was being called with null as the cxxReactPackage. Why; 1. The default turbomodule manager delegate receives cxxReactPackage as a constructor param. 2. Java then executes all the constructors of the class hierarchy. This executes initHybrid. 3. Java finally initializes the properties of the derived class: default tmmd.cxxReactPackage (i.e: the parameter to initHybrid). **This is too late** ## The Fix Refactor the code such that hybrid data creation doesn't depend on property initialization: 1. Create a static initHybrid method in default tmmd. 2. Call this static method with the cxxReactPackage, and assign the resultant HybridData to mHybridData (in tmmd). Changelog: [Internal] Reviewed By: javache Differential Revision: D51550623
21fe983
to
3e13c7b
Compare
This pull request was exported from Phabricator. Differential Revision: D51550623 |
3e13c7b
to
35c3f1b
Compare
This pull request was exported from Phabricator. Differential Revision: D51550623 |
Base commit: a8fc206 |
This pull request has been merged in 129bd9d. |
Summary: Pull Request resolved: facebook#41673 ## The Problem The cxxReactPackage property isn't initialized by the time initHybrid was executed. So, initHybrid was being called with null as the cxxReactPackage. Why; 1. The default turbomodule manager delegate receives cxxReactPackage as a constructor param. 2. Java then executes all the constructors of the class hierarchy. This executes initHybrid. 3. Java finally initializes the properties of the derived class: default tmmd.cxxReactPackage (i.e: the parameter to initHybrid). **This is too late** ## The Fix Refactor the code such that hybrid data creation doesn't depend on property initialization: 1. Create a static initHybrid method in default tmmd. 2. Call this static method with the cxxReactPackage, and assign the resultant HybridData to mHybridData (in tmmd). Changelog: [Internal] Reviewed By: javache Differential Revision: D51550623 fbshipit-source-id: ed2b7587351cfca408cda3c8cef4dcf7547e5f1e
Summary:
The Problem
The cxxReactPackage property isn't initialized by the time initHybrid was executed. So, initHybrid was being called with null as the cxxReactPackage.
Why;
The Fix
Move init hybrid to a static method in default turbomodule manager. Call this static method with the passed in cxxreactpackage. And use the resultant HybridData to initialize the turbomodulemanager delegate hierarchy.
Changelog: [Internal]
Reviewed By: javache
Differential Revision: D51550623