Make shouldRevalidate work

[bkrmendy]

Problem

When navigating from a rendered route module to the same route module (for example, from /posts/0 to /posts/1 in the example project), the navigation doesn't happen and an error is thrown.

Root cause

DataRouteObject.shouldRevalidate is a way to optimize data revalidation in Remix (see https://remix.run/docs/en/main/route/should-revalidate for details). It's called after actions and on client-side navigations (this is our use case).

It's optionally exported from route modules, and a factory function called createShouldRevalidate (

utopia/editor/src/third-party/remix/client-routes.tsx

Line 143 in 37481e7

function createShouldRevalidate(

) wraps it into a higher-order function. The wrapping happens even if shouldRevalidate isn't exported from the route module. This createShouldRevalidate picks out the route to be validated from routeModulesCache, which is where our code needs to be improved since we never actually fill out routeModulesCache, and because of this, an invariant exception is thrown.

Fix

When creating the route modules in UtopiaRemixRootComponent, write the route module into CreateRemixDerivedDataRefsGLOBAL.routeModulesCache.current.

Details

As of this PR, the route module cache is a global variable (somewhat modeled on RemixBrowser, where the route module cache is a global on the window, window.__routeModules (https://github.com/remix-run/remix/blob/d5ec2f8e01527998470c98db6d15cb489823187e/packages/remix-react/browser.tsx#L139)). This is problematic for us because createShouldRevalidate also creates a variable that it captures (

utopia/editor/src/third-party/remix/client-routes.tsx

Line 148 in bdad305

let handledRevalidation = false

), and since we can have multiple Remix scenes, this variable can be written by otherwise unrelated shouldRevalidate calls. This might lead to revalidation not working correctly. Still, we should accept this as a known bug so that the problem above is fixed.

[github-actions]

Try me

[relativeci]

Job #8256: Bundle Size — 62.65MiB (~+0.01%).

f3cd2f9(current) vs 5957b21 master#8248(baseline)

⚠️ Bundle contains 64 duplicate packages

Metrics (2 changes)

	Current Job #8256	Baseline Job #8248
Initial JS	34.92MiB(~+0.01%)	34.92MiB
Initial CSS	0B	0B
Cache Invalidation	19.31%	18.34%
Chunks	27	27
Assets	31	31
Modules	3966	3966
Duplicate Modules	420	420
Duplicate Code	30.84%	30.84%
Packages	409	409
Duplicate Packages	64	64

Total size by type (1 change)

	Current Job #8256	Baseline Job #8248
CSS	0B	0B
Fonts	0B	0B
HTML	11.43KiB	11.43KiB
IMG	0B	0B
JS	62.64MiB (~+0.01%)	62.64MiB
Media	0B	0B
Other	0B	0B

---

View job #8256 report View fix/populate-route-modules-cache branch activity

[github-actions]

Performance test results:

[gbalint]

I think the compromise is acceptable, because shouldRevalidate is stored on the Remix route, which is stored in DerivedState, so it would be a really involved refactor to make that non-global. Basically we would have to move back plenty of code to UtopiaRemixRootComponent hooks.
But I think it you should add a TODO comment about this, so we will take this into account when we migrate to Remix V2, and we rethink what can happen in the derivestate and what needs to happen separately for the different remix scenes.