[keep-awake] Stop useKeepAwake calls deactivating each other by default.

[macksal]

Why

Currently, useKeepAwake has an unexpected quirk. If two mounted components call useKeepAwake() without providing an explicit tag, the screen can go to sleep after either one is unmounted - even if the other is present.

This result is surprising and goes against the most obvious interpretation of the useKeepAwake docs:

A React hook to keep the screen awake for as long as the owner component is mounted.

A more sensible default behaviour would be that mounting multiple components that each call the hook would each bind the keep-awake effect to their own lifecycle, and the screen could only be dimmed if all the components were unmounted in turn. However, with the default behaviour, if any one of them unmounts, the effect from all of them is deactivated.

My primary use case is a component that performs a long-running task on a button press, showing a progress bar to the user. I need to keep the screen awake as long as the task is running. Now let's say the user performs two tasks at the same time on two adjacent components. The completion of one will deactivate the keep-awake effect and allow the screen to dim while the other task is still running.

An additional motivation is that developers can be tripped up by expo activating expo-keep-awake by default in development mode. The default tag is used there, which means that any call to deactivateKeepAwake without an explicit tag will deactivate the devtools' keep-awake. This means the default behaviour only lasts until a component calling useKeepAwake() is mounted and then unmounted. This is another unexpected side effect of what otherwise appears to be a declarative API.

How

This happens because the tag passed to the native implementation defaults to a global constant value if none is explicitly provided. The native implementation doesn't count the number of activations for each tag, but instead adds or removes the tag from a set and keeps the screen awake as long as at least one tag is present in the set.

When no tag is provided to useKeepAwake, let's stop using the shared default tag. Instead, we'll generate a tag which is unique to the component calling useKeepAwake. React's useId is a drop-in candidate to create the unique tag per-component.

Other options considered and rejected:

• Use a different mechanism than useId to create a unique tag for each caller of the hook. A simple random string generated inside the body of the useEffect would be fine. On the other hand, useId is already designed to be unique based on the component hierarchy, and React is already a dependency and isn't going anywhere.
• Modify the native implementation to use a multimap/counter to store tags instead of a set. This would mean that calls to activateKeepAwakeAsync would need to be matched by an equal number of calls to deactivateKeepAwakeAsync with the same tag. This change would be a lot more pervasive and change the imperative part of the API significantly.

Test Plan

A unit test has been added that checks that two different calls to useKeepAwake each pass a different tag to the activate method in the native layer. It fails in the current SDK and passes with the changes in this PR.

Checklist

• Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md).
• Conforms with the Documentation Writing Style Guide
• This diff will work correctly for npx expo prebuild & EAS Build (eg: updated a module plugin).

[tsapeta]

Thank you @macksal for this PR and providing a very good explanation and motivation about this change! 🙇‍♂️

It looks good to me 👍 I'm only wondering if it should be a breaking change or a bug fix, considering the fact that the documentation should be the source of truth and expected behavior and the current implementation not doing exactly what is said in the docs. What do you think @macksal @alanjhughes?

[alanjhughes]

Thanks @macksal! I think it's a bug fix rather than a breaking change. I don't think there would be a lot of people relying on the current broken behaviour and breaking dev mode is very unexpected. This will likely go unnoticed by most @tsapeta

[macksal]

@alanjhughes @tsapeta I agree - I had it marked as a bugfix but I preempted a complaint about changing the behaviour 😅 I've just moved the changelog entry.

Thanks for approving and feel free to merge as-is.