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

Remove fork in TurboModuleBinding.cpp #12731

Closed
chiaramooney opened this issue Feb 13, 2024 · 1 comment · Fixed by #12764
Closed

Remove fork in TurboModuleBinding.cpp #12731

chiaramooney opened this issue Feb 13, 2024 · 1 comment · Fixed by #12764
Assignees
Labels
Area: Native Modules Deforking Integration Follow-up Invalid Triage https://github.com/microsoft/react-native-windows/wiki/Triage-Process (label applied by bot)
Milestone

Comments

@chiaramooney
Copy link
Contributor

Following the removal of TurboModuleBindingMode in commit https://github.com/facebook/react-native/pull/38220/files, RN now defaults to the prototype code flow. This source now runs https://github.com/facebook/react-native/blob/f7f9250f6e90a41e798a8f62738925a2859a751a/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.cpp#L198-L202 in replace of return jsi::Object::createFromHostObject(runtime, std::move(module));. The new source causes app to render blank when you attempt to load a JS page.

We are encountering this issue now because we are finally able to defork some code in the TurboModuleBinding/LongLivedObject files which were held at a fixed version for 1-2 years.

Solution for now is to keep this one line forked and continue to return the hostobject. Ultimately we should figure out how to support the scenario upstream is running.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Feb 13, 2024
@jonthysell jonthysell removed the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Feb 22, 2024
@jonthysell jonthysell added this to the Backlog milestone Feb 22, 2024
@jonthysell
Copy link
Contributor

jonthysell commented Feb 22, 2024

@acoates-ms It looks like a lot of the reasons we had for forking this file are now gone (fixed upstream), can you see why this last line still needs to be forked and if the fork can be removed?

@microsoft-github-policy-service microsoft-github-policy-service bot added the Invalid Triage https://github.com/microsoft/react-native-windows/wiki/Triage-Process (label applied by bot) label Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Native Modules Deforking Integration Follow-up Invalid Triage https://github.com/microsoft/react-native-windows/wiki/Triage-Process (label applied by bot)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants