expo / router

[ARCHIVE]: Expo Router has moved to expo/expo -- The File-based router for universal React Native apps
https://docs.expo.dev/routing/introduction/
1.36k stars 113 forks source link

Change store.initailize execution hook from useMemo to useEffect and add test for useInitializeExpoRouter #882

Closed naporin0624 closed 11 months ago

naporin0624 commented 11 months ago

Motivation

This PR proposes a modification in the interaction between the singleton "RouteStore" and React. Currently, the useMemo hook is being utilized, which is typically used for caching values (useMemo documentation). This PR suggests that the useEffect hook might be more suitable for this context, as detailed in its documentation.

Execution

To realize the proposed change, i undertook the following steps:

Test Plan

To substantiate these modifications, a comprehensive test plan has been devised. The testing strategy underscores:

The following strategies were employed during the testing phase:

  1. The store.initialize function was monitored to ensure it is called when there is a change in reference and context, further validated by rerendering with new context and location values.
  2. Observations were made to ascertain that the store.initialize function is not called repetitively when the same context is passed, fostering system efficiency.

These strategies were systematically executed using the renderHook function alongside the implementation of mock functions to mimic the behavior of the store.initialize and store.subscribeToStore methods, thereby offering a robust and reliable testing framework.

marklawlor commented 11 months ago

useEffect fires after the initial render, meaning the store will be initialised too late. If you use useEffect you'll now have two renders, one where no routes are rendered and then a second once the store is initialised.

naporin0624 commented 11 months ago

Thank you for taking a look.

It was good to learn more about why you are running side effects in useMemo.