invertase / react-native-google-mobile-ads

React Native Google Mobile Ads enables you to monetize your app with AdMob.
https://docs.page/invertase/react-native-google-mobile-ads
Other
663 stars 134 forks source link

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

Open xdmx opened 1 month ago

xdmx commented 1 month ago

What happened?

I've been using this library for a while (thank you!) and recently migrated to the expo config, which worked well under 14.0.1, then I upgraded to 14.2.1 and I've started getting the compilation under android failing with:

> Configure project :react-native-google-mobile-ads
:react-native-google-mobile-ads package.json found at /Projects/app/node_modules/react-native-google-mobile-ads/package.json
:app (dev) package.json found at /Projects/app/package.json

FAILURE: Build failed with an exception.

* Where:
Build file '/Projects/app/node_modules/react-native-google-mobile-ads/android/build.gradle' line: 85

* What went wrong:
A problem occurred evaluating project ':react-native-google-mobile-ads'.
> No signature of method: java.lang.Boolean.getStringValue() is applicable for argument types: (String, String) values: [android_app_id, ]

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org.

I've tried older versions and it also happened with 14.1.0 and 14.2.0. 14.0.1 is the latest that works, without doing any change in the config or code other than changing the version in the package.json. It both fails in local and EAS compilations

Ios is working (compiling through EAS, I cannot do it locally)

Platforms

Only on Android

React Native Info

System:
  OS: Linux 6.10 Arch Linux
  CPU: (16) x64 AMD Ryzen 7 PRO 4750U with Radeon Graphics
  Memory: 8.60 GB / 29.10 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 20.14.0
    path: ~/.nodenv/versions/20.14.0/bin/node
  Yarn:
    version: 1.22.22
    path: /usr/bin/yarn
  npm:
    version: 10.7.0
    path: ~/.nodenv/versions/20.14.0/bin/npm
  Watchman:
    version: 20240414.112832.0
    path: /usr/bin/watchman
SDKs:
  Android SDK: Not Found
IDEs:
  Android Studio: AI-241.18034.62.2411.12071903
Languages:
  Java:
    version: 17.0.12
    path: /usr/bin/javac
  Ruby:
    version: 3.3.2
    path: ~/.rbenv/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: ~18.2.0
  react-native:
    installed: 0.74.3
    wanted: 0.74.3
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: Not found
  newArchEnabled: Not found

### Are your using Typescript?

- [ ] My project is using Typescript

### package.json

```JSON
{
  ...
  "react-native-google-mobile-ads": "~14.0.1",
  ...
}

### app.json

```JSON
[
        'react-native-google-mobile-ads',
        {
          androidAppId: process.env.ADMOB_ANDROID_APP_ID,
          iosAppId: process.env.ADMOB_IOS_APP_ID,
          delayAppMeasurementInit: true,
          userTrackingUsageDescription: 'This identifier will be used to deliver personalized ads to you.',
          skAdNetworkItems: [
            'cstr6suwn9.skadnetwork',
            ...
          ]


### ios/Podfile

_No response_

### android/build.gradle

_No response_

### android/app/build.gradle

_No response_

### android/settings.gradle

_No response_

### AndroidManifest.xml

_No response_
ycmjason commented 1 month ago

Using the expo plugin as instructed in the doc, I also encounter the same error even for 14.0.1.

Build file '/home/expo/workingdir/build/node_modules/react-native-google-mobile-ads/android/build.gradle' line: 85
* What went wrong:
A problem occurred evaluating project ':react-native-google-mobile-ads'.
> No signature of method: java.lang.Boolean.getStringValue() is applicable for argument types: (String, String) values: [android_app_id, ]
simonboisset commented 1 month ago

Same issue for me with expo 51 and app.config.ts file iOS build was successfully completed but android failed.

I must add a duplicated app.json config like this to fix it :

{
  "expo": {
    "name": "My app",
    "slug": "my-slug"
  },
  "react-native-google-mobile-ads": {
    "android_app_id": "ca-app-pub-xxx",
    "ios_app_id": "ca-app-pub-xxx",
    "user_tracking_usage_description": "My message",
    "delay_app_measurement_init": true,
    "sk_ad_network_items": []
}
minna607842 commented 1 month ago

Same issue

"expo": "^50.0.18",
"react-native-google-mobile-ads": "^14.2.1",
* What went wrong:
[RUN_GRADLEW] A problem occurred evaluating project ':react-native-google-mobile-ads'.
[RUN_GRADLEW] > No signature of method: java.lang.Boolean.getStringValue() is applicable for argument types: (String, String) values: [android_app_id, ]
[RUN_GRADLEW] * Try:
[RUN_GRADLEW] > Run with --stacktrace option to get the stack trace.
[RUN_GRADLEW] > Run with --info or
[RUN_GRADLEW] --debug option to get more log output.
[RUN_GRADLEW] > Run with --scan to get full insights.
[RUN_GRADLEW] > Get more help at https://help.gradle.org.
[RUN_GRADLEW] ==============================================================================
[RUN_GRADLEW] 2: Task failed with an exception.
[RUN_GRADLEW] -----------
[RUN_GRADLEW] * What went wrong:
[RUN_GRADLEW] A problem occurred configuring project ':react-native-google-mobile-ads'.
[RUN_GRADLEW] > compileSdkVersion is not specified. Please add it to build.gradle
[RUN_GRADLEW] * Try:
[RUN_GRADLEW] > Run with --stacktrace option to get the stack trace.
[RUN_GRADLEW] > Run with --info or --debug option to get more log output.
[RUN_GRADLEW] > Run with --scan to get full insights.
[RUN_GRADLEW] > Get more help at https://help.gradle.org.
[RUN_GRADLEW] ==============================================================================
jpdriver commented 1 month ago

caused by the above changes -- react-native-google-mobile-ads currently doesn't work with app.config.js files, only app.json

@dylancom can this support be re-introduced?


in the meantime, if anyone needs it here's a patch-package patch for anyone else who hits this 🫡

``` diff --git a/node_modules/react-native-google-mobile-ads/android/app-json.gradle b/node_modules/react-native-google-mobile-ads/android/app-json.gradle index 3a687fd..4a5a493 100644 --- a/node_modules/react-native-google-mobile-ads/android/app-json.gradle +++ b/node_modules/react-native-google-mobile-ads/android/app-json.gradle @@ -1,30 +1,67 @@ import groovy.json.JsonOutput import groovy.json.JsonSlurper -String fileName = "app.json" +String[] fileNames = ["app.json", "app.config.js"] +String fileName = null String jsonRoot = "react-native-google-mobile-ads" String jsonRaw = "GOOGLE_MOBILE_ADS_JSON_RAW" -File jsonFile = null +File configFile = null File parentDir = rootProject.projectDir for (int i = 0; i <= 3; i++) { if (parentDir == null) break parentDir = parentDir.parentFile if (parentDir != null) { - jsonFile = new File(parentDir, fileName) - if (jsonFile.exists()) break + configFile = new File(parentDir, fileNames[0]) + if (configFile.exists()) { + fileName = fileNames[0] + break + } + else { + configFile = new File(parentDir, fileNames[1]) + if (configFile.exists()) { + fileName = fileNames[0] + break + } + } } } -if (jsonFile != null && jsonFile.exists()) { - rootProject.logger.info ":${project.name} ${fileName} found at ${jsonFile.toString()}" +if (configFile != null && configFile.exists()) { + rootProject.logger.info ":${project.name} ${fileName} found at ${configFile.toString()}" Object json = null try { - json = new JsonSlurper().parseText(jsonFile.text) + // On windows, we need to escape path separators in the path before exec'ing shell + def configFileAbsolutePath = configFile.absolutePath + if (System.properties['os.name'].toLowerCase().contains('windows')) { + configFileAbsolutePath = configFileAbsolutePath.replace("\\", "\\\\") + } + // rootProject.logger.warn "have a path of ${configFileAbsolutePath}" + + // The config may be either in Expo javascript (app.config.js) or JSON (app.json) + // If it is configured in Expo javascript, requiring it will generate a config + // If it is configured in JSON, requiring it will also generate the config + // So, use node to pull in the config file to get us a JSON string for either case + def configOutput = new StringBuffer(); + def configReadProc = [ + "node", + "-e", + "console.log(JSON.stringify(require('${configFileAbsolutePath}')));" + ] + .execute(null, projectDir) + configReadProc.waitForProcessOutput(configOutput, System.err) + configOutput = configOutput.toString().trim() + // rootProject.logger.warn "got configOutput of ${configOutput.toString()}" + + if (configOutput && !configOutput.isEmpty()) { + json = new JsonSlurper().parseText(configOutput) + } else { + throw new Exception(":${project.name} received empty output while parsing ${configFile} found at ${configFile.toString()}.") + } } catch (Exception ignored) { - rootProject.logger.warn ":${project.name} failed to parse ${fileName} found at ${jsonFile.toString()}." + rootProject.logger.warn ":${project.name} failed to parse ${configFile} found at ${configFile.toString()}." rootProject.logger.warn ignored.toString() } ```
dylancom commented 1 month ago

I see different types of errors coming up here. Some people seem to have forgotten to switch over to using the new "Expo Config Plugin".

{
  "expo": {
    "plugins": [
      [
        "react-native-google-mobile-ads",
        {
          "androidAppId": "ca-app-pub-xxxxxxxx~xxxxxxxx",
          "iosAppId": "ca-app-pub-xxxxxxxx~xxxxxxxx"
        }
      ]
    ]
  }
}

@DoctorJohn Can you also have a look?

jpdriver commented 1 month ago

for what it's worth, our app is using the Expo Config Plugin within our app.config.js.

for some unrelated reasons (other Config Plugins we apply conditionally), we need to keep the dynamic config and a static app.json isn't an option for us.

with the workaround patch above reverting the app-json.gradle changes, everything is working great now -- but it would be even better if this could be restored and officially supported again so we don't have to leverage patch-package..


p.s. happy to spin up a minimal reproduction if that helps!

DoctorJohn commented 1 month ago

@DoctorJohn Can you also have a look?

app.config.js and app.config.ts should just work as long as they contain the config plugin. Looking into it right now.

DoctorJohn commented 1 month ago

p.s. happy to spin up a minimal reproduction if that helps!

@jpdriver it would be nice if you could share your app.config.js file. So far I have not seen a single valid app.config.js file in this issue.

jpdriver commented 1 month ago

minimal reproduction of the bug is here

https://github.com/jpdriver/expo-google-ads-repro

checkout this repo and run yarn android to see the build error.

steps taken can be seen in the commits to main 🙇🏻‍♂️

DoctorJohn commented 1 month ago

Thank you very much @jpdriver. I just identified the issue and I'm working on a PR for it. I will post an explanaiting in a bit.

DoctorJohn commented 1 month ago

@dylancom (cc: @mikehardy) unfortunately we need to discuss the configuration workflows again. The expo config plugin always works as expected (even in the repro provided in this issue), but the bailout conditions for the custom config workflow keep causing trouble.

Originally we bailed out of the custom config workflow if the expo package was found in the package.json file. This works great it almost all scenarious, except with bare react native projects that just happen to depend on the expo package but don't use the expo config plugin. This issue was reported in #614.

To address #614, we adjusted the bailout condition. Currently we intent to bail out when the project is a managed expo app by checking for the non-existance of android and ios directories. While drafting up #626 I noticed I not only got the relative paths to these folders wrong (../ is missing in both cases) but also that these folders will obviously always exist during build time. In affect every project is currently considered an expo project during Android builds. While this sounds bad, correctly configured projects are not affected by it at all (that's the reason I didn't catch this problem while testing). Currently the build.gradle file simply does not complain if a bare react native project does not have admob IDs configured. Note that fixing the wrong paths would still cause the errors reported in this issue, so let's discuss this one first.

In a nutshell: The error reported in this issue happens, because rootProject.ext.googleMobileAdsJson is not properly guarded in builds that don't have a app.json file. The reported error occures as soon as the project directory has no app.json file. I tried to guard it in #626 using isManagedExpoProject but then noticed this check is insufficient because of the following reasons:

Our current strategy is to bail out of the custom config workflow when a managed expo project is detected (i.e. no android and ios dirs exist). However, as soon as a user or EAS runs npx expo prebuild, native files are generated and placed in android and ios folders. At this point the projects admob IDs and other settings are fully configured (via the config plugin and expo prebuild), however, the project is now no longer considered a managed expo project (because android and ios directories now exist). When such a project is build (e.g. via yarn android in the repro), the custom config workflow no longer bails out and fails with the error message reported in this issue in case a app.json file is not present. TL;DR: Our current bailout condition only works during prebuild, but not during builds.*

To address this issue we have a few limited options:

  1. drop the custom configuration workflow so we can get rid of bailout conditions
  2. let build.gradle and ios_config.sh no longer fail but only warn if a bare react native project without config keys in app.json is detected
  3. come up with a second bailout condition that detects when a bare react native project is allowed to build without providing admob app IDs via app.json
  4. go back to the old bailout condition and drop support for bare react native projects using any expo packages

I still believe (1) would save us a lot of trouble, though we discussed this before and Mike was in favor of at least trying to keep the custom config workflow. Option (2) would mean that incorrectly configured native projects would only yield a warning about missing admob app ID's in app.json files. This warning would also be shown when building a project generated with expo prebuild. This option would spare us from having to come up with yet another bailout condition while keeping the custom config workflow. Option (3) is the hardest IMHO, I currently don't know how to reliably detect that case without introducing more edge cases. Option (4) is essentially causing #614 again and IMHO is a non-option.

For now I will create a hotfix PR that fixes the relative paths mentioned above and basically implements option (2). This will make sure all correctly configured projects can build while we figure out how to proceed.

*You might be wondering how I did not realize this was broken when I claimed my PRs work with EAS builds. The reason is simple: I tested the EAS builds with a correctly configured expo project that has both app.json and app.config.js files. This way the error reported in this issue does not appear because an app.json file exists (note that it only has to exist to avoid the error, it does in fact not contain an android_app_id).

jpdriver commented 1 month ago

i think we might be on different pages 😅

this is a Gradle build issue introduced because we no longer read either app.json or app.config.js into rootProject.ext.googleMobileAdsJson.

again, with a simple patch which re-introduces support for both app.json or app.config.js in app-json.gradle, everything works as expected -- including in Bare Expo projects. I've added this to the repro for illustration.

the config plugin is working perfectly even though this is a Bare project; and i can see the com.google.android.gms.ads.APPLICATION_ID being set correctly in my AndroidManifest.xml.

what i'm trying to ask is if we can revert just that one app-json.gradle file so that we can fill rootProject.ext.googleMobileAdsJson in all cases.

i don't believe there needs to be any change to detection logic, the config plugin itself, or really anywhere else in the lib 😅

DoctorJohn commented 1 month ago

@jpdriver You're correct that the expo config plugin does not require any changes. It already fully supports app.json, app.config.js, and app.config.ts. The issue here is caused by build.gradle.

The build.gradle file is used by configuration workflow that is fully separate from the expo config plugin and therefore does not need to be able to handle app.config.js or app.config.ts at all. However, build.gradle interferes with the expo config plugin during build time. To stop this interference we have bailout conditions in place which are supposed to detect when the expo config plugin is used. These conditions are faulty and my super long comment outlines the second attempt to fix them.

The solution you propose is a collaborative approach which could work for android but is merely a workaround for the underlying concerns and needs extra consideration, namely:

While your patch (which I fully appreciate!!) avoid the error in this issue it would result in the following:

  1. During prebuild the expo config plugin would be used to generate a valid AndroidManifest.xml file. build.gradle and your patch have not been invoked at this point at all.
  2. Next, during the actual build, the build.gradle file and your patch would inject a second set of metadata into the AndroidManifest.xml file.
  3. Gradle would then resolve these conflicts prefering the values provided by the config plugin, because we configured it to insert metadata with higher priority. While this ultimately works for app.config.js files under android, I want to avoid the redudant insertion of metadata and maintaining app.config.*s support for gradle without every actually using any of the parsed values.
jpdriver commented 1 month ago

i tried having both app.json and app.config.js (defining the plugin options in the JSON version and then spreading that into our dynamic config) which seemed to build correctly but then crashed on app launch.. 😅

i'm not sure if that's related to this lib or just something else with our app though so please take that with a pinch of salt..

the only other thing i can think to suggest is maybe considering whether expo-dev-client is installed as part of the Managed / Non-Managed decision.

for projects which:-

if these were correctly detected as Bare projects, would that work? 🤔

DoctorJohn commented 1 month ago

which seemed to build correctly but then crashed on app launch..

This is could be because the admob app ids in your app.config.js are not valid. (you can try to use testids to make it work :) )

maybe considering whether expo-dev-client is installed as part of the Managed / Non-Managed decision.

Unfortunately bare react native project can also install and work with that package (that would result in the following issue where someone is using another arbitrary expo package in their bare react native app #614)

if these were correctly detected as Bare projects, would that work? 🤔

Unfortunately every react native project has ios and android dirs at build time and we need to be able to detect at build time whether someone is using expo.

Things I tried so far:

@jpdriver Feel free to take a look at the PR I just created. It would be very helpful if you could verify whether it works with your app.

mikehardy commented 1 month ago

:tada: This issue has been resolved in version 14.2.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

jpdriver commented 1 month ago

no longer seeing this issue with v14.2.2 🥳

h/t @DoctorJohn @dylancom

DoctorJohn commented 1 month ago

@dylancom if we're fine with build.gradle and ios_config.sh only warning about missing admob app IDs instead of failing the build, we can ignore my super long comment and close this issue. Otherwise we would need to come up with a rock solid approach for detecting at build time whether a project uses the app.json based config approach vs the expo config plugin. My attempts so far are summarized at the end of my previous comment.

github-actions[bot] commented 5 days ago

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.