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: non-eas expo android builds #626

Closed
wants to merge 1 commit into from

Conversation

DoctorJohn
Copy link
Contributor

@DoctorJohn DoctorJohn commented Aug 8, 2024

Description

This PR fixes non-eas expo Android builds.

To create a non-eas expo build, users first run npx expo prebuild which generates native android and ios files. At this point the project is no longer considered a managed expo project but a bare react native project. This bare react native project has its native files already fully configured by expo prebuild (i.e. the admob app ID is already present in the AndroidManifest.xml file).
In this scenario we also want the custom configuration workflow to bail out, however, our expo detection does no longer work.

Before this PR, building such a fully configured bare react native android project failed, because our build.gradle script assumed the project is a bare react native project and still requires configuration. The custom config workflow build.gradle script then tries to obtain the relevant config keys from the app.json file which fails with the error message reported in #620.

Edit: while this PR indeed fixes the linked issue, I'm not fully sure the second half of my explanation above is correct. Converted this to a draft while thinking about it.
Edit2: This PR only works because of a bug. The android and ios file paths in the build.gradle file are wrong. This is only affecting the error messages shown for incorrectly configured projects (hence I never noticed this issue). Correctly configured projects are not affected.

Related issues

Checklist

  • I read the Contributor Guide
    and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in __tests__e2e__
    • jest tests added or updated in __tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

Copy link

docs-page bot commented Aug 8, 2024

To view this pull requests documentation preview, visit the following URL:

docs.page/invertase/react-native-google-mobile-ads~626

Documentation is deployed and generated using docs.page.

Copy link

codecov bot commented Aug 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 43.26%. Comparing base (a34c7ba) to head (ecee061).
Report is 49 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #626      +/-   ##
==========================================
- Coverage   43.72%   43.26%   -0.45%     
==========================================
  Files          30       31       +1     
  Lines         549      571      +22     
  Branches      151      152       +1     
==========================================
+ Hits          240      247       +7     
- Misses        309      324      +15     

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

Successfully merging this pull request may close these issues.

[🐛] No signature of method: java.lang.Boolean.getStringValue() under android v14.1.0+
1 participant