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

Bulk adding and removing Markers #7

Open
friedPotat0 opened this issue Dec 3, 2017 · 14 comments · May be fixed by #8
Open

Bulk adding and removing Markers #7

friedPotat0 opened this issue Dec 3, 2017 · 14 comments · May be fixed by #8

Comments

@friedPotat0
Copy link

friedPotat0 commented Dec 3, 2017

Is there any way to achieve bulk adding/removing of markers to a cluster group like it's described in the Leaflet.markercluster doc?
https://github.com/Leaflet/Leaflet.markercluster#bulk-adding-and-removing-markers

I tried to reference the v-marker-cluster component and call the function contained in the mapObject to add the markers this.$refs.cluster.mapObject.addLayers(<MARKER_ARRAY>). But it throws an error saying 'layer.addEventParent is not a function'.

@jperelli
Copy link
Owner

I might be implementing this somehow in this days. Seems a good feature to have. Thanks for the report, I'll let you know any news.

@jperelli jperelli self-assigned this Jan 31, 2018
@jperelli jperelli linked a pull request Jan 31, 2018 that will close this issue
@jperelli
Copy link
Owner

@friedPotat0 @nickforddesign I made a pull request #8 can you help me test if that works to fix this issue? you can checkout the branch and test it locally using the example app

@skinnyjames
Copy link

skinnyjames commented Feb 19, 2018

@jperelli
I'd love to test this out with a vue app using a lot of markers, but I can't get the bulk_test branch to build. build successful with yarn

@skinnyjames
Copy link

I'm getting this error - not exactly sure why though :(

[Vue warn]: Error in beforeDestroy hook: "TypeError: Cannot read property 'removeLayer' of undefined"

setVisible: function setVisible(newVal, oldVal) {
      if (newVal == oldVal) return;
      if (this.mapObject) {
        if (newVal) {
          this.mapObject.addTo(this.parent);
        } else {
          this.parent.removeLayer(this.mapObject);
        }
      }
  }

@friedPotat0
Copy link
Author

friedPotat0 commented Feb 19, 2018

@jperelli Thanks for trying to implement the feature.

I was able to build the example successfully with Poi.
After deployment the example took about 1 minute to completely show all the markers on the map. The site was not responding at all during this period. I also tested the chunk options mentioned in Leaflet.markercluster doc but it seems like the page is just locked until all markers are added successfully to the map. I logged the values in the chunkProgress-function to see the progress but there is only one output in the console at the very start of the adding progress. Most times I receive a "Paused before potential out-of-memory crash" from the Chrome dev tools and the page doesn't load up at all.

The original markercluster doc has two examples for bulk adding with chunk progress: One with 10,000 and one with 50,000 markers to add (https://github.com/Leaflet/Leaflet.markercluster#user-content-handling-lots-of-markers). The 50,000 marker example only takes about 5 seconds to load all markers!

@jperelli
Copy link
Owner

ok, thanks for the reports, evidently the proposed solution is not working. I'm thinking if this is a problem with the implementation I made or if this is a problem with vue's v-for performance. Can't find any help on fixing vue's v-for performance easily, so I'll check my code and let you know if I can improve it somehow.

Any comments on the pull request are infinitely welcomed :)

@friedPotat0
Copy link
Author

The _add function in the Vue2LeafletMarkercluster component is called twice. Once in the mounted function and a few seconds later in the watcher for the options prop again. That is the reason why I got only one output of the chunkProgress-function. The second _add destroyed or stacked with the first adding progress in any way. After commenting out the options watcher I get a page load with the progress logged in every interval when setting the clusterOptions to:

clusterOptions: { chunkedLoading: true, chunkInterval: 200, chunkDelay: 50, chunkProgress: function (processed, total, time) { console.log(processed, total, time) } }

But the overall page loading time is still very bad in comparison to the example.

@friedPotat0
Copy link
Author

friedPotat0 commented Feb 19, 2018

Okay, figured out that there is a setTimeout function in the example component which causes the options prop to be rewritten after 5 seconds. If you remove these lines the bulk adding only gets called once when mounting the markercluster. Probably the function is there for testing purposes only?!

@jperelli
Copy link
Owner

Right, the setTimeout was for testing purposes. I will make two test files, one only for bulk. I'll modify the PR today in a couple of hour. Thank you for your help!

@friedPotat0
Copy link
Author

No problem. Please keep in mind that the build command of vue-cli is no longer useable for building your examples. When using vue build somefile.vue, like it's written in your package.json for building, it throws an error saying:

We are slimming down vue-cli to optimize the initial installation by removing the vue build command.
Check out Poi (https://github.com/egoist/poi) which offers the same functionality!

I fixed this by installing Poi npm i poi -D and adding a new index.js to the main script in the package.json.
Content of the index.js:
`import Vue from "vue";
import Example from "./example";

new Vue({
el: "#app",
render: h => h(Example)
});`

After that I could build the example by running npx poi build.

Hope this helps not only for the bulk testing but for the normal example, too.

Alternatively the process could also be done with yarn like @skinnyjames mentioned earlier but I didn't test it.

@jperelli
Copy link
Owner

I modified the examples, but couldn't improve performance.
Maybe vue or vue2leaflet are adding some overhead.

Please let me know if the new Pull Request is ok or if you see that it runs slow
run with:

yarn example_bulk

@skinnyjames
Copy link

I got marker clusters to work well with vue as a standalone library. If it helps, I can take a closer look soonish!

@jperelli
Copy link
Owner

Yes please :)

@skinnyjames
Copy link

Hi all,
I looked into it, and I redesigned a bit how vue2leaflet components are initialized
(using an event bus instead of deferredMountedTo)
It might not be the best way, but components like marker cluster, are working a lot faster with lots of markers.
There is a provisional working demo at https://github.com/skinnyjames/mapa

git clone https://github.com/skinnyjames/mapa
npm install
cd examples && poi app.js

Would love to hear thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants