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
629 stars 121 forks source link

feat(expo): improve loading expo configs, support app.config.ts #521

Open evelant opened 5 months ago

evelant commented 5 months ago

Description

Parse expo configs with @expo/config. This adds support for app.config.ts and makes projects configured with expo conform to the schema for app.config.ts/js.

Fixes potential crash in ios_config.sh when attempting to parse .js file as .json. Minor fixes to app-json.gradle.

Note: This works on the assumption that app.config.js/ts means it's an expo project and safe to load @expo/config. I'm not aware of any use of these files other than expo.

Related issues

Release Summary

feat(expo): improve loading expo configs, support app.config.ts

Checklist

Test Plan

There aren't any tests for expo configs yet. Tested manually on my production project. app.config.ts is resolved and configuration loaded correctly for iOS and Android.


Think react-native-google-mobile-ads is great? Please consider supporting the project with any of the below:

docs-page[bot] commented 5 months ago

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

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

Documentation is deployed and generated using docs.page.

CLAassistant commented 5 months ago

CLA assistant check
All committers have signed the CLA.

evelant commented 5 months ago

I signed the CLA, not sure why the bot doesn't see it

VictorioMolina commented 5 months ago

Thank you :D Awesome you included .ts support too. You saved my day.

evelant commented 5 months ago

Just occurred to me there's a potential for confusion here if someone uses pnpm since @expo/config won't be hoisted to root node_modules by default so it won't get resolved. Not sure there's much that can be done about that, maybe just a note in the docs that they need to explicitly install @expo/config.

github-actions[bot] commented 4 months ago

Hello 👋, this PR has been opened for more than 2 months with no activity on it.

If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing!

You have 15 days until this gets closed automatically

evelant commented 4 months ago

IDK Why the CLA bot isn't working, I signed the CLA and this is ready to be merged. Any idea @mikehardy ?

mikehardy commented 4 months ago

@evelant perhaps a mismatch between commit email address and registered github email addresses?

evelant seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

That's all I can think of since evelant is a github user, but the commits are actually keyed by email IIRC

github-actions[bot] commented 3 months ago

Hello 👋, this PR has been opened for more than 2 months with no activity on it.

If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing!

You have 15 days until this gets closed automatically

evelant commented 3 months ago

Sorry I haven't had a chance to make the requested changes yet. I'll try to get to it soon.

evelant commented 3 months ago

Should be good to go now @mikehardy. I improved the ambiguous docs and added a descriptive error message if @expo/config fails to resolve for some reason.

sairam4123 commented 3 months ago

Hey! So I'm using this Pull Request for my own app, and it seems like I'm having this error,

Logs ``` [eval]:11 Sat, 30 Mar 2024 18:11:22 GMT console.error("You have an expo app.config[.js|.ts] file but do not have "@expo/config" installed. "@expo/config" is required for react-native-google-mobile-ads to read the app.config[.js|.ts] file and should be installed by default with expo. Please install "@expo/config" and try again.") Sat, 30 Mar 2024 18:11:22 GMT ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sat, 30 Mar 2024 18:11:22 GMT SyntaxError: missing ) after argument list Sat, 30 Mar 2024 18:11:22 GMT at new Script (node:vm:100:7) Sat, 30 Mar 2024 18:11:22 GMT at createScript (node:vm:265:10) Sat, 30 Mar 2024 18:11:22 GMT at Object.runInThisContext (node:vm:313:10) Sat, 30 Mar 2024 18:11:22 GMT at node:internal/process/execution:79:19 Sat, 30 Mar 2024 18:11:22 GMT at [eval]-wrapper:6:22 Sat, 30 Mar 2024 18:11:22 GMT at evalScript (node:internal/process/execution:78:60) Sat, 30 Mar 2024 18:11:22 GMT at node:internal/main/eval_string:28:3 Sat, 30 Mar 2024 18:11:22 GMT Node.js v18.12.1 Sat, 30 Mar 2024 18:11:22 GMT FAILURE: Build completed with 2 failures. Sat, 30 Mar 2024 18:11:22 GMT 1: Task failed with an exception. Sat, 30 Mar 2024 18:11:22 GMT ----------- Sat, 30 Mar 2024 18:11:22 GMT * Where: Sat, 30 Mar 2024 18:11:22 GMT Build file '/home/expo/workingdir/build/node_modules/react-native-google-mobile-ads/android/build.gradle' line: 82 Sat, 30 Mar 2024 18:11:22 GMT * What went wrong: Sat, 30 Mar 2024 18:11:22 GMT A problem occurred evaluating project ':react-native-google-mobile-ads'. Sat, 30 Mar 2024 18:11:22 GMT > Cannot get property 'googleMobileAdsJson' on extra properties extension as it does not exist Sat, 30 Mar 2024 18:11:22 GMT * Try: Sat, 30 Mar 2024 18:11:22 GMT > Run with --stacktrace option to get the stack trace. Sat, 30 Mar 2024 18:11:22 GMT > Run with --info or --debug option to get more log output. Sat, 30 Mar 2024 18:11:22 GMT > Run with --scan to get full insights. Sat, 30 Mar 2024 18:11:22 GMT > Get more help at https://help.gradle.org. Sat, 30 Mar 2024 18:11:22 GMT ============================================================================== Sat, 30 Mar 2024 18:11:22 GMT 2: Task failed with an exception. Sat, 30 Mar 2024 18:11:22 GMT ----------- Sat, 30 Mar 2024 18:11:22 GMT * What went wrong: Sat, 30 Mar 2024 18:11:22 GMT A problem occurred configuring project ':react-native-google-mobile-ads'. Sat, 30 Mar 2024 18:11:22 GMT > compileSdkVersion is not specified. Please add it to build.gradle Sat, 30 Mar 2024 18:11:22 GMT * Try: Sat, 30 Mar 2024 18:11:22 GMT > Run with --stacktrace option to get the stack trace. Sat, 30 Mar 2024 18:11:22 GMT > Run with --info or --debug option to get more log output. Sat, 30 Mar 2024 18:11:22 GMT > Run with --scan to get full insights. Sat, 30 Mar 2024 18:11:22 GMT > Get more help at https://help.gradle.org. Sat, 30 Mar 2024 18:11:22 GMT ============================================================================== ```
Dependencies ``` "dependencies": { "@expo/config": "^8.5.4", "@gorhom/bottom-sheet": "^4", "@react-native-async-storage/async-storage": "1.21.0", "@react-native-community/datetimepicker": "7.6.1", "@react-native-community/netinfo": "11.1.0", "@react-native-community/slider": "4.4.2", "@react-native-picker/picker": "2.6.1", "@react-navigation/bottom-tabs": "6.5.7", "@react-navigation/drawer": "^6.6.3", "@react-navigation/material-top-tabs": "6.6.2", "@react-navigation/native": "6.1.6", "@react-navigation/native-stack": "6.9.12", "@supabase/supabase-js": "2.12.1", "@tanstack/react-query": "4.28.0", "@trpc/client": "10.37.1", "@trpc/react-query": "10.37.1", "@trpc/server": "10.37.1", "@types/react": "~18.2.45", "base64-arraybuffer": "^1.0.2", "clsx": "^2.0.0", "dayjs": "^1.11.9", "expo": "^50.0.0", "expo-av": "~13.10.3", "expo-checkbox": "~2.7.0", "expo-constants": "~15.4.5", "expo-dev-client": "~3.3.6", "expo-image": "~1.10.4", "expo-image-picker": "~14.7.1", "expo-linear-gradient": "~12.7.0", "expo-linking": "~6.2.2", "expo-splash-screen": "~0.26.3", "expo-status-bar": "~1.11.1", "expo-updates": "~0.24.8", "expo-web-browser": "~12.8.1", "nativewind": "^2.0.11", "react": "18.2.0", "react-native": "0.73.2", "react-native-alert-notification": "0.3.4", "react-native-gesture-handler": "~2.14.0", "react-native-google-mobile-ads": "https://github.com/sairam4123/react-native-google-mobile-ads#feat/better_expo_config_support", "react-native-keyboard-aware-scroll-view": "0.9.5", "react-native-marked": "^6.0.2", "react-native-modal": "13.0.1", "react-native-pager-view": "6.2.3", "react-native-popup-menu": "0.16.1", "react-native-reanimated": "~3.6.0", "react-native-round-flags": "1.0.4", "react-native-safe-area-context": "4.8.2", "react-native-screens": "~3.29.0", "react-native-shadow-2": "^7.0.8", "react-native-svg": "14.1.0", "react-native-swiper-flatlist": "3.0.18", "react-native-tab-view": "3.5.1", "react-native-url-polyfill": "1.3.0", "react-native-util": "1.0.2", "superjson": "^1.13.1", "tailwind-merge": "^1.14.0", "typescript": "4.9.4", "uid": "^2.0.2", "use-debounce": "9.0.4" } ```

is there anything I can do to fix this problem??

evelant commented 3 months ago

@sairam4123 are you using pnpm? If so the lack of hoisting may make it so that implicitly installed @expo/config cannot be resolved. Try installing @expo/config explicitly in your package.json.

sairam4123 commented 3 months ago

Hello, I'm using yarn "1.22.x", and I'm trying to build the Android APK using EAS Build. It seems that it is unable to find googleMobileAdsJson. It seems to work for iOS, only Android doesn't seem to work.

What could be done to fix this issue?

sairam4123 commented 2 months ago

I am also getting this error.

Unable to resolve "./version" from "..\..\node_modules\react-native-google-mobile-ads\src\index.ts"

Could you please help me get this working? This has completely stopped the development of the app. @evelant

sairam4123 commented 2 months ago

Hello @evelant, I'm sorry to let you know but I am currently stuck with this issue which caused the development to grind to an complete stop. I have been facing pressure in the last couple of days from my clients to get the project done in time but this issue is currently a major blocker, could I possibly expect a bug-fix soon?

Thank you so much for your time and effort! 💖

(I am extremely sorry if you find my posts repetitive)

evelant commented 2 months ago

@sairam4123 I'm not sure where that Unable to resolve "./version" is coming from, I don't think it's related to this PR. It sounds like maybe you have a configuration problem with your EAS build or package.json such that modules aren't getting resolved correctly. Are you using a monorepo structure?

sairam4123 commented 2 months ago

@sairam4123 I'm not sure where that Unable to resolve "./version" is coming from, I don't think it's related to this PR. It sounds like maybe you have a configuration problem with your EAS build or package.json such that modules aren't getting resolved correctly. Are you using a monorepo structure?

Yes, I'm using mono repo structure, it seems that version itself is missing from this repo, do we have any options to fix this? Do you have any idea about the failure in the eas build? Waiting for eas to complete a new build.

export const version = "v13.2.0"

adding this works, not sure whether this is even correct, lets see!

@evelant

evelant commented 2 months ago

I'm not really sure. It seems like you've got a module resolution issue due to monorepo hoisting most likely. I'm not a regular maintainer of this package so I don't know the details why it's trying to load that version module. I just wanted to contribute back the support for expo configs that I added for my own project.

I'd suggest you read the source code here to see why it's trying to load "..\..\node_modules\react-native-google-mobile-ads\src\index.ts". That could cause an issue if the path is hard coded as "..\..\node_modules\" since yarn hoists all of your dependencies to the root of your monorepo. The actual path is probably something like ..\..\..\..\node_modules/react-native-google-mobile-ads\src\index.ts depending on where it's trying to import from. Check the docs on hoisting with yarn, those should point you in the right direction.

sairam4123 commented 2 months ago

I think I have found the issue, it seems to be an issue with the console.error() because if you see the message more clearly, it says that, it needs ) after the argument list. It could be a possibility that the quotes seem to complete the strings and @expo/config seems to be a some sort of a variable to the compiler

I would love to hear your thoughts on this.

Screenshot_2024-04-05-07-34-23-55_40deb401b9ffe8e1df2f1cc5ba480b12.jpg

sairam4123 commented 3 weeks ago

I have been waiting for an update for two months, @evelant could you please merge my pr fixing the above-mentioned issue? Let me know when you do!

Also, please rebase this branch if possible!

Thank you for your help! 💖