Queries are not reloaded when the environment changes

[steinybot]

The example code in the README for how to change the environment does not work.

export function MyRouter() {
  const environment = useRelayEnvironment();
  // Potentially unnecessary if you never change your environment
  const environmentRef = useRef(environment);
  useLayoutEffect(() => {
    environmentRef.current = environment;
  }, []);

  const router = useMemo(() => {
    const routes = preparePreloadableRoutes(MY_ROUTES, {
      getEnvironment() {
        return environmentRef.current;
      },
    });

    return createBrowserRouter(routes);
  }, []);

  return <RouterProvider router={router} />;
}

The queries are not reloaded and relay will print the following warning:

Warning: usePreloadedQuery(): usePreloadedQuery was passed a preloaded query that was created with a different environment than the one that is currently in context. In the future, this will become a hard error.

See a reproduction here: https://github.com/steinybot/react-router-relay/blob/bug/stale-environment/examples/todo/src/MyRouter.tsx

[steinybot]

Even the naive code that recreates the router every time the environment changes still doesn't pickup the new environment which is even more concerning:

export function MyRouter() {
  const environment = useRelayEnvironment();

  console.debug('MyRouter', getId(environment));

  const routes = preparePreloadableRoutes(MY_ROUTES, {
    getEnvironment() {
      console.debug('preparePreloadableRoutes getEnvironment', getId(environment));
      return environment;
    },
  });

  const router = createBrowserRouter(routes);

  return <RouterProvider router={router} />;
}

Reproduction: https://github.com/steinybot/react-router-relay/blob/bug/stale-environment-2/examples/todo/src/MyRouter.tsx

I printed a unique id for each environment and you can clearly see here that when the entry point root rerenders it has a query with a stale environment:

[steinybot]

This is a bit strange. getPreloadProps is definitely being called and the query has the new environment but when the RouterProvider re-renders it has the old state.

[steinybot]

The first issue that I have noticed is that startNavigation is called and it will call the loader. RouterProvider will render before completeNavigation has been called and it will have an empty state. Control will return back to startNavigation and then completeNavigation then updateState is called which will call setState in the RouterProvider and it will now have the correct loaderData. Now RouterProvider will rerender and pass the loaderData down to the route as expected. During first load, this intermediate render is fine, there is no state yet and so the route is not rendered. The issue is that when the environment changes and the router is recreated, this intermediate render will have the old state and so the route will render with the previous query.

I haven't tracked down exactly why startNavigation is yielding and allowing the RouterProvider to render but that might be an issue with React Router itself.

What I haven't figured out yet is why setState in RouterProvider is not being called with the new loaderData.

[steinybot]

Hmm I'm not sure what has changed but now I am getting a 3rd render which does include the updated state. The intermediate renders with the old state are going to be problematic.

[steinybot]

This will be fixed in remix-run/react-router#11301. The example will still be wrong. You must create a new router when the environment changes. The router state must go back to initialized = false to prevent the routes from being rendered with old data.

A workaround is to put a key on the RouterProvider:

function usePrevious<A>(a: A): A | undefined {
  const previousRef = useRef<A | undefined>(undefined)
  useEffect(() => {
    previousRef.current = a
  })
  return previousRef.current
}

function useChangeBit<A>(a: A): boolean {
  const previous = usePrevious(a)
  const previousChangeBitRef = useRef<boolean>(false)
  const isSame = Object.is(previous, a)
  const changeBit = isSame ? previousChangeBitRef.current : !previousChangeBitRef.current
  useEffect(() => {
    previousChangeBitRef.current = changeBit
  });
  return changeBit
}

function useChangeKey<A>(a: A): Key {
  const changeBit = useChangeBit(a)
  return changeBit ? 1 : 0
}

export function MyRouter() {
  const environment = useRelayEnvironment();

  // If the environment changes then flip the bit.
  const environmentChangeKey = useChangeKey(environment)

  console.debug('MyRouter', getId(environment));

  const routes = preparePreloadableRoutes(MY_ROUTES, {
    getEnvironment() {
      console.debug('preparePreloadableRoutes getEnvironment', getId(environment));
      return environment;
    },
  });

  const router = createBrowserRouter(routes);

  return <RouterProvider key={environmentChangeKey} router={router} />;
}

[steinybot]

I just spent about half a day trying to figure out why the workaround wasn't working. preparePreloadableRoutes mutates the passed in routes and sets the loader which includes the environmentProvider. When preparePreloadableRoutes is called again it will not update the loader to use the new environmentProvider meaning that it retains the old environment in its closure. That is so evil 🤬.

That'll be why the original example in the readme uses a ref and does not update it purely in a useEffect like it really ought to. render must be pure and that means no updating refs.

[kejistan]

So the example used a ref just to avoid recreating the routes and recreating the router on environment changes. It sounds like we do always need to do that though. In our production system it looks like we do recreate the router when the environment changes. Definitely open to changing the readme to better outline changing the environment.

preparePreloadableRoutes mutates the passed in routes and sets the loader which includes the environmentProvider. When preparePreloadableRoutes is called again it will not update the loader to use the new environmentProvider meaning that it retains the old environment in its closure.

I'm not sure I follow this part, preparePreloadableRoutes shouldn't be mutating anything, it creates a new set of routes based on what was passed in (the code is pretty simple), so it shouldn't leak things between different calls or modifying the passed in routes.