Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
eatyourgreens opened this issue Jul 1, 2024 · 4 comments

Comments

@eatyourgreens
Copy link

const [viewState, setViewState] = useRecoilState(mapViewStateState);
useSyncMapUrl();
const background = useRecoilValue(backgroundState);
const showLabels = useRecoilValue(showLabelsState);
const { mapStyle, firstLabelId } = useBasemapStyle(background, showLabels);
const viewLayers = useRecoilValue(viewLayersState);
const viewLayersParams = useRecoilValue(viewLayersParamsState);
const interactionGroups = useRecoilValue(interactionGroupsState);
const fitBounds = useRecoilValue(mapFitBoundsState);
const resetFitBounds = useResetRecoilState(mapFitBoundsState);
useEffect(() => {
// reset map fit bounds whenever MapView is mounted
resetFitBounds();
}, [resetFitBounds]);

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
Copy link
Contributor

mz8i commented Nov 8, 2024

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 #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 #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
Copy link
Author

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

@mz8i
Copy link
Contributor

mz8i commented Nov 8, 2024

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
Copy link
Author

eatyourgreens commented Nov 8, 2024

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.
nismod/irv-jamaica#7 (comment)

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants