visgl / react-map-gl

React friendly API wrapper around MapboxGL JS
http://visgl.github.io/react-map-gl/
Other
7.8k stars 1.35k forks source link

How to reduce bundle size of mapbox-gl? #476

Closed Stupidism closed 6 years ago

Stupidism commented 6 years ago

image As you can see, the size of mapbox-gl is over a quater of my app.

ibgreen commented 6 years ago

I am not sure that there is much that we can do, your graph shows clearly that the fundamental issue is mapbox-gl itself which react-map-gl is just wrapping.

Looking at your chart, mapbox is 503KB and react-map-gl adds 42KB on top of that. It would of course be nice to reduce react-map-gl's overhead but it would not really move the needle in terms of your total bundle size.

Probably makes most sense for you to discuss size reduction techniques directly with the mapbox team?

Stupidism commented 6 years ago

https://github.com/mapbox/mapbox-gl-js/issues/6320 @ibgreen Thank you. I proposed another issue in mapbox-gl-js

M1ke commented 5 years ago

@ibgreen this seems the best place to ask this rather than opening a new issue. Has it been considered for consumers of react-map-gl to be able to omit the import of mapbox-gl itself in favour of that file being pulled into the browser as normal via Mapbox's CDN? This would dramatically reduce the bundle size of the React applications using it, take advantage of browser caching (i.e. user visits any site with Mapbox and they might have Mapbox GL cached for when they arrive at your own site) and move the data transfer servicing costs to Mapbox. Is that feasible within the scope of what's being handled via React Map GL?

ibgreen commented 5 years ago

@M1ke First let me say that like any service, CDNs are flaky and while fine for demos etc, relying on CDNs for production applications has never been an option in my experience.

The first time your app is not available to users because a CDN is down tends to be the last time your app relies on CDNs.

But perhaps things are improving in this regard?

That said, we did consider separating out the import of mapbox-gl for other reasons, namely to allow the user to upgrade the mapbox version without upgrading react-map-gl, although there were concerns about incompatible versions.

An idea was that you would pass mapbox as a prop to react-map-gl.

Doing that it would potentially enable injecting mapbox via script instead of importing it from mapbox' npm module, and leave the choice to you.

This would require more configuration for all users though, as everyone else would now need to add and maintain an additional dependency in their `package.json.

I think that is as far as we got in our thinking at the time...

M1ke commented 5 years ago

In terms of relying on a CDN I'm not sure the point is valid for Mapbox - you're already depending on the entirety of Mapbox's service being up, and their normal mechanism for users to install their platform is providing a <script> element that loads the JS file from their CDN. This is the same for many services, Google Maps, Stripe, Facebook etc. Very few encourage you to build in their script to your own application.

I can see the reasoning here; a simple solution could be to split out entry points, so one entrypoint loads the main component by importing mapboxgl and then creating the component, and another could require the component be passed a prop. Then a calling scope can choose to grab mapbox from the browser's global state, or can use it as bundled.

Given this is a 500-600Kb reduction in app size I do imagine people could be interested. We likely need to do this anyway so I may see if I can implement it in react-map-gl rather than having to create our own mapbox implementation in React. If I manage I'll send you the code to see if you're interested in a PR.

ibgreen commented 5 years ago

Good point, and yes, it would certainly be a stronger case if the availability of the script CDN and the mapbox vector tile service itself were strongly correlated.

As a side note, we actually have a lot of internal applications that can continue to run without the base map (primarily our deck.gl based visualizations, that are enhanced by, but not dependent on, the mapbox base map).

So - if we could make the script loading fail "gracefully" (i.e. map doesn't render but app still runs, just without basemap), it could be an interesting option for us.

Ideating on this:

And yes, some decisions would be if we keep this a breaking API change for v6, requiring users to separately install mapbox, or if try to offer a bunch of different entry points as you suggested.

Ultimately, in the case of react-map-gl, this decision is in the hands of @pessimistress.

That said, I have similar thoughts about offering script based loading for some of our loaders.gl modules, so perhaps we can come up with a unified guideline that we apply in similar situations across the vis.gl framework suite.

Pessimistress commented 5 years ago

@M1ke We use webpack's externals config to omit mapbox-gl:

externals: {
  'mapbox-gl': 'mapboxgl'
}

The mapbox script tag must appear before your own bundle.

M1ke commented 5 years ago

Thanks, I'll see how this affects things. If it works then maybe all the library needs is a reference to this somewhere in its documentation.

derwaldgeist commented 4 years ago

Stumbled upon this issue, since the bundle size is pretty critical to us, as we're developing a mobile app, where react-map-gl is displayed inside a Unity web view. This package is - by far - the largest module we're using (adds up to 711kb minified, which is huge and about 1/3 of the entire app). Would be cool to mention this caveat in the docs, so users are aware of this. Although we love the component, we're considering a switch to Leaflet now, which has a way smaller footprint.

ibgreen commented 4 years ago

Would be cool to mention this caveat in the docs, so users are aware of this.

We are planning to mention the ability to use the webpack externals config in the docs, but we usually don't update the website until we make a new major or minor release. It this mainly a question around wanting some confirmation from us that this technique will continue be supported?

Although we love the component, we're considering a switch to Leaflet now, which has a way smaller footprint.

Understood. Even though you can use the webpack externals trick to make your own bundle "look small", the client device will still need to download the same amount of code, now just in two requests (your app with the small react-map-gl wrapper + the full mapbox library) instead of one.

If there is anything else you think we can do to improve things on our end, let us know.

derwaldgeist commented 4 years ago

We are planning to mention the ability to use the webpack externals config in the docs

This only works for webpack users, right? I use Meteor for bundling my app, so this wouldn't help me much.

Besides, as you already mention, it still requires the app user to download the entire package from a CDN. It's still huge. I think the only way to get this down is to modularise mapbox-gl itself, then.