[native] Add URI scheme to debug AndroidManifest.xml
ClosedPublic
Actions

Authored by bartek on Jan 20 2023, 3:46 AM.

Details

Reviewers

ashoat
atul

Commits

rCOMMafb9582081b9: [native] Add URI scheme to debug AndroidManifest.xml

Summary

Resolves ENG-2731

Added the comm:// URI scheme to debug AndroidManifest.xml

Followed the instruction from Setup Android URI scheme and the "Android Guide" linked there.
Added the <intent-filter> to src/debug/AndroidManifest.xml as suggested above, used Manifest merging to ensure it is applied correctly
Removed the leftover intent filter from src/main/AndroidManifest.xml - this wasn't fully removed in D1447 and Android Studio was signaling missing "data" attribute. This can be added back when necessary, it doesn't collide with my debug intent filter.

Test Plan

Build the app in Android Studio, install on emulator

cd native
yarn dev --scheme comm

Press the a (or shift+a for device select) key in Metro console to open the deeplink. The Comm app should open and skip the Expo launcher screen.

Manifest correctness was checked in Android Studio using "merge view":

Screenshot 2023-01-20 at 12.48.30.png (342×668 px, 66 KB)

Darker is the main manifest, while lighter blue is the debug one.

It is also possible to check if Expo's scheme utility uri-scheme detects it correctly:

$ npx uri-scheme list --manifest-path ./android/app/src/debug/AndroidManifest.xml 
› Android: Schemes for config: ./android/app/src/debug/AndroidManifest.xml
› comm://

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Jan 20 2023, 3:46 AM

bartek held this revision as a draft.

Herald added a subscriber: tomek. · View Herald TranscriptJan 20 2023, 3:46 AM

bartek edited the test plan for this revision. (Show Details)Jan 20 2023, 3:48 AM

bartek edited the test plan for this revision. (Show Details)

Harbormaster completed remote builds in B15494: Diff 21137.Jan 20 2023, 3:59 AM

bartek published this revision for review.Jan 20 2023, 3:59 AM

bartek attached a referenced file: F336770: Screenshot 2023-01-20 at 12.48.30.png. (Show Details)

In the Linear task you mentioned:

Now this is opt-in by running yarn dev --scheme comm

Can you clarify what you mean by "opt-in"? What are the pros/cons of "opting in"? Should we just make this the default? If the point is just to avoid patching Expo, should we then update our yarn start command to pass in --scheme comm?

native/android/app/src/debug/AndroidManifest.xml
18	This line isn't in the Expo link above – why did you need to add it? To match the main `AndroidManifest.xml`?

ashoat mentioned this in D6319: [native] Add URI scheme to debug Info.plist.Jan 20 2023, 10:36 AM

In D6320#189432, @ashoat wrote:

In the Linear task you mentioned:

Now this is opt-in by running yarn dev --scheme comm

Can you clarify what you mean by "opt-in"? What are the pros/cons of "opting in"? Should we just make this the default? If the point is just to avoid patching Expo, should we then update our yarn start command to pass in --scheme comm?

Oh, I wasn't clear enough. It is "opt-in" until scheme diffs (this and D6319) are landed on both platforms. Otherwise, the QR code and URL below would be different and pretty unusable on the "platform without scheme yet", See screenshot:

Screenshot 2023-01-20 at 10.50.59.png21 KBDownload

I'll add a diff that depends on both platforms and either:

adds the --scheme flag
patches expo-cli as this scheme should be detected automatically, mentioned it in Linear tasks)

The former has a disadvantage: the --scheme flag cannot be provided to the expo run command, which is also capable of running Metro. The latter adds another patch that will need to be maintained unless they fix it upstream.

native/android/app/src/debug/AndroidManifest.xml
18	Android Studio complains if it's not here. It doesn't matter much as it is the same value as in the "main" manifest and merging rules will skip it. You can see the final result on the screenshot in diff description.