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

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

Closed
basuneko opened this issue Jul 7, 2023 · 8 comments · Fixed by #302 or #303
Closed

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

basuneko opened this issue Jul 7, 2023 · 8 comments · Fixed by #302 or #303
Assignees
Labels
bug Something isn't working released solved The problem presented is considered solved and the issue closed

Comments

@basuneko
Copy link

basuneko commented Jul 7, 2023

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

@basuneko basuneko added the bug Something isn't working label Jul 7, 2023
@diegoazh
Copy link
Owner

@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 diegoazh self-assigned this Jul 19, 2023
@create-issue-branch
Copy link

@diegoazh diegoazh added the WIP work in progress label Jul 20, 2023
@diegoazh diegoazh pinned this issue Jul 22, 2023
@diegoazh diegoazh unpinned this issue Jul 22, 2023
@diegoazh
Copy link
Owner

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.

@diegoazh diegoazh reopened this Jul 22, 2023
@diegoazh
Copy link
Owner

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.

@basuneko
Copy link
Author

basuneko commented Jul 22, 2023

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

@diegoazh
Copy link
Owner

diegoazh commented Jul 23, 2023

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.

github-actions bot pushed a commit that referenced this issue Jul 23, 2023
## [1.0.3](gmv3_v1.0.2...gmv3_v1.0.3) (2023-07-23)

### Bug Fixes

* **v3:** fix reactivity problems with google maps objects ([#303](#303)) ([0ef42e7](0ef42e7)), closes [#301](#301)
@diegoazh
Copy link
Owner

🎉 This issue has been resolved in version 1.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@diegoazh
Copy link
Owner

@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.

@diegoazh diegoazh added solved The problem presented is considered solved and the issue closed and removed WIP work in progress labels Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released solved The problem presented is considered solved and the issue closed
Projects
None yet
2 participants