[Hold for Payment on 16 June] Make permissions checking asynchronous by creating a withBetas HOC

[iwiznia]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Problem

The methods in permissions.js file should be asynchronous. They kind of already are if you look at the code, since data is coming from Onyx, but the methods themselves as synchronous, which does not align with how the data is loaded, so there are possible race conditions.

Solution

Change all places where permissions is used to instead subscribe to the ONYXKEYS.BETAS onyx key and call permissions methods passing the betas it got from onyx (see this comment)

Upwork posting - https://www.upwork.com/jobs/~01f29f2d2fa738d842

/cc @tgolen @marcaaron

[MelvinBot]

Triggered auto assignment to @SofiedeVreese (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[tgolen]

Yeah, I think this is a good move. I'd like to see those moved to an HOC or at the very least be made asynchronous. Without them being asynchronous, we are limited by the amount of checks we can do. For example, we can't use Environment.getEnvironment() because that is an async method.

While this is not currently a problem, I'd like to change this code so that when new betas are added, they do not follow this old sync pattern anymore.

[marcaaron]

Gonna try to rephrase this problem solution to make sure I understand...

Problem

• The beta list is fetched async then saved to storage before it updates the local list in memory.
• We also use a native module to determine if the app version is beta which happens async.
• Checks in Permissions.js need to wait for those things to happen before returning or else they may be inaccurate.

Solution

Make the Permissions methods async + create an HOC to delay rendering the wrapped component until after the async things are finished so we can check the betas via props?

Or was the HOC supposed to work differently...?

[iwiznia]

What do you mean by delay rendering? As in other HOCs, I think we would not delay it, right? If the betas data is not present, it would render the same as if you were not in any beta and when the data is available, it would re-render.

[marcaaron]

As in other HOCs, I think we would not delay it, right? If the betas data is not present, it would render the same as if you were not in any beta and when the data is available, it would re-render.

withOnyx delays the rendering of a component until the data is retrieved from AsyncStorage so the Onyx.connect() part of our problem is already solved. That leaves the API call and the native modules async data.

Delaying came to mind since I'd assume we would not want to initialize a view with all beta features disabled by default and then flash in the beta features as the various data becomes available. But if we don't care about that then maybe another idea is to just subscribe directly to the betas key via withOnyx and update it with an action like...

let isOnDev;

function updateBetas(betas = []) {
    if (isOnDev) {
        Onyx.set(ONYXKEYS.BETAS, CONST.BETAS.ALL);
        return;
    }
    
    Onyx.merge(ONYXKEYS.BETAS, betas);
}

getEnvironment()
    .then((environment) => {
        isOnDev = environment === CONST.ENVIRONMENT.DEV;
        updateBetas();
    });

Now any component that is connected to withOnyx() can just pass this.props.betas to a Permissions.js method to check that feature.

render() {
    if (canUseChronos(this.props.betas)) {
        return (...)
    };
}

export default withOnyx({
    betas: {key: ONYXKEYS.BETAS}
})(SomeComponent);

[SofiedeVreese]

I'm out of office today so removing my assignment and adding the AutoAssigner label back in for triaging.