diegoazh/gmap-vue

Bug: impossible to remove markers with v-if or v-for

Closed this issue · 8 comments

basuneko commented

Please complete all sections that you can but please don't remove any section, otherwise the bot will close your issue because it doesn't meet our issue template (you can remove this comment)

Describe the bug

Hi, I'm working a project where the user can select things and corresponding markers should be added/removed from the map.
I have code similar to this:

<GmvMap>
  <GmvMarker v-if="showMarker" ... />

  <GmvMarker v-for="marker in currentlyVisibleMarkers" ... />
</GmvMap

I can add markers to the map with no problem. But when I try to remove them (e.g. by setting showMarker to false, or by changing the currentlyVisibleMarkers array), the markers remain on the map.

💡 Updated:
After a lot of monkey-patchning I believe I've gotten to the bottom of it: google maps api doesn't work well with proxies.

// components/marker-icon.vue
onUnmounted(() => {
  ...
  } else if (markerInstance.value) {
    console.log(markerInstance.value) // this is a proxy object.
    markerInstance.value.setMap(null);
  }
});

The above code is executed correctly. However, the markerInstance.value object is not the actual google maps api' marker - it's a reactivity proxy. I'm not sure if the issue is in the maps api or in vue reactivity system but it just doesn't do what it's supposed to.

Fix

The solution seems to be to unproxify the object:

const rawMarkerInstance = toRaw(markerInstance.value)
rawMarkerInstance.setMap(null);

And this seems to work as expected.

To reproduce

Code sandbox
github
(branch: gmap-vue-bug-repro)

Steps to reproduce the behavior:

  1. Open the code sandbox repro
  2. Enter your google maps api key, the page will reload
  3. click "Show marker", "Show markers list"
  4. click "Hide marker", "Hide markers list"

Expected behavior

The markers should appear and then get removed

Current behavior

The markers appear but never get removed from the map

Screenshots

Desktop (please complete the following information)

  • OS: MacOS
  • Browser Chrome
  • Version 114.0.5735.198

Smartphone (please complete the following information)

Additional context

Versions

  • Node: v20.2.0
  • NPM: 9.7.1

Package manager

NPM

Plugin version

@gmap-vue/v3@1.0.1

diegoazh commented

@basuneko thank you for reporting it I'll take a look at it as soon as possible but, if you want you can send a PR, your contribution is welcome here 😉

diegoazh commented

Again, thank you for reporting this issue. Thanks to this problem I found a way to have a dev server using the same examples that I used before to make e2e tests, and also I found other minor issues and improvements while I was working on this bug.

On the other hand, first, the issue was not caused by the proxy object, it was caused because I omitted to exclude only null and undefined values from props, you can check the fix here.

Then, on the marker component you have a prop called visible, you should use it to manage the visibility of the markers rather than the options prop; I only left the options props available as a wildcard, to anticipate possible changes on the Google Maps API, if they added new properties or they changed the current names we can use the options props to work without need to wait until a fix or a new release of the plugin.

<!-- In your example you should use a visible prop like the example below -->
    <div class="map">
      <GmvMap :center="{ lat: 10, lng: 10 }" :zoom="7" map-type-id="terrain">
        <GmvMarker v-if="showMarker" :position="{ lat: 10, lng: 10 }" :visible="markersVisible" />

        <GmvMarker v-for="(marker, index) in markersList" :key="index" :position="marker" :visible="markersVisible" />
      </GmvMap>
    </div>

Finally, I added two new e2e tests to guarantee the correct behaviour of the visible prop on markers.

You can test this new version 1.0.2, let me know if you find something new.

Regards.

  • ❤️1
diegoazh commented

Sorry, after finding the other issue I miss the other part of this bug, the unMounted behaviour, it was my fall 🤦🏻‍♂️. Today later a will land another fix maybe in all components if this behaviour is common to all proxies objects. Thank you for your research.

  • ❤️1
basuneko commented

@diegoazh wonderful, thanks 🙌 I was just setting up gmap-vue locally to start working on a PR

diegoazh commented

After investigating js Proxies, I found this post on the Vue repository. After debugging the unMounted behaviour I found there were many closures and this is the cause of this behaviour.
After thinking all night, I decided to make a more deeply refactor, I'm going to remove all reactivity from all Google Maps objects and leave them as the original raw objects. If all tests pass I will land this new version very soon, but these changes maybe will take more time than I guess.

diegoazh commented

🎉 This issue has been resolved in version 1.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

diegoazh commented

@basuneko new version landed.
I added the tests that you made in the sandbox code to our e2e tests and now markers work well with v-if and v-for directives. Please test the new version and if you find something new report it again, also PRs are welcome too.
Regards.

  • ❤️1