search

maplibre / maplibre-react-native

A MapLibre react native module for creating custom maps
Other
261 stars 55 forks source link

MapView.getZoom() throwing TypeError in Alpha 25 & New Architecture #488

Open brentforder opened 2 days ago

brentforder commented 2 days ago

Steps to Trigger Behavior

  1. Create a simple screen with a MapView component and any map tile source
  2. Capture the MapView reference into a screen variable with useRef
  3. Create a simple handler method that calls mapViewReference.current.getZoom() and logs the output to console
  4. Assign the handler method to the MapView's onDidFinishLoadingMap property
  5. Configure the project for React Native's New Architecture (i.e. in app.json)

Expected Behavior

Actual Behavior

Screenshots (if applicable)

Screenshot_20241111-183901

Environment

Additional context

brentforder commented 2 days ago

@KiwiKilian Any chance you can look into this per your wonderful recent changes? Let me know if you need more information. Other than this issue, I'm noticing my app now loads much faster with RN New Architecture, so I'm excited to get this working!

KiwiKilian commented 2 days ago

Thanks for the report, we published the new arch support on alpha to get some more of these problems.

Could you please paste in a reproduction? I think I understood the setup, but it would make it much to have a repro.

From you description this is only about the new architecture. The changes between alpha 24 and 25 changed nothing about the new architecture – it only fixes the Camera behavior.

tyrauber commented 2 days ago

Thanks @brentforder, really appreciate you testing the new releases and reporting back.

Reverting to MLRN 10.0.0-alpha.24 might eliminate the issue as well, but there are many more serious issues with RN New Arch in that version It appears that MLRN 10.0.0-alpha.25 is much more stable with RN New Arch, but more cleanup is likely needed to get methods like getZoom() working

This is really interesting to me. The RN New Arch Changes were only in 10.0.0-alpha.24. There were only two changes in 10.0.0-alpha.25, neither should have effected stability with RN New Arch. Any issues in alpha.24 should exist in alpha.25.

10.0.0.alpha.25 commits:

It would be great if you could provide specifics as to why alpha.25 is more stable than alpha.24.

KiwiKilian commented 2 days ago

Can confirm, the imperative Methods of MapView are broken on Android with new Architecture. iOS works fine, also imperative Methods of Camera work on both platforms.

If anyone has an idea, where to look, feel free to comment. I can't tell, when I have capacity to investigate, switching to new arch is currently not the main priority on my project.

brentforder commented 1 hour ago

Thanks for the report, we published the new arch support on alpha to get some more of these problems.

Could you please paste in a reproduction? I think I understood the setup, but it would make it much to have a repro.

From you description this is only about the new architecture. The changes between alpha 24 and 25 changed nothing about the new architecture – it only fixes the Camera behavior.

I'm struggling to get a proper minimal reproduction example together for this case. It appears that the maplibre-react-native example setup/dependencies are currently broken (even without RN New Architecture enabled), I've also never been able to get the "Getting Started" example working in the last ~10 months (MapView never mounts). My project's working example is quite complex.

If you can get the official example app dependencies working, it should be easy to reproduce by:

  1. Basing on this official example:

  2. Adding these imports:

    import { MapViewRef } from "@maplibre/maplibre-react-native";
import { SymbolLayerStyleProps } from "@maplibre/maplibre-react-native/javascript/utils/MaplibreStyles";

  3. Adding a MapView reference hook to the screen/component:

    const mapViewRef = useRef<MapViewRef>(null);

  4. Adding the following method to the screen/component:

    const onDidFinishLoadingMap = async () => {
if (mapViewRef.current == null) {
  console.log(
    "can't process viewed map location because mapViewRef.current is null!"
  );
  return;
}
console.log("getZoom()");
const currentZoomLevel = await mapViewRef.current.getZoom();
console.log("currentZoomLevel: " + currentZoomLevel);
};

  5. Adding the following property to the screen/component:

    const markerStyle: SymbolLayerStyleProps = {
iconImage: ["get", "markerImage"],
iconSize: 1,
};

  6. Merging the following into the screen/component's JSX output:

    <MapLibreGL.MapView
ref={mapViewRef}
style={styles.map}
logoEnabled={false}
styleURL="https://demotiles.maplibre.org/style.json"
onDidFinishLoadingMap={onDidFinishLoadingMap}
>
<MapLibreGL.Camera
  defaultSettings={{
    centerCoordinate: [-119.47349414723843, 49.8406644754194],
    zoomLevel: 12,
  }}
/>
<MapLibreGL.Images
  images={{
    "example-pin": require("../../assets/pin.png"),
  }}
>
  <View></View>
  {/* Note: MapLibreGL.Images requires at least one child element */}
</MapLibreGL.Images>
<MapLibreGL.ShapeSource
  id="my-shape-source"
  shape={{
    type: "FeatureCollection",
    features: [
      {
        type: "Feature",
        id: "79d7b3f1-e3d1-4886-8fac-7351d9e64267",
        geometry: {
          type: "Point",
          coordinates: [-119.4922071, 49.8736052],
        },
        properties: {
          markerImage: "example-pin",
        },
      },
    ],
  }}
>
  <MapLibreGL.SymbolLayer id="icon-layer" style={markerStyle} />
</MapLibreGL.ShapeSource>
</MapLibreGL.MapView>

  7. Enabling RN New Architecture