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

fix(android): Fix bridgeless mode support #3352

Merged
merged 3 commits into from
Mar 21, 2024

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Mar 7, 2024

Why

react-native-webview does not work on react-native 0.74 with bridgeless mode

How

there were two errors actually:

  1. build error from override shouldStartLoadWithLockIdentifier(). the newer codegen supports Int32 and generates java code as int type. the current code is using double type. the pr tries to use double type as the implementation using double under the hood.
  2. on bridgeless mode, we cannot access CatalystInstance. it seems react-native-webview used it for sending direct message. BridgelessReactContext does not have equivalent callFunction(moduleName: string, ...). this pr tries to register a single RNCWebViewMessagingModule module and check the underlying messagingModuleName inside the event. each webview will only receive events belong its messagingModuleName.

Test Plan

  • test example on android
  • test example on android (with bumping react-native 0.74 and newArchEnabled=true)
  • test on react-native 0.71 old architecture to make sure no regression

// Fallback to use `BatchedBridge.registerCallableModule()` for older versions.
// eslint-disable-next-line global-require
= require('react-native').registerCallableModule
?? BatchedBridge.registerCallableModule.bind(BatchedBridge);
Copy link

@RSNara RSNara Mar 20, 2024

Choose a reason for hiding this comment

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

@Kudo, in 0.73, we also have access to global.RN$registerCallableModule in bridgeless mode. If you also include that in the fallback, then we could make this library bridgeless mode compatible in 0.73.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @RSNara for taking a look and having the feedback. i'm lean toward to claim the bridgeless support for react-native-webview from 0.74, because i didn't run any bridgeless test on on 0.73.

@Titozzz Titozzz merged commit 8411ba0 into react-native-webview:master Mar 21, 2024
11 checks passed
react-native-community-bot pushed a commit that referenced this pull request Mar 21, 2024
## [13.8.2](v13.8.1...v13.8.2) (2024-03-21)

### Bug Fixes

* **android:** Fix bridgeless mode support ([#3352](#3352)) ([8411ba0](8411ba0))
@react-native-community-bot
Copy link
Collaborator

🎉 This PR is included in version 13.8.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Kudo
Copy link
Contributor Author

Kudo commented Mar 27, 2024

late response but much appreciated for the help from @Titozzz to get the fix published.

@Kudo Kudo deleted the @kudo/bridgeless branch March 27, 2024 15:42
@andreialecu
Copy link

I believe this may have introduced a regression. #3368


useEffect(() => {
BatchedBridge.registerCallableModule(messagingModuleName, directEventCallbacks);
registerCallableModule('RNCWebViewMessagingModule', directEventCallbacks);

Choose a reason for hiding this comment

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

Should this first parameter be static and not messagingModuleName? Perhaps this is what may cause #3368

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

Successfully merging this pull request may close these issues.

None yet

5 participants