-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
RNTester - Legacy component example: do not convert commands to string #42822
Conversation
This pull request was exported from Phabricator. Differential Revision: D53123956 |
facebook#42822) Summary: The conversion to string was introduced in D45043929. It was supposed to fix command execution for `MyLegacyNativeComponent` in RNTester on Android/Old Architecture. At the same time it introduced a regression on iOS, since we have [different code path](https://www.internalfb.com/code/fbsource/[ffee789cab9514c0a15b8a63869cbfdf4e534a56]/xplat/js/react-native-github/packages/react-native/React/Modules/RCTUIManager.m?lines=1088-1092) for string commands in iOS, where we expect command name, and not command number converted to string. I tried to remove that conversion, did local tests, and saw no issues with executing commands on Android. Looks like the underlying issue has been fixed in some other way. So let's just remove those conversions. Changelog: [Internal] Reviewed By: cortinico Differential Revision: D53123956
5134546
to
8b02feb
Compare
This pull request was exported from Phabricator. Differential Revision: D53123956 |
facebook#42822) Summary: The conversion to string was introduced in D45043929. It was supposed to fix command execution for `MyLegacyNativeComponent` in RNTester on Android/Old Architecture. At the same time it introduced a regression on iOS, since we have [different code path](https://www.internalfb.com/code/fbsource/[ffee789cab9514c0a15b8a63869cbfdf4e534a56]/xplat/js/react-native-github/packages/react-native/React/Modules/RCTUIManager.m?lines=1088-1092) for string commands in iOS, where we expect command name, and not command number converted to string. I tried to remove that conversion, did local tests, and saw no issues with executing commands on Android. Looks like the underlying issue has been fixed in some other way. So let's just remove those conversions. Changelog: [Internal] Reviewed By: cortinico Differential Revision: D53123956
8b02feb
to
e9e0fbc
Compare
This pull request was exported from Phabricator. Differential Revision: D53123956 |
e9e0fbc
to
a6d2c5a
Compare
facebook#42822) Summary: The conversion to string was introduced in D45043929. It was supposed to fix command execution for `MyLegacyNativeComponent` in RNTester on Android/Old Architecture. At the same time it introduced a regression on iOS, since we have [different code path](https://www.internalfb.com/code/fbsource/[ffee789cab9514c0a15b8a63869cbfdf4e534a56]/xplat/js/react-native-github/packages/react-native/React/Modules/RCTUIManager.m?lines=1088-1092) for string commands in iOS, where we expect command name, and not command number converted to string. I tried to remove that conversion, did local tests, and saw no issues with executing commands on Android. Looks like the underlying issue has been fixed in some other way. So let's just remove those conversions. Changelog: [Internal] Reviewed By: cortinico Differential Revision: D53123956
This pull request was exported from Phabricator. Differential Revision: D53123956 |
facebook#42822) Summary: The conversion to string was introduced in D45043929. It was supposed to fix command execution for `MyLegacyNativeComponent` in RNTester on Android/Old Architecture. At the same time it introduced a regression on iOS, since we have [different code path](https://www.internalfb.com/code/fbsource/[ffee789cab9514c0a15b8a63869cbfdf4e534a56]/xplat/js/react-native-github/packages/react-native/React/Modules/RCTUIManager.m?lines=1088-1092) for string commands in iOS, where we expect command name, and not command number converted to string. I tried to remove that conversion, did local tests, and saw no issues with executing commands on Android. Looks like the underlying issue has been fixed in some other way. So let's just remove those conversions. Changelog: [Internal] Reviewed By: cortinico Differential Revision: D53123956
a6d2c5a
to
c1c7fcd
Compare
This pull request was exported from Phabricator. Differential Revision: D53123956 |
This pull request has been merged in d66d651. |
Summary:
The conversion to string was introduced in D45043929. It was supposed to fix command execution for
MyLegacyNativeComponent
in RNTester on Android/Old Architecture.At the same time it introduced a regression on iOS, since we have different code path for string commands in iOS, where we expect command name, and not command number converted to string.
I tried to remove that conversion, did local tests, and saw no issues with executing commands on Android.
Looks like the underlying issue has been fixed in some other way.
So let's just remove those conversions.
Changelog: [Internal]
Differential Revision: D53123956