yuzhva / react-leaflet-markercluster

React wrapper of the official Leaflet.markercluster for react-leaflet
https://yuzhva.github.io/react-leaflet-markercluster/
MIT License
284 stars 101 forks source link

chunkedLoading doesn't work on subsequent updates #76

Open shauravg opened 6 years ago

shauravg commented 6 years ago

I currently load the map with 0 markers while the marker data is being fetched from the backend. Once the markers are available, they are passed as children to MarkerClusterGroup. Even though I am loading quite a few data points (~10-15K) and using chunkInterval=50, I see no calls chunkProgress function.

If I don't render the map until the markers are available, then chunkedLoading works just fine. Let me know what details I could provide here to help solve this issue.

yuzhva commented 6 years ago

What version of react-leaflet-markercluster do you use - 1.8.1 or 2.0.0-rc3?

zach-ovic commented 6 years ago

I'm having exactly the same problem, running 1.8.1.

zach-ovic commented 6 years ago

I've just tested 2.0.0-rc3 and I'm seeing the same results...

zach-ovic commented 6 years ago

My code:

<MarkerClusterGroup chunkedLoading={true} chunkInterval={100} chunkProgress={this.chunkProgress}>
    {
        clientsById && (
            Object.keys(clientsById).reduce<React.ReactNode[]>((result, key) => {
                const client: Client = clientsById[key];

                // Don't show clients without location
                if (!client.location) {
                    return result;
                }

                // Don't show clients that are inactive if the filter is turned on
                if (!areInactiveUsersVisible && !client.isActive) {
                    return result;
                }

                result.push(<ClientMarker key={client.id} client={client} />);
                return result;
            }, [])
        )
    }
</MarkerClusterGroup>

PS. sorry for spamming

shauravg commented 6 years ago

I am using 1.1.8 and react-leaflet 1.8.2

shauravg commented 6 years ago

Another update. I noticed that addLayers (where chunkedLoading is supported) is not called when the markers are sent as new props to the already mounted map and markerclustergroup component. Not sure why.

When I use a ref to manually call addLayers even if the markerclustergroup is already mounted, then it works fine. I don't know enough about the react-leaflet architecture to know if this is valid behavior or not.

StazriN commented 6 years ago

Is there any progress on the issue?

yuzhva commented 6 years ago

@StazriN Did you try to use react-leaflet-markercluster@next next 2.0.0-rc3 version to reproduce that issue?

StazriN commented 6 years ago

Yes I did. Same problem. chunckedLoading works only with first render when I change location or number of markers chunkedLoading doesn't work and everything freezes.

radiallogic commented 6 years ago

I'm also having this issue on 2.0.0-rc3

ash-vd commented 5 years ago

Yep, same problem here (also 2.0.0-rc3).

olabalboa commented 5 years ago

Same issue for me. After looking at the code I notice that 2.0.0-rc3 doesn't support bulk adding of markers, see https://github.com/Leaflet/Leaflet.markercluster#bulk-adding-and-removing-markers.

Each time a child to MarkerClusterGroup is rendered the MapLayer from react-leaflet calls layerContainer.addLayer(). This causes severe performance issues for us when having about 2000 markers.

I forked this repo and modified the addLayer function to support bulk adding of many layers/markers (see code here https://github.com/olabalboa/react-leaflet-markercluster/blob/master/src/react-leaflet-markercluster.js).

Try it out if you like to see if it solves your problem. In that case I can open a pull request. Any comments or improvement suggestions are appreciated :-)

radiallogic commented 5 years ago

Hi, I've tested this and it works great. I've measured performance before and after but it makes a noticeable difference.

Thanks, Max B

On Tue, Nov 20, 2018 at 11:53 AM olabalboa notifications@github.com wrote:

Same issue for me. After looking at the code I notice that 2.0.0-rc3 doesn't support bulk adding of markers, see https://github.com/Leaflet/Leaflet.markercluster#bulk-adding-and-removing-markers .

Each time a child to MarkerClusterGroup is rendered the MapLayer from react-leaflet calls layerContainer.addLayer(). This causes severe performance issues for us when having about 2000 markers.

I forked this repo and modified the addLayer function to support bulk adding of many layers/markers (see code here https://github.com/olabalboa/react-leaflet-markercluster/blob/master/src/react-leaflet-markercluster.js ).

Try it out if you like to see if it solves your problem. In that case I can open a pull request. Any comments or improvement suggestions are appreciated :-)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/YUzhva/react-leaflet-markercluster/issues/76#issuecomment-440247126, or mute the thread https://github.com/notifications/unsubscribe-auth/ABbGQHj5WCGaorzffiREXzPvO8JJXaYxks5uw-1OgaJpZM4VcLI9 .

radiallogic commented 5 years ago

Apologies, I've sent my last mail too soon.

I've not measured performance before and after, but it makes a noticeable difference.

Thanks, Max B

On Tue, Nov 27, 2018 at 1:08 PM Max max@radiallogic.co.uk wrote:

Hi, I've tested this and it works great. I've measured performance before and after but it makes a noticeable difference.

Thanks, Max B

On Tue, Nov 20, 2018 at 11:53 AM olabalboa notifications@github.com wrote:

Same issue for me. After looking at the code I notice that 2.0.0-rc3 doesn't support bulk adding of markers, see https://github.com/Leaflet/Leaflet.markercluster#bulk-adding-and-removing-markers .

Each time a child to MarkerClusterGroup is rendered the MapLayer from react-leaflet calls layerContainer.addLayer(). This causes severe performance issues for us when having about 2000 markers.

I forked this repo and modified the addLayer function to support bulk adding of many layers/markers (see code here https://github.com/olabalboa/react-leaflet-markercluster/blob/master/src/react-leaflet-markercluster.js ).

Try it out if you like to see if it solves your problem. In that case I can open a pull request. Any comments or improvement suggestions are appreciated :-)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/YUzhva/react-leaflet-markercluster/issues/76#issuecomment-440247126, or mute the thread https://github.com/notifications/unsubscribe-auth/ABbGQHj5WCGaorzffiREXzPvO8JJXaYxks5uw-1OgaJpZM4VcLI9 .