Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create rememberLauncher to use Launchers in Compose #5274

Merged
merged 13 commits into from
Jul 27, 2022

Conversation

brnunes-stripe
Copy link
Contributor

@brnunes-stripe brnunes-stripe commented Jul 12, 2022

Summary

Create rememberLauncher for GooglePayLauncher, GooglePayPaymentMethodLauncher and PaymentLauncher.
Deprecate PaymentLauncher.createForCompose, which doesn't remember the PaymentLauncher instance, creating a new one at every recomposition.
Create sample Activities that use those methods on the example app.
Doc

Motivation

Support for Compose.

Testing

  • Added tests
  • Modified tests
  • Manually verified

@github-actions
Copy link
Contributor

github-actions bot commented Jul 12, 2022

Diffuse output:

OLD: paymentsheet-example-release-master.apk (signature: none)
NEW: paymentsheet-example-release-pr.apk (signature: none)

          │            compressed            │           uncompressed           
          ├───────────┬───────────┬──────────┼───────────┬───────────┬──────────
 APK      │ old       │ new       │ diff     │ old       │ new       │ diff     
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
      dex │  15.3 MiB │  15.3 MiB │ +2.5 KiB │  51.7 MiB │  51.7 MiB │ +7.4 KiB 
     arsc │   1.8 MiB │   1.8 MiB │      0 B │   1.8 MiB │   1.8 MiB │      0 B 
 manifest │     4 KiB │     4 KiB │      0 B │  18.5 KiB │  18.5 KiB │      0 B 
      res │ 871.6 KiB │ 871.6 KiB │      0 B │   1.4 MiB │   1.4 MiB │      0 B 
   native │   2.5 MiB │   2.5 MiB │      0 B │   5.9 MiB │   5.9 MiB │      0 B 
    asset │     3 MiB │     3 MiB │     -1 B │     3 MiB │     3 MiB │     -1 B 
    other │  81.7 KiB │  81.7 KiB │      0 B │ 155.5 KiB │ 155.5 KiB │      0 B 
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
    total │  23.5 MiB │  23.5 MiB │ +2.5 KiB │  63.9 MiB │  63.9 MiB │ +7.4 KiB 

         │          raw           │             unique              
         ├────────┬────────┬──────┼────────┬────────┬───────────────
 DEX     │ old    │ new    │ diff │ old    │ new    │ diff          
─────────┼────────┼────────┼──────┼────────┼────────┼───────────────
   files │      4 │      4 │    0 │        │        │               
 strings │ 250225 │ 250249 │  +24 │ 213328 │ 213350 │ +22 (+30 -8)  
   types │  44136 │  44143 │   +7 │  40513 │  40520 │  +7 (+8 -1)   
 classes │  37713 │  37720 │   +7 │  37713 │  37720 │  +7 (+8 -1)   
 methods │ 220615 │ 220645 │  +30 │ 212656 │ 212684 │ +28 (+40 -12) 
  fields │ 162620 │ 162628 │   +8 │ 161583 │ 161591 │  +8 (+13 -5)  

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  292 │  292 │  0   
 entries │ 6224 │ 6224 │  0
APK
     compressed     │    uncompressed    │                               
─────────┬──────────┼─────────┬──────────┤                               
 size    │ diff     │ size    │ diff     │ path                          
─────────┼──────────┼─────────┼──────────┼───────────────────────────────
 2.2 MiB │ +2.5 KiB │ 6.8 MiB │ +7.4 KiB │ ∆ classes3.dex                
 8.3 KiB │     -1 B │ 8.2 KiB │     -1 B │ ∆ assets/dexopt/baseline.prof 
─────────┼──────────┼─────────┼──────────┼───────────────────────────────
 2.2 MiB │ +2.5 KiB │ 6.8 MiB │ +7.4 KiB │ (total)
DEX
STRINGS:

   old    │ new    │ diff         
  ────────┼────────┼──────────────
   213328 │ 213350 │ +22 (+30 -8) 
  + ,
  ���
  ��
  ���
  ���
  
  ���
  
  ���
  
  ���
  
  ���
  �������2�0�B���¢����J%����0�2�����0�2��	��0
  2�����0H�¢���
  R�����0�X�T¢��
  ¨��
  + .
  ���
  ��
  ���
  ���
  
  ���
  
  ���
  ���
  ���
  
  ���
  �������2�0�B���¢����J*����0�2�����0�2�����0�2
  ���	����0�2��
  ��0�J*����0�2����0
  2�����0�2
  ���	����0�2��
  ��0�J)����0�2�����0�2
  ���	����0�2��
  ��0�H�¢����J)����0�2�����0�2
  ���	����0�2��
  ��0�H�¢����¨��
  + 4
  ���
  ��
  ���
  ���
  ���
  ���
  
  ���
  
  ���
  
  ���
  
  ���
  �������2�0�B���¢����J%�	��0
  2�����02��
  ��0�2�����0�H�¢����R�����0�X�T¢��
  R�����0�X�T¢��
  R�����0�X�T¢��
  R�����0�X�T¢��
  ¨��
  + ¡�
  ���
  ��
  
  ���
  
  ���
  
  ���
  
  ���
  ���
  ���
  ���
  ���
  
  ���
  
  ���
  ���
  ���
  ���
  
  ���
  ���
  ���
  
  ��
  ���
  
  ���
  ���
  ���
  
  ���
  
  ���
  
  ���
  ���
  ���
  ���
  
  ���
  ���
  ���
  ��	*�-��� <2�0�:�;<=>?@AB'��������0�������0�������0�������0	¢���
  B'��������0������0�������0�������0	¢���
  B5��������0�������0����������0�0�������0�������0�¢����B¿�����������0���������0���������0�������������0�0���������0�������0�����������0�����0�0�������������0�0�������������0�0 �����!�
  ������0�0 �������0������#��0_�����%��0&�����'��0(�����)��0*¢���+J&�5��062��7��0�2����8��092
  ���:����0�H�R���������0�0�X��¢��
  R�����0�X��¢��
  R����0�X��¢��
  R���������0�����0�0�X��¢��
  R��,��0-X��¢��
  ��.R��/��0�8�X��¢��
  ���0�1R��#��0_X��¢��
  R��2��0�X��¢��
  R��3��04X��¢��
  R���������0�0�X��¢��
  R���������0�0 X��¢��
  R�����0�X��¢��
  R�����0�X��¢��
  R��!�
  ������0�0 X��¢��
  ¨�B
  + _currentReadyCallback_delegate
  + _r8_lambda_iO--70zjc9p8JtpH7PupQdRU-jc
  + (Landroid/content/Context;Lkotlinx/coroutines/CoroutineScope;Landroidx/activity/result/ActivityResultLauncher;Lcom/stripe/android/googlepaylauncher/GooglePayPaymentMethodLauncher_Config;Lcom/stripe/android/googlepaylauncher/GooglePayPaymentMethodLauncher_ReadyCallback;)V
  + (Lcom/stripe/android/googlepaylauncher/GooglePayLauncher_Config;Lcom/stripe/android/googlepaylauncher/GooglePayLauncher_ReadyCallback;Lcom/stripe/android/googlepaylauncher/GooglePayLauncher_ResultCallback;Landroidx/compose/runtime/Composer;I)Lcom/stripe/android/googlepaylauncher/GooglePayLauncher;
  + (Lcom/stripe/android/googlepaylauncher/GooglePayPaymentMethodLauncher_Config;Lcom/stripe/android/googlepaylauncher/GooglePayPaymentMethodLauncher_ReadyCallback;Lcom/stripe/android/googlepaylauncher/GooglePayPaymentMethodLauncher_ResultCallback;Landroidx/compose/runtime/Composer;I)Lcom/stripe/android/googlepaylauncher/GooglePayPaymentMethodLauncher;
  + C(rememberLauncher)
  + C(rememberLauncher)P(1,2)
  + Lcom/stripe/android/googlepaylauncher/GooglePayLauncher_Companion_rememberLauncher_1_1;
  + Lcom/stripe/android/googlepaylauncher/GooglePayLauncher_Companion_rememberLauncher_1_2;
  + Lcom/stripe/android/googlepaylauncher/GooglePayLauncher_Companion_rememberLauncher_activityResultLauncher_1;
  + Lcom/stripe/android/googlepaylauncher/GooglePayPaymentMethodLauncher__ExternalSyntheticLambda1;
  + Lcom/stripe/android/googlepaylauncher/GooglePayPaymentMethodLauncher_6;
  + Lcom/stripe/android/googlepaylauncher/GooglePayPaymentMethodLauncher_Companion_rememberLauncher_1_1;
  + Lcom/stripe/android/googlepaylauncher/GooglePayPaymentMethodLauncher_Companion_rememberLauncher_activityResultLauncher_1;
  + Lcom/stripe/android/payments/paymentlauncher/PaymentLauncher_Companion_rememberLauncher_activityResultLauncher_1;
  + PaymentLauncher.rememberLauncher(publishableKey, stripeAccountId, callback)
  + SMAP
  GooglePayLauncher.kt
  Kotlin
  *S Kotlin
  *F
  + 1 GooglePayLauncher.kt
  com/stripe/android/googlepaylauncher/GooglePayLauncher_Companion
  + 2 CompositionLocal.kt
  androidx/compose/runtime/CompositionLocal
  + 3 Composables.kt
  androidx/compose/runtime/ComposablesKt
  + 4 Composer.kt
  androidx/compose/runtime/ComposerKt
  + 5 SnapshotState.kt
  androidx/compose/runtime/SnapshotStateKt
  *L
  1#1,341:1
  76#2:342
  76#2:343
  36#3:344
  955#4,6:345
  89#5:351
  *S KotlinDebug
  *F
  + 1 GooglePayLauncher.kt
  com/stripe/android/googlepaylauncher/GooglePayLauncher_Companion
  *L
  306#1:342
  307#1:343
  313#1:344
  313#1:345,6
  304#1:351
  *E
  
  + SMAP
  GooglePayPaymentMethodLauncher.kt
  Kotlin
  *S Kotlin
  *F
  + 1 GooglePayPaymentMethodLaunch
...✂

Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! bringing the same approach to Financial Connections.

resultCallback::onResult
)

return remember {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link

@chr-stripe chr-stripe Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be keyed off all of the input parameters? So that in case a different instance of any of those parameters are provided on a subsequent composition, a new instance is returned using those new values.

return remember(config, readyCallback, resultCallback) {
    // ...
}

I think the same question would apply to the other rememberLauncher methods also. (I could be wrong – still a bit of a Compose novice!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also pretty new to Compose. You're partially right, I've updated the example to use the config as a key. For the callbacks we use rememberUpdatedState, which is also what rememberLauncherForActivityResult uses. I added you to the API review doc, ptal and let me know if you have any suggestions.

GMetaxakis
GMetaxakis previously approved these changes Jul 15, 2022
Copy link

@GMetaxakis GMetaxakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good :D

chr-stripe
chr-stripe previously approved these changes Jul 20, 2022
@brnunes-stripe brnunes-stripe merged commit 2c4e0e8 into master Jul 27, 2022
@brnunes-stripe brnunes-stripe deleted the brnunes/composelauncher branch July 27, 2022 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants