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

[LOCAL] Fabric Interop - Properly dispatch integer commands (#38527) #38835

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

cortinico
Copy link
Contributor

Summary:
Pull Request resolved: #38527

This fixes a bug that got reported for the Fabric Interop for Android with Command dispatching. (See terrylinla/react-native-sketch-canvas#236)

The problem is that libraries that were receiving commands as ints with:

public void receiveCommand(
      int surfaceId, int reactTag, int commandId, Nullable ReadableArray commandArgs) {

would not receive command with the Fabric Interop for Android.

The problem is that with Fabric, events are dispatched as string always. cipolleschi took care of this for iOS, but we realized that the Android part was missing. I'm adding it here.

The logic is, if the event is dispatched as a string that represents a number (say "42") and the user has Fabric Interop enabled, then we dispatch the event as int (so libraries will keep on working).

Changelog:
[Android] [Fixed] - Fabric Interop - Properly dispatch integer commands

Reviewed By: cipolleschi

Differential Revision: D47600094

fbshipit-source-id: c35f0509e6c6c0cddc7090a069882f92dd95532e

Summary:

This is a pick of ccc50dd against the 0.72-stable branch where I've resolved all the merge conflicts. See #38527

Old Summary:

This fixes a bug that got reported for the Fabric Interop for Android with Command dispatching. (See terrylinla/react-native-sketch-canvas#236)

The problem is that libraries that were receiving commands as ints with:

public void receiveCommand(
      int surfaceId, int reactTag, int commandId, Nullable ReadableArray commandArgs) {

would not receive command with the Fabric Interop for Android.

The problem is that with Fabric, events are dispatched as string always. cipolleschi took care of this for iOS, but we realized that the Android part was missing. I'm adding it here.

The logic is, if the event is dispatched as a string that represents a number (say "42") and the user has Fabric Interop enabled, then we dispatch the event as int (so libraries will keep on working).

Changelog:

[Android] [Fixed] - Fabric Interop - Properly dispatch integer commands

Test Plan:

Already merged on main

Summary:
Pull Request resolved: #38527

This fixes a bug that got reported for the Fabric Interop for Android with Command dispatching.
(See terrylinla/react-native-sketch-canvas#236)

The problem is that libraries that were receiving commands as ints with:
```
public void receiveCommand(
      int surfaceId, int reactTag, int commandId, Nullable ReadableArray commandArgs) {
```

would not receive command with the Fabric Interop for Android.

The problem is that with Fabric, events are dispatched as string always.
cipolleschi took care of this for iOS, but we realized that the Android part was missing. I'm adding it here.

The logic is, if the event is dispatched as a string that represents a number (say `"42"`) and the user has Fabric Interop enabled, then we dispatch the event as `int` (so libraries will keep on working).

Changelog:
[Android] [Fixed] - Fabric Interop - Properly dispatch integer commands

Reviewed By: cipolleschi

Differential Revision: D47600094

fbshipit-source-id: c35f0509e6c6c0cddc7090a069882f92dd95532e
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Aug 8, 2023
@lunaleaps lunaleaps merged commit 3386bb4 into 0.72-stable Aug 8, 2023
@lunaleaps lunaleaps deleted the nc/pick-fabric-interop branch August 8, 2023 19:13
facebook-github-bot referenced this pull request Aug 9, 2023
Summary:
This PR converts SimpleViewPropertyTest into Kotlin as requested in [https://github.com/facebook/react-native/issues/38835](https://github.com/facebook/react-native/issues/38825#issuecomment-1669634951)

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[INTERNAL] [CHANGED] - Convert to SimpleViewPropertyTest Kotlin

Pull Request resolved: #38856

Test Plan:
1. Run `./gradlew :packages:react-native:ReactAndroid:test.`
2. All tests should pass.

Reviewed By: NickGerleman

Differential Revision: D48171939

Pulled By: mdvacca

fbshipit-source-id: eaa93a696faba1793b85186e6a072a7d3f043d0c
Kudo pushed a commit to expo/react-native that referenced this pull request Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner Pick Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants