feat(android): simple installation by bang9 · Pull Request #414 · invertase/notifee

bang9 · 2022-05-25T09:12:59Z

Hello Team, thank you for always supporting high-quality open source!
I got the idea from react-native-gradle and simplifies the installation.
It works, but I'm not sure of any side effects because I don't have deep knowledge of groovy.
so feel free to comment. thank you!

helenaford · 2022-05-25T11:11:24Z

ooh thanks for this p/r 😍 it looks like it should work 👀 I'll double-check locally, and I'll kick off the jobs.

Would you be able to sign the cla?

Thank you!

mikehardy

The CI failures seem both real and unrelated to this PR

jest is failing and has been for a few builds, looks like ts-jest module is missing somehow
iOS build is failing because Podfile and Podfile.lock are out of sync

Not sure when this happens but it looks like an incomplete dependency update happened at some point. That's usually me doing dependency updates so perhaps I made a mistake along the road somewhere. Either way this will definitely simplify the android installation if it works, curious for @helenaford testing results, at the same time, I think this PR would be most complete if the tests_react_native app removed the repository definition in order for this to actually be exercised in CI -

notifee/tests_react_native/android/build.gradle

Lines 39 to 41 in c2050f1

    
           maven { 
        
             url "$rootDir/../node_modules/@notifee/react-native/android/libs" 
        
           }

as well as the example

mikehardy

Nice!

The CI failures seem both real and unrelated to this PR

jest is failing and has been for a few builds, looks like ts-jest module is missing somehow
iOS build is failing because Podfile and Podfile.lock are out of sync

Not sure when this happens but it looks like an incomplete dependency update happened at some point. That's usually me doing dependency updates so perhaps I made a mistake along the road somewhere. Either way this will definitely simplify the android installation if it works, curious for @helenaford testing results, at the same time, I think this PR would be most complete if the tests_react_native app removed the repository definition in order for this to actually be exercised in CI -

notifee/tests_react_native/android/build.gradle

Lines 39 to 41 in c2050f1

    
           maven { 
        
             url "$rootDir/../node_modules/@notifee/react-native/android/libs" 
        
           }

as well as the example

notifee/packages/react-native/example/android/build.gradle

Line 26 in c2050f1

maven { url "$rootDir/../node_modules/@notifee/react-native/android/libs" }

and the example construction script

notifee/packages/react-native/refresh-example.sh

Lines 38 to 39 in c2050f1

    
           sed -i -e $'s/mavenLocal()/mavenLocal()\\\n        maven \{ url "$rootDir\/..\/node_modules\/@notifee\/react-native\/android\/libs" \}/' android/build.gradle 
        
           rm -f android/build.gradle??

That will purge it out of everywhere and then CI will demonstrate whether it works or not I think?

bang9 · 2022-05-25T13:51:24Z

packages/react-native/example/package.json

  "dependencies": {
-    "@notifee/react-native": "github:notifee/react-native-notifee",
+    "@notifee/react-native": "link:../",


I linked to the current project to check the build with changes, please let me know if a revert is needed!

I feel like this was there for some reason, but in retrospect I can't remember what it is. I think the link to local is better personally and I'd like it to work. Willing to go with it and see if it works, personally

mikehardy · 2022-05-25T14:26:03Z

packages/react-native/plugin/src/withNotifeeProjectGradlePlugin.ts

  return buildGradle.replace(/compileSdkVersion = 30/, `compileSdkVersion = 31`);
 };

-const setMavenRepository = (projectBuildGradle: string): string => {


Oh you got the expo plugin too, I forgot about it 🤦 - nicely done

I'd never know if you hadn't told me about the tests. 👍

mikehardy · 2022-05-25T15:17:47Z

Believe it or not, the CI e2e android test is encouraging - it did fail, but it did build, failure is just a flaky test.
It would (should?) fail completely if the repo addition in the build.gradle was unable to locate the generated AAR
This is a big enough change that it still makes sense to pull it locally, and I have restarted the android e2e ci run (it does pass sometimes, though it has a flaky test more frequently then I wish), but this appears to be working!

bang9 · 2022-05-26T14:43:24Z

I attached a jest result screenshot ran from my local !

If there is a high rate that tests cannot be performed because the ts-jest path cannot be found, we may consider specifying the path directly💡

helenaford · 2022-05-29T21:24:50Z

thank you!! Merging! And about to release 😎

bang9 · 2022-05-30T01:05:54Z

@mikehardy @helenaford Thank you!

mikehardy · 2022-06-02T18:03:51Z

packages/react-native/android/build.gradle

 ReactNative.shared.applyDefaultExcludes()
 ReactNative.module.applyAndroidVersions()
 ReactNative.module.applyReactNativeDependency("api")
+rootProject.allprojects {


@christocracy - you might like this, removes an error-prone install step for react-native repos with local maven repositories to load aars, appears to work fine for us here, could clean up my app build files from background-fetch and geolocation-android :-)

Whoa, cool!!

feat(react-native): added maven repository apply script

02ae7d0

mikehardy reviewed May 25, 2022

View reviewed changes

mikehardy requested changes May 25, 2022

View reviewed changes

bang9 added 4 commits May 25, 2022 22:16

docs(readme): update android installation

6c3adaa

chore(react-native): remove core maven related lines

a78cfbd

fix(example): set compileSdkVersion to 31

e1eef2d

chore: link dependencies to current notifee project

e15d304

bang9 commented May 25, 2022

View reviewed changes

bang9 requested a review from mikehardy May 25, 2022 13:53

mikehardy reviewed May 25, 2022

View reviewed changes

helenaford merged commit 379d0a6 into invertase:main May 29, 2022

bang9 deleted the patch-1 branch May 31, 2022 17:58

mikehardy reviewed Jun 2, 2022

View reviewed changes

mikehardy mentioned this pull request Jun 7, 2022

Could not find any matches for app.notifee:core:+ as no versions of app.notifee:core are available. #350

Closed

	maven {
	url "$rootDir/../node_modules/@notifee/react-native/android/libs"
	}

	sed -i -e $'s/mavenLocal()/mavenLocal()\\\n maven \{ url "$rootDir\/..\/node_modules\/@notifee\/react-native\/android\/libs" \}/' android/build.gradle
	rm -f android/build.gradle??

Conversation

bang9 commented May 25, 2022

Uh oh!

helenaford commented May 25, 2022

Uh oh!

mikehardy left a comment

Choose a reason for hiding this comment

Uh oh!

mikehardy left a comment

Choose a reason for hiding this comment

Uh oh!

bang9 May 25, 2022

Choose a reason for hiding this comment

Uh oh!

mikehardy May 25, 2022

Choose a reason for hiding this comment

Uh oh!

mikehardy May 25, 2022

Choose a reason for hiding this comment

Uh oh!

bang9 May 25, 2022

Choose a reason for hiding this comment

Uh oh!

mikehardy commented May 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bang9 commented May 26, 2022

Uh oh!

helenaford commented May 29, 2022

Uh oh!

bang9 commented May 30, 2022

Uh oh!

mikehardy Jun 2, 2022

Choose a reason for hiding this comment

Uh oh!

christocracy Jun 2, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mikehardy commented May 25, 2022 •

edited

Loading