search

mariusandra / pigeon-maps

ReactJS Maps without external dependencies
https://pigeon-maps.js.org/
MIT License
3.44k stars 142 forks source link

Could you please upgrade to React 16.8+ add TS typings if possible? #68

Closed mikhailbartashevich closed 4 years ago

mikhailbartashevich commented 5 years ago

Could you please upgrade to React 16.8+ add TS typings if possible?

mariusandra commented 5 years ago

Hi, the upgrade is done with PR #70 and out with 0.14.0

Regarding TS, I don't use it myself and won't have time to build this. However I'll gladly accept a PR that adds this.

zdila commented 4 years ago

I have rewritten pigeon-maps to TypeScript to replace Leaflet and react-leaflet in our project with some more "native" solution. I included the source code directly to our repo to easily do further modifications and fixes (with plan to contribute it back). Code was also re-formatted by prettier. I have also fixed #71 there.

https://github.com/FreemapSlovakia/freemap-v3-react/tree/pigeon-maps/src/pigeon-maps

In any case I have decided to stay with Leaflet at least for the near future as pigeon-maps animation is choppy for big (full-window) map where Leaflet is still smooth.

mariusandra commented 4 years ago

@zdila wow, amazing to see the conversion! I actually started on something similar myself in the typescript branch, but never had the time to finish it. Any contribution would be appreciated! :)

Sad to hear about the full screen map issue. As you have gone through all the effort of converting it to TS, you probably thought a bit about it. What did you conclude was causing the slowdown and do you think there's anything that can be done with it?

And perhaps could React's upcoming concurrent mode could help with this? This demo looks pretty amazing: https://twitter.com/0xca0a/status/1199997552466288641

zdila commented 4 years ago

Any contribution would be appreciated!

Feel free to grab the code back :-). It will also require to adjust the tooling (webpack / rollup) but shouldn't be difficult. If you wish, I can try to do that but can't promise you when.

As you have gone through all the effort of converting it to TS, you probably thought a bit about it.

Actually not very much :-). I tried to run React profiler but found no obvious problem. If you are interested I can take a deeper look on it.

Rewriting to TS wasn't any big deal. I even didn't need to understand the code logic. I was just adding types and fixing errors reported by TS :-)

Regarding concurrent mode I am not sure. I haven't observed any CPU load peaks during animation.

mariusandra commented 4 years ago

TS is now in the master branch