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

[RNMobile] Upgrade compile and target sdk version to Android API 33 #5789

Merged

Conversation

wpmobilebot
Copy link
Collaborator

@wpmobilebot wpmobilebot commented May 18, 2023

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 19, 2023

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

@geriux
Copy link
Contributor

geriux commented May 19, 2023

It looks like some tests are failing here, do we know why? 🤔

@geriux geriux self-requested a review May 19, 2023 13:45
@fluiddot
Copy link
Contributor

It looks like some tests are failing here, do we know why? 🤔

Each test fails due to a different reason:

iOS Build:

Undefined symbols for architecture x86_64:
  "_OBJC_CLASS_$_RCTViewManager", referenced from:
      _OBJC_CLASS_$_BVLinearGradientManager in BVLinearGradientManager.o
  "_OBJC_METACLASS_$_RCTViewManager", referenced from:
      _OBJC_METACLASS_$_BVLinearGradientManager in BVLinearGradientManager.o
  "_OBJC_CLASS_$_RCTView", referenced from:
      _OBJC_CLASS_$_BVLinearGradient in BVLinearGradient.o
  "_OBJC_METACLASS_$_RCTView", referenced from:
      _OBJC_METACLASS_$_BVLinearGradient in BVLinearGradient.o
  "_RCTRegisterModule", referenced from:
      +[BVLinearGradientManager load] in BVLinearGradientManager.o
  "_OBJC_CLASS_$_RCTConvert", referenced from:
      objc-class-ref in BVLinearGradient.o
ld: symbol(s) not found for architecture x86_64

Seems there's an incompatibility in the react-native-linear-gradient that makes the build fail. I think it's because the inter-dependency between react-native-hsv-color-picker and this library. I've just realized that when we bump react-native-linear-gradient, we also have to update the reference in react-native-hsv-color-picker (reference) 🙃 .

Test Android:

TypeError: Cannot read property 'isFileUploadSupported' of undefined

      3 |  */
      4 | import { Dimensions, StyleSheet } from 'react-native';
    > 5 | import { WebView } from 'react-native-webview';
        | ^
      6 |
      7 | /**
      8 |  * WordPress dependencies

This is probably caused by the new version of react-native-webview, although I can't reproduce it when running the Gutenberg demo app. If it's only affecting unit tests, we might need to mock the module. I'll take a look.

@geriux
Copy link
Contributor

geriux commented May 19, 2023

Seems there's an incompatibility in the react-native-linear-gradient that makes the build fail. I think it's because the inter-dependency between react-native-hsv-color-picker and this library. I've just realized that when we bump react-native-linear-gradient, we also have to update the reference in react-native-hsv-color-picker (reference) 🙃 .

Ahh I see, I just encountered this issue, couldn't build the iOS demo project due to BVLinearGradient

@fluiddot
Copy link
Contributor

@geriux With the last changes I introduced in WordPress/gutenberg#50731, the failures in the CI jobs should be fixed 🤞.

iOS Build:

Undefined symbols for architecture x86_64:
  "_OBJC_CLASS_$_RCTViewManager", referenced from:
      _OBJC_CLASS_$_BVLinearGradientManager in BVLinearGradientManager.o
  "_OBJC_METACLASS_$_RCTViewManager", referenced from:
      _OBJC_METACLASS_$_BVLinearGradientManager in BVLinearGradientManager.o
  "_OBJC_CLASS_$_RCTView", referenced from:
      _OBJC_CLASS_$_BVLinearGradient in BVLinearGradient.o
  "_OBJC_METACLASS_$_RCTView", referenced from:
      _OBJC_METACLASS_$_BVLinearGradient in BVLinearGradient.o
  "_RCTRegisterModule", referenced from:
      +[BVLinearGradientManager load] in BVLinearGradientManager.o
  "_OBJC_CLASS_$_RCTConvert", referenced from:
      objc-class-ref in BVLinearGradient.o
ld: symbol(s) not found for architecture x86_64

This one was tricky. Seems that due to the inter-dependency between react-native-hsv-color-picker and react-native-linear-gradient, I generated the Podfile.lock before creating wordpress-mobile/react-native-hsv-color-picker#9 and the reference to react-native-linear-gradient was wrong. I tried to re-generate but the only solution that worked was editing the commits manually and install Pods after both libraries were updated 🙃 .

Test Android:

TypeError: Cannot read property 'isFileUploadSupported' of undefined

      3 |  */
      4 | import { Dimensions, StyleSheet } from 'react-native';
    > 5 | import { WebView } from 'react-native-webview';
        | ^
      6 |
      7 | /**
      8 |  * WordPress dependencies

This issue was only happening in the testing environment due to accessing a native module on Android in react-native-webview. I updated the Jest configuration and mocked the native module to make it work.

That said, the PR is ready to be reviewed, as well as WordPress/gutenberg#50731. Let me know if you could take a look, thanks 🙇 !

@geriux
Copy link
Contributor

geriux commented May 22, 2023

Thank you for updating the PR! I was able to run the Demo app 🎉 but I'm unable to run the editor using a local build of WordPress iOS. I know this change is Android only but I'm testing both platforms due to the forks updates. I'm getting:

Installing BVLinearGradient 2.5.6-wp-4 (was 2.5.6-wp-3)
warning: Could not find remote branch v2.5.6-wp-4 to clone.
fatal: Remote branch v2.5.6-wp-4 not found in upstream origin

Is this expected?

@fluiddot
Copy link
Contributor

Thank you for updating the PR! I was able to run the Demo app 🎉 but I'm unable to run the editor using a local build of WordPress iOS. I know this change is Android only but I'm testing both platforms due to the forks updates. I'm getting:

Installing BVLinearGradient 2.5.6-wp-4 (was 2.5.6-wp-3)
warning: Could not find remote branch v2.5.6-wp-4 to clone.
fatal: Remote branch v2.5.6-wp-4 not found in upstream origin

Is this expected?

Thanks @geriux for reviewing the PR 🙇 !

How are you generating the local build? I'm able to build WP-iOS by using local Gutenberg installation via the following command: LOCAL_GUTENBERG=1 bundle exec pod install --repo-update. In any case, I'll create a WP-iOS PR for testing that an installable build can be generated as a double-check.

@fluiddot
Copy link
Contributor

@geriux I've tested installing pods pointing to this branch and managed to reproduce the failure you shared. I think it's expected because the podspec files point to tag versions in the forked repositories. However, the tags haven't been generated because the PRs in the forked repositories are not merged yet:

We could either merge them and test WP-iOS, or update the podspec files to point to the update/android-13 branch of the forked repositories. Do you have a preference?

@geriux
Copy link
Contributor

geriux commented May 22, 2023

However, the tags haven't been generated because the PRs in the forked repositories are not merged yet:

Ohhh right! It's been a while since I worked with forks 😅 thanks for confirming and explaining why it was failing.

Let me review those and approve them so we can merge them, then we can test it over the WP-iOS PR.

@fluiddot fluiddot force-pushed the version-toolkit/gutenberg/rnmobile/update/upgrade-android-13 branch from 93db9c8 to ec7dd0a Compare May 30, 2023 15:18
@fluiddot
Copy link
Contributor

I created wordpress-mobile/WordPress-iOS#20774 to reflect the React Native dependencies update in WP-iOS.

@fluiddot fluiddot requested a review from derekblank May 31, 2023 10:40
Copy link
Contributor

@geriux geriux left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work! 🚀 Should we add an internal note with these changes?

@fluiddot
Copy link
Contributor

Should we add an internal note with these changes?

Done in 6d7852a. Thanks @geriux for reviewing the PR 🙇 !

@fluiddot fluiddot added this to the 1.97.0 (22.6) milestone May 31, 2023
@fluiddot fluiddot enabled auto-merge May 31, 2023 15:24
@fluiddot fluiddot merged commit 98a029b into trunk May 31, 2023
@fluiddot fluiddot deleted the version-toolkit/gutenberg/rnmobile/update/upgrade-android-13 branch May 31, 2023 16:58
@jhnstn jhnstn mentioned this pull request Jun 9, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android 13 dependencies Pull requests that update a dependency file [OS] Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants