Page MenuHomePhabricator

[native] Introduce a hook that allows running an effect just once per installation
ClosedPublic

Authored by tomek on May 9 2023, 2:53 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 3, 8:22 PM
Unknown Object (File)
Wed, Apr 3, 8:22 PM
Unknown Object (File)
Wed, Apr 3, 8:22 PM
Unknown Object (File)
Wed, Apr 3, 8:21 PM
Unknown Object (File)
Wed, Apr 3, 8:14 PM
Unknown Object (File)
Feb 18 2024, 3:50 AM
Unknown Object (File)
Feb 18 2024, 2:58 AM
Unknown Object (File)
Feb 18 2024, 2:10 AM
Subscribers

Details

Summary

When checking referrer on Android, we would like to do it at most once per installation. To achieve that we can set a value in AsyncStorage and run the effect only if the value is not set.

Depends on D7743

Test Plan

Tested in combination with the next diff. Install the app using the referrer on Android, open the app and cancel invitation, close and open the app - the invitation shouldn't appear again.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek requested review of this revision.May 9 2023, 3:11 AM
This revision is now accepted and ready to land.May 9 2023, 4:42 AM
native/utils/hooks.js
9 ↗(On Diff #26267)

There is no guarantee here that this will only execute once.

  1. Is that potentially concerning in your use case? Or is your use case "idempotent", meaning it's okay for it to happen multiple times, but would be preferred to happen only once?
  2. If it is not concerning in your use case, we should consider that you are introducing a utility that others might consume in the future. You should add a code comment clarifying that effect should be idempotent, and the caller should be okay with it potentially be called more than once if effect is ever changed.
native/utils/hooks.js
9 ↗(On Diff #26267)

In this case the effect is about displaying invitation modal. So displaying it twice should be avoided but isn't a huge issue. I'm going to add comments that explain that it's possible for an effect to run multiple times.

I can also spend some time thinking about making this safer.

Make the logic a lot safer.

native/utils/hooks.js
9 ↗(On Diff #26267)

I modified the logic so that now it's extremely unlikely that an effect might run twice - the only serious case is when saving to async storage fails - then the next run of the app will result in running the effect again.

Thanks, this is much safer!

native/utils/hooks.js
21 ↗(On Diff #26449)

I guess it's safe to call await on something that isn't a promise?

native/utils/hooks.js
21 ↗(On Diff #26449)