search

NathanAP / vue-google-maps-community-fork

The community fork for Vue Google Maps library
https://vue-google-maps-community-fork.netlify.app
MIT License
107 stars 32 forks source link

Cluster component gives error #18

Closed danix89 closed 1 year ago

danix89 commented 1 year ago

Codepen support Unfortunately, I'm not able to make the code work on Codepen.

Describe what you're reporting I suppose that a recent update of the VueJs is conflicting with the component code. The library was correctly grouping the markers in clusters until a couple of weeks ago. Now, the markers are no more shown on the map due to the following error:

bindProps.js?2403:32 Uncaught (in promise) Error: setMaxZoom is not a method of (the Maps object corresponding to) undefined
    at bindProps (bindProps.js?2403:32:1)
    at eval (build-component.js?b97c:108:20)

Note that if you try to make the markers be rendered directly on the map (i.e., removing the Cluster component and using the Marker component directly), the problem is not showing up.

How can we reproduce it? You should reproduce it using the example code that is in the README file in the Cluster component section.

What were your expecting to happen? The marker should be grouped correctly in clusters and shown on the map without the above error.

Device or environment

AlexDanault commented 1 year ago

I also have this issue when using the Cluster.

NathanAP commented 1 year ago

Hello! Thanks for reporting this problem, sorry for the delay, its holiday where I live today.

I suppose that a recent update of the VueJs is conflicting with the component code

Can you remember the last version you were using before? I'll check if something changed in Vue that could affect us and see if something in setMaxZoom could be broken as soon as I can.

danix89 commented 1 year ago

Hi @NathanAP!

Hello! Thanks for reporting this problem, sorry for the delay, its holiday where I live today.

Never mind. It's not an emergency, and you've responded quickly to the issue ticket. I've temporarily solved removing the Cluster component.

Can you remember the last version you were using before?

Tomorrow, I've tried to downgrade to vue@3.2.31 but the problem is continuing to present. So, apparently, I was wrong about the VueJs conflict.

Hoping it will be more helpful, however here it is my dependencies:

 "devDependencies": {
    "@inertiajs/inertia": "^0.11.0",
    "@inertiajs/inertia-vue3": "^0.6.0",
    "@inertiajs/progress": "^0.2.7",
    "@tailwindcss/forms": "^0.5.0",
    "@tailwindcss/typography": "^0.5.2",
    "@vue/compiler-sfc": "^3.2.31",
    "axios": "^0.21.1",
    "browser-sync": "^2.26.13",
    "browser-sync-webpack-plugin": "^2.3.0",
    "cross-env": "^7.0",
    "eslint": "^7.18.0",
    "eslint-config-standard": "^16.0.2",
    "eslint-plugin-import": "^2.22.1",
    "eslint-plugin-node": "^11.1.0",
    "eslint-plugin-promise": "^4.2.1",
    "laravel-mix": "^6.0.18",
    "laravel-vapor": "^0.6.0",
    "lodash": "^4.17.19",
    "postcss-import": "^14.0.2",
    "resolve-url-loader": "^5.0.0",
    "sass": "^1.32.5",
    "sass-loader": "^8.0.2",
    "tailwindcss": "^3.0.0",
    "vue-loader": "^17.0.0",
    "vue-template-compiler": "^2.6.12",
    "webpack-cli": "^4.5.0",
    "webpack-node-externals": "^3.0.0"
},
"dependencies": {
    "@vue/reactivity": "^3.2.20",
    "@vue/runtime-core": "^3.2.20",
    "@vuepic/vue-datepicker": "^3.1.0",
    "crypto-js": "^4.1.1",
    "js-cookie": "^2.2.1",
    "laravel-vue-i18n": "^1.4.3",
    "luxon": "^2.2.0",
    "suggestags": "^1.25.0",
    "tiny-emitter": "^2.1.0",
    "vee-validate": "^4.5.11",
    "vue": "v3.2.31",
    "vue-google-maps-community-fork": "^0.1.4",
    "vue-multiselect": "^3.0.0-alpha.2",
    "vue-router": "^4.0.12",
    "vue-simple-addthis-share": "^0.1.5",
    "vue3-loading-overlay": "^0.0.0",
    "vue3-touch-events": "^4.1.0",
    "vuedraggable": "^4.1.0",
    "vuex": "^4.0.0",
    "webpack-s3-plugin": "^1.2.0-rc.0",
    "yup": "^0.32.11"
}
NathanAP commented 1 year ago

OK, I found out some information about this issue:

@tahaipek , @lugrinder , @YassineChe sorry to ping you guys directly. Did you find something about this problem?

YassineChe commented 1 year ago

@NathanAP,

Since i posted the issue i tried many solutions but none of 'em worked, Hopefully i'm using this library for admin side, until the owner wake up and fix it 😉

Keep me up to date. Thanks.

NathanAP commented 1 year ago

@YassineChe I recommend you read this. Fawmi never answered any of us since July. We forked to keep this library alive, many bugs are fixed in our fork.

NathanAP commented 1 year ago

Some more research:

Working case: image

Not working case (when we get the error): image

There is a lot of cases it is using correctly, so I guess that setMaxZoom is not used anymore or was changed for something else.

danix89 commented 1 year ago

The history behind this bug seems to be related to changing the googlemaps/markerclusterer dependency. Fawmi used another called markerclustererplus but it is deprecated now. Fawmi changed to what they recommended.

You're totally right @NathanAP!

I've also debugged the code, and as you pointed out it obviously couldn't work due to the change of the new class used for rendering the clusters on the map. The bindProps method (that you mention here), that is called from here, tries to bind the Cluster component properties to the relative googlemaps/markerclusterer getters and setters methods raising the error.

As you can see from here, this is due to the lack of the setters and getters for the Cluster component props in the new class googlemaps/markerclusterer.

So, the bindProps function can't work for the Cluster component. Maybe, isn't anymore necessary for this component?

NathanAP commented 1 year ago

If thats the case, in 

// src / components / cluster.vue
maxZoom: {
    type: Number,
    twoWay: false,
  },

we could just add noBind: true, because in

src / utils / bindProps.js

line 24: if (noBind) continue

... Right? 🤔🤔🤔🤔🤔

I will test it tomorrow!

danix89 commented 1 year ago

we could just add noBind: true, because in

Then it should be added to all the Cluster props because none of these props hasn't a setter/getter method in that class. However, I tried brutally removing all the props from the component, and this only solves the error rising but the clusters aren't rendered.

I will test it tomorrow!

Of course, it's too late to continue thinking about it 😅

NathanAP commented 1 year ago

However, I tried brutally removing all the props from the component, and this only solves the error rising but the clusters aren't rendered.

Yeah, I don't think it will resolve because we need to reference the map that we wanna show the clusters...

Anyway, I'm investigating the code again. It seems that when we are using the MarkerCluster, that googleMapsInst variable is not what it should be, so I guess Fawmi didn't adapt the code or they changed later.

For example, if we only use a simple Marker and keep track of googleMapsInst, all of his getters and setters are there correctly, but not for MarkerClusters. Those variables are being set in this part of the code. These are the actual getters and setters when we use MarkerCluster:

image

Most of these getters and setters seems related to Map component. I guess there is something wrong with googleMapsInst then?

Edit: my mistake in the last part. I changed the image I first uploaded.

danix89 commented 1 year ago

Anyway, I'm investigating the code again. It seems that when we are using the MarkerCluster, that googleMapsInst variable is not what it should be, so I guess Fawmi didn't adapt the code or they changed later.

You get the point! In particular, there is no bind between the buildComponent function argument ctrArgs of the cluster component and the constructor of the new class MarkerCluster. In fact, the latter is using destructured parameters so the ctrArgs (as it has been defined in the cluster.vue) will pass to this constructor the wrong parameters.

These are the actual getters and setters when we use MarkerCluster:

Yes, as you can see from your screenshot, there're no setters or getters. That is the problem that leads to the setMaxZoom error.

However, I'm currently working on the fix to work with the new class constructor. Also, the fix will permit to define a custom render function (default DefaultRenderer) and choose the clustering algorithm (default SuperClusterAlgorithm).

Within today, I will make a PR.

NathanAP commented 1 year ago

Yeah, I start to make some console.log in the entire build-component.js and notice that the bindProps arguments were almost in the wrong format / class.

However, I'm currently working on the fix to work with the new class constructor. Also, the fix will permit to define a custom render function (default DefaultRenderer) and choose the clustering algorithm (default SuperClusterAlgorithm).

Thanks you so much and good luck with the PR! If you need something please let us know.

YassineChe commented 1 year ago

@NathanAP

Hello again, any news about Clustering ?

danix89 commented 1 year ago

As promised, I made PR #19 which ends the porting started by Fawmi to the new MarkerClusterer class and should fix the setMaxZoom error!

Edit: I forgot to add the link to the PR in the comment.

NathanAP commented 1 year ago

@NathanAP

Hello again, any news about Clustering ?

Almost sure that today is the last day of this problem!

YassineChe commented 1 year ago

@NathanAP Great, let's up to date, thanks Happy coding!

NathanAP commented 1 year ago

Fixed by #19 and its up in 0.1.5. You can download the new version :) Edit: docs were adapted in #20 . Edit 2: official documentation and changelogs are up! Edit 3: the correct version is 0.1.6.

If you need help or find some bug, please feel free to open another issue!

YassineChe commented 1 year ago

@NathanAP

Still getting 

Uncaught (in promise) Error: setMaxZoom is not a method of (the Maps object corresponding to) undefined
    at bindProps (bindProps.js?v=e9f5f753:32:13)
    at build-component.js?v=e9f5f753:108:11

Even i added :maxZoom="10" in the latest version ^0.1.5

Any suggestions ?

NathanAP commented 1 year ago

Sorry guys, I made a mistake when I publish the 0.1.5 into npm. My mistake.

I'm fixing it right now, will update this comment when everything is ok.

Edit: its up. You can update to 0.1.6. I'm sorry for the mistake ): my terminal didn't switch branch after the PR, so the Github files were correct, but the npm version was in another branch. Its fixed.

YassineChe commented 1 year ago

@NathanAP , You did a great job! Its working now! Just a quick question is there is any way to customize cluster Color ? Thanks.

danix89 commented 1 year ago

@YassineChe of course you can. And not only the color.

Note that the component properties have been changed since version 0.1.5.

Please, have a look at PR #19. In particular, I suggest having a deep look at the @googlemaps/markerclusterer documentation linked at the end of the PR. Here you can find more examples of the render function you could implement to change marker cluster color.

NathanAP commented 1 year ago

I think we could add a color change example later in the docs!

YassineChe commented 1 year ago

@NathanAP Yes, Please

ivan006 commented 5 months ago

Still having this issue in 


    "vue-google-maps-community-fork": "^0.3.1",