Generates assetlinks.json when building the App #179
Conversation
- Uses information from they keystore and the applicationId to generate the Digital Asset Links file that matches the key used to build the signed APK. - If the developer opted into Play Signing by Play the fingerprint won't match and the Digital Asset Links validation will not validate
packages/cli/src/lib/cmds/build.ts
Outdated
| import {AndroidSdkTools, Config, GradleWrapper, JdkHelper, Log, TwaManifest} | ||
| from '@bubblewrap/core'; | ||
| import {AndroidSdkTools, Config, DigitalAssetLinks, GradleWrapper, JdkHelper, Log, TwaManifest, | ||
| KeyTool} from '@bubblewrap/core'; |
There was a problem hiding this comment.
Is there an ordering to these imports? If they're alphabetical, KeyTool is in the wrong place.
There was a problem hiding this comment.
VS Code doesn't automatically sort it, do I've been doing this manually after auto-imports. Forgot about this one.
packages/cli/src/lib/cmds/build.ts
Outdated
| @@ -137,5 +139,31 @@ export async function build( | |||
| ); | |||
|
|
|||
| log.info(`Signed Android App generated at "${outputFile}"`); | |||
|
|
|||
| try { | |||
| const digitalAssetLinksFile = './assetlinks.json'; | |||
There was a problem hiding this comment.
Is this going to be relative to the directory the tool was run from? Is that what we want?
There was a problem hiding this comment.
I noticed that, but for now I'm following what we're doing for ./app-release-signed.apk. I do think a flag to output somewhere different in the future will be useful (will be a larger change than this, as it'll touch other steps).
packages/cli/src/lib/cmds/build.ts
Outdated
| @@ -137,5 +139,31 @@ export async function build( | |||
| ); | |||
|
|
|||
| log.info(`Signed Android App generated at "${outputFile}"`); | |||
|
|
|||
| try { | |||
There was a problem hiding this comment.
I'd say you probably want to move this into its own function, that way we can make the if on line 153 a guard clause and return early afterwards.
There was a problem hiding this comment.
+1. I thought about it when writing the if statement but ended up forgetting. 🤦
packages/cli/src/lib/cmds/build.ts
Outdated
| await fs.promises.writeFile(digitalAssetLinksFile, digitalAssetLinks); | ||
|
|
||
| log.info(`Digital Asset Links file generated at ${digitalAssetLinksFile}`); | ||
| log.warn('If opting into "App Signing by Google Play" when uploading the Android app for ' + |
There was a problem hiding this comment.
I understand why you're doing this, but I don't like the idea of the tool outputting a warning that the user can't get rid of. I'm not sure there's a better way though.
There was a problem hiding this comment.
We can do info and just hightlight it with colors. How does that sound?
There was a problem hiding this comment.
I think it's more that even if the user does everything right, they're going to have a big highlighted warning every time they run the tool.
I think longer term we should have something in twa-config for Play Signing - either a "isUsingPlaySigning: false" the user can set or a "playSigningSignatures: '...'" they can use to make things work right. It's probably outside of the scope for this change though.
- Adds tests
packages/cli/src/lib/cmds/build.ts
Outdated
| } | ||
|
|
||
| const digitalAssetLinks = | ||
| DigitalAssetLinks.generateAssetLinks(twaManifest.packageId, sha256Fingerprint); |
Uses information from they keystore and the applicationId to
generate the Digital Asset Links file that matches the key used
to build the signed APK.
If the developer opted into Play Signing by Play the fingerprint
won't match and the Digital Asset Links validation will not
validate
Closes #5