Selecting a different Marker when a Marker already has it's Popup in view leads to a crash when using maplibre-gl 2.0.0-prerelease 6.

[joshuacrew]

Description

Selecting a different Marker when a Marker already has it's Popup selected leads to a crash when using maplibre-gl 2.0.0-prerelease 6. This does work however when using mapbox-gl v2.6.1.

Repro Steps

Here is a gif:

Here is a codesandbox with an example: https://codesandbox.io/s/trusting-bartik-zp0eh?file=/README.md

This code is taken from the Controls example.

Environment (please complete the following information):

• Library Version: 7.0.0-alpha.6
• Mapbox Version: 2.6.1
• Maplibre Version: 2.0.0-pre.6
• React Version: react 17.0.2

Logs

The above error occurred in the <Popup> component:

    at Popup (webpack://App/./node_modules/react-map-gl/dist/esm/components/popup.js?:18:66)
    at div
    at eval (webpack://App/./node_modules/react-map-gl/dist/esm/components/map.js?:46:81)
    at App (webpack://App/./src/app.tsx?:34:66)

Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://reactjs.org/link/error-boundaries to learn more about error boundaries.

Uncaught TypeError: popup._updateClassList is not a function
Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
A fix for this may be needed for #1646

[joshuacrew]

After a bit of research, _updateClassList is a function that was added to mapbox-gl-js here, but doesn't exist in the corresponding maplibre-gl-js code. I'm assuming the issue that brought about that change here might be related to this same bug, so a simple fix might just be applying similar changes to the maplibre codebase? cc @HarelM

[HarelM]

We can't copy code from mapbox unfortunately...

[joshuacrew]

We can't copy code from mapbox unfortunately...

Yeah I figured that might be an issue, which is why I was hoping similar changes might be able to be made. What tends to be the protocol for addressing react-map-gl API changes?

[HarelM]

It might sound stupid, but are you addressing this question to me?

[joshuacrew]

It might sound stupid, but are you addressing this question to me?

Yes I am 🙂 Sorry, I'm very new to the library and was wondering what you've had to do as a maintainer of maplibre-gl-js when the react-map-gl library has changed in the past and broke support with maplibre? Is it just an issue for react-map-gl to fix?

[HarelM]

No clue :-) I'm using angular and maintaining ngx-maplibre-gl, a fork of ngx-mapbox-gl precisely in order to avoid these kind of issues.
It's inevitable that the libraries will diverge, so keeping a single wrapper seems to me like wishful thinking. But I'll be happy to be proven wrong :-)

[joshuacrew]

No worries, I had seen you were quite active in the maplibre-gl-js repo and thought you'd be a good person to ask 🙂

[joshuacrew]

@Pessimistress Any idea if there's a route to this being sorted for v7.0.0? Happy to help if so.

[Pessimistress]

This was my fault. I thought I checked both mapbox v1 and v2 branches when I called private methods but apparently not for this component. The above PR switches to use public APIs instead. It could be slightly inefficient in some cases, but less likely to break.

To address the more generic question regarding compatibility - we want to support maplibre when possible. mapbox-gl will continue to add new APIs but so far they have been pretty good at not breaking existing (v1) ones, which means the part of maplibre-gl that is a subset of mapbox-gl features should continue to work. As maplibre-gl diverges further, I would expect it to keep backward compatibility as well, unless there is a good reason not to.

Part of the v7.0 rewrite was removing custom code and hard-coded mapbox-specific content. If a fork of react-map-gl is needed, I would think this effort has made it easier.

[Pessimistress]

This is fixed in 7.0.0-alpha.7.