search

nismod / irv-frontend

IRV frontend
MIT License
2 stars 0 forks source link

Suspending the `MapViewContent` component will crash the map with React 18 #61

Open eatyourgreens opened 4 months ago

eatyourgreens commented 4 months ago

https://github.com/nismod/irv-frontend/blob/e32a9ba0fc52b8fff80a25e10c4f9d1f95b08d75/src/map/MapView.tsx#L26-L44

Heads up that the Recoil state here works fine with React 17, but crashes with the new createRoot(node).render() in React 18. Moving the Recoil state down into individual components fixes it. I think that the React Map GL Map component crashes if it renders while suspended, but only with the new concurrent rendering API.

See also:

mz8i commented 2 weeks ago

Moving to React 18 in general would be desirable, however at this point this has been held back by the dependencies of react-spring-bottom-sheet: (see rationale in https://github.com/nismod/irv-frontend/pull/18 ). Does the bottom drawer on a mobile layout work despite that when updating?

If we managed to replace this by vaul as discussed in https://github.com/nismod/irv-frontend/issues/17 - I think that would remove the last blockers to an update.

However, there has been an issue with react-map-gl + deck.gl + react 18 that hasn't been resolved in a long time (something to do with 18's batching of updates and its impact on map interactivity) - we'd have to check if this appears when updating.

eatyourgreens commented 2 weeks ago

I haven't seen any problems on https://jamaica.infrastructureresilience.org, since moving that code to React 18 in June.

mz8i commented 2 weeks ago

That's great to hear! I was under the impression that I did run into some issues when trying to update it, but that was long ago so a lot could have changed. Hopefully it will be safe to update for this project, too (after implementing the fix you mention above)

eatyourgreens commented 2 weeks ago

I have run into a bug, but only in production, where Recoil's urlSyncEffect doesn't always write state changes to the URL. I'm not sure if that's because of the upgrade to React 18, or if maybe it was a bug with React 17 too. I added the URL sync code after upgrading React. https://github.com/nismod/irv-jamaica/issues/7#issuecomment-2292227651

irv-jamaica now has these long URLs that preserve sidebar state (and selected feature state.) I don't know if that would easily port across to irv-frontend.

eg. https://jamaica.infrastructureresilience.org/risk?lat=18.01903&lon=-76.9601&zoom=11.71&assetsStyle=damages&assets=true&hazards=true&fluvial=false&surface=false&coastal=false&cyclone=false&buildings=false&buildingsStyle=type&regions=false&regionsStyle=boundaries&hazardsStyle=&terrestrial=false&terrestrialStyle=landuse&marine=false&marineStyle=habitat&risksStyle=lossGdp&risks=true&netTree=01.02.03.04.05.06.07.08.09.0a&selectedassets=elec_nodes_substation.1100021806