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

Consider removing mapboxgl-js as a depedency #31

Closed
vicapow opened this issue Dec 13, 2015 · 6 comments
Closed

Consider removing mapboxgl-js as a depedency #31

vicapow opened this issue Dec 13, 2015 · 6 comments
Labels

Comments

@vicapow
Copy link
Contributor

vicapow commented Dec 13, 2015

Even though the original intent of this library was to create a React friendly wrapper around MapboxGL-js, it's more or less just the bits of mapboxGL-js that handle the UI and viewport changes but in a React friendly (stateless) way as well as a few built in overlays and examples. The overlays themselves don't have a direct dependence on MapboxGL-js. They only require the necessary viewport state information. There could eventually be an independent <MapboxGLOverlay> instead.

@cusspvz
Copy link

cusspvz commented Feb 17, 2016

+1
I'm currently trying to implement it on a project and I'm having issues with mapboxgl-js module on bundling.

@vicapow
Copy link
Contributor Author

vicapow commented Feb 17, 2016

Hey @cusspvz, the issue you're having might be unrelated. Could you provide more details?

@cusspvz
Copy link

cusspvz commented Feb 17, 2016

After commenting here, I've found that issue #21 was actually what I was looking for, but then I've read some of the codebase and decided to keep my comment here.

@cusspvz
Copy link

cusspvz commented Feb 17, 2016

In fact, I think this solution seems a little bit nasty but, what if you require your mapboxgl-js dependencies from the files directly instead?

Since they aren't ignoring js folder import LatLng from 'mapboxgl-js/js/geo/lng_lat.js should work.

Another solution would be import { LatLng } from 'mapboxgl-js', but AFAIK, only rollup is scrapping js tree-shaking for fetching what you really need from files ATM.

EDIT: The right term for the solution above seems to be tree-shaking.

refs:
https://github.com/uber/react-map-gl/blob/3ec7c00c094ddd4b8fa44cf5503fb50f88b057f5/src/map.react.js#L28-L30
https://github.com/uber/react-map-gl/blob/56fd78cf65e06debcd504d4580351bcb14f3e9ee/src/map-interactions.react.js#L23-L24

@alex3165
Copy link

What about defining mapbox-gl as a peerdependency ? so that we could use the fix for the mapbox-gl files mapbox-gl/js/render/shaders.js and webworkify/index.js right ? cf #21

Btw, does it make sense to avoid having direct dependencies by either compiling everything into one file or defining some as peer dependencies ?

@abmai
Copy link
Contributor

abmai commented Dec 6, 2016

Closing this since there are multiple issues presented here and some are already resolved. feel free to create follow-ups if that's not the case.

@abmai abmai closed this as completed Dec 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants