search

JustFly1984 / react-google-maps-api

React Google Maps API
MIT License
1.82k stars 443 forks source link

Contributors are welcome! #18

Open uriklar opened 5 years ago

uriklar commented 5 years ago

Hi everyone, We'd love to get your code contributions! We have a slack channel that is currently on an invite basis. Leave a note here if you want to join.

There are some components that need to be revised from their old (react-google-maps) implementation, to our new implementation. For an examples of such a conversion, see this commit

Here are the components that still need to be implemented

Other open tasks:

shafqatalix commented 5 years ago

Hi @uriklar , I am working on another fork of react-google-maps, but since you guys already started this one, i would like to contribute instead of perusing my own fork. I am working on project which uses react-google-maps, and i faced some issues for which i had to override some behaviors in some of the components.

Please let me know If that is something you would like to incorporate into this fork? Then i can start working on preparing PR. Plus one more thing this lib doesn't build on windows machine, and i have to add few things to package.json make is work.

uriklar commented 5 years ago

Hi @xalisys, we'd be happy to add you code. I saw your fork, but didn't see the commits you mentioned. Can you send me a link?

And a PR to fix the windows build would be greatly appreciated!

shafqatalix commented 5 years ago

@uriklar thanks for quick response, the code i mentioned is in my project where i override the react-google-maps, components. I'll create separate PRs for things aforementioned.

thanks.

gregoryfm commented 5 years ago

Hi @uriklar I'm willing to help. Can you add me in the slack group to let me know where to start?

uriklar commented 5 years ago

Hi @gregoryfm! Send me the email of your slack account and i'll send you an invite. (You can also mail me at uriklar@gmail.com if you prefer)

gregoryfm commented 5 years ago

I sent you an email

danielkcz commented 5 years ago

As I already mentioned somewhere else ... I would like to contribute, however seeing this project is almost dead already, I don't feel like it's worth the time. Did everyone get so busy suddenly to let this die?

uriklar commented 5 years ago

Hi @FredyC, Why do you say it's dead? I think the lib has reached an MVP state and from now on it will grow to wherever people will have the need and the time to make it grow.

As Alexey mentioned in several places, we use this lib for our personal and company's projects, so our work on it is focused on those needs. But we will gladly help anyone who wants to contribute and will surely put in some more work in when we have the time for it.

danielkcz commented 5 years ago

I totally understand it's not like the main project you want to invest too much time into. In that case, it might been better to just keep it private instead of saying everywhere that this is a replacement lib. How it can be when it's lacking features that react-google-maps have?

Do you even adhere to semver? With your "hobby" approach I feel like the stability could be rather volatile. There are no tests which I would say is kinda a must for a library. Otherwise, it's just a code that was mangled together to make some basic scenarios working (with fingers crossed). Documentation is rather sparse and it's hard to tell what's actually working and what isn't.

I do appreciate your valiant effort for sure, but at this point, I feel like I would just fork react-google-maps, cherry-pick commits from existing PRs and it would be much safer and easier for existing production apps.

uriklar commented 5 years ago

First of all, for the best of my knowledge react-google-maps doesn't have any tests either. This is a project at the beginning of it's path. We made it public so we can start gathering react-google-map users and show them there's a home for their map needs if they wish to come and help build it.

I know that some components are missing, but, did you look at the code? It's so easy to just pick a component up and implement it! For documentation we use docz which makes adding docs even easier. I agree with you this project deserves more work then it gets..

If react-google-maps works for you, I don't see any reason to migrate. For me, the main use case for migrating from would have to be Async React readiness (which @react-google-maps/api is), also reduced bundle size is a nice touch.

danielkcz commented 5 years ago

First of all, for the best of my knowledge react-google-maps doesn't have any tests either.

Fair enough, there are a few basic tests, but probably not that useful.

I know that some components are missing, but, did you look at the code?

I did not. Honestly, seeing JS code base just gives me shivers as it means that touching anything can break stuff fairly easily (especially with no tests). Unless the decision for TypeScript happens, I don't think I will be able to contribute.

If react-google-maps works for you, I don't see any reason to migrate. For me, the main use case for migrating from would have to be Async React readiness (which @react-google-maps/api is), also reduced bundle size is a nice touch.

It works, but there are some quicks for sure that could be solved here better. For starters we have fully embraced React Hooks, so seeing class-based context is just bad :)

uriklar commented 5 years ago

We haven't used hooks in this project as they haven't been released yet. Once they're released this whole lib might as well be shrunk into a single hook :-) You can see a (non-working) POC here: https://github.com/JustFly1984/react-google-maps-api/blob/master/packages/react-google-maps-api/src/utils/use-map-component.js

If you feel like making a PR to kick off the Typescript migration effort I will gladly join in on that branch!

JustFly1984 commented 5 years ago

@FredyC We have done refactor to typescript, if you have time can you please review?

danielkcz commented 5 years ago

@JustFly1984 It's definitely on my todo list. Lately, more and more issues are coming from the abandoned maps project, so there is a hoping it will improve with this one.

JustFly1984 commented 5 years ago

@FredyC We have published @1.2.0 with full typescript support.

danielkcz commented 5 years ago

Thank you for all the hard work. Tried to integrate into a project today and it was surprisingly easy to migrate. Not that it's some heavy map usage app, mostly just a couple of OverlayView, but 😍

I got a few pointers.

I might work on PRs tomorrow if desired.

uriklar commented 5 years ago

Hi @FredyC, Thanks so much for the feedback! Sounds like pretty straight forward stuff. We should remove any non mandatory LoadScript props as mandatory. I'm not sure I understand the use case for useGoogleMap though. The onLoad lets you access (and cache if needed) an instance of that specific component. useGoogleMap would only give you access to the map instance.

While you're here :-) I actually have a Typescript question that maybe you'll know the answer to: I integrated this lib in my company's project that uses typescript (but also js) and was getting a module not found ts error that resolved only by adding moduleResolution: "node" to my tsconfig.json.

I'm using other libraries with direct (that is, not via DefinitelyTyped) TS support (for example styled-component) and they didn't require me to change the module resolution. Changing it makes my initial build suuuuper slower.

Got any ideas on that? I'm wondering if our tsconfig setup is correct... Should we be using rollup? I see you're using it on mobx-react-lite.

Thanks again!

danielkcz commented 5 years ago

I'm not sure I understand the use case for useGoogleMap though. The onLoad lets you access (and cache if needed) an instance of that specific component. useGoogleMap would only give you access to the map instance.

I need access to the map instance for panning purposes and often do that within components which are deeper in the tree. So the hook comes in really handy for that.

Regarding your typescript problem I am not sure really, but I am using moduleResolution: "node" pretty much everywhere. The other option is classic which is legacy anyway. Sounds strange it would slow down build so much, but never tried really.

I'm wondering if our tsconfig setup is correct

Well, you definitely have a fairly robust config, I usually get away with defaults and tweak some basics. I don't know half of that stuff what is it doing actually :) Might be worth investigating if you need all that.

Should we be using rollup?

I used it because I know it, but I heard lately good opinions about microbundle, might be worth the shot.

JustFly1984 commented 5 years ago

@FredyC Thank you for improvements, I will publish new version tomorrow

JustFly1984 commented 5 years ago

@FredyC Please add yourself to contributors list in /package/react-google-maps-api/package.json

JustFly1984 commented 5 years ago

@FredyC can you please provide an example of useGoogleMap hook for docs and for gatsby-example?

danielkcz commented 5 years ago

Please add yourself to contributors list in /package/react-google-maps-api/package.json

There is no list in that file :) Doesn't matter, it would be too hidden anyway, so I don't care.

can you please provide an example of useGoogleMap hook for docs and for gatsby-example?

Ok, hopefully by tomorrow I will able to stitch something together.

mathieu-anderson commented 5 years ago

Hello! We have started using this library for the implementation at my workplace (well, mostly me). As I understand it from perusing the issues, it looks like it's the case for most contributors.

I don't know yet if I will have the ability to contribute anything meaningful (still pretty junior), but I certainly would like to try.

And I am wondering if it would be desirable, on top of the private Slack channel, to maintain a Slack / Spectrum / Gitter channel for questions regarding specific use cases. I personally have a few, but as their place is probably not in the Issues, and the documentation / examples are very basic, and the library is pretty young, it's hard to find resources.

danielkcz commented 5 years ago

I would kinda vote for Spectrum as well. Personally, I already have too many Slacks and want to limit that. Spectrum at least does not force to you have a separate set of credentials for each community. Although I don't exactly adore that platform just yet, it's still under heavy development and sometimes wonky. But it's a good alternative. The Gitter is useful for quick chatting too, but no separation per topic makes it useless for QA.

uriklar commented 5 years ago

Hi @mathieu-anderson. Our Issues are indeed getting filled with usage questions. Spectrum sounds like a great idea! It has the visibility that slack lacks and it is more specific the regular stackoverflow.

Here's a link to our newly created community: https://spectrum.chat/react-google-maps

Maybe your first PR could be adding it to the docs? :-)

mathieu-anderson commented 5 years ago

Lightning fast reaction time @uriklar ! I posted the first question on Spectrum!

And yes, I might look at adding to the docs! What's the process to make PRs to the project?

shobishani commented 5 years ago

HI @uriklar I'm not sure if you should have asked this question here are hooks implemented in this library I need to access map Instance for some reasons in deep down tree.

uriklar commented 5 years ago

Hi @shobishani, It's better you ask usage questions in our spectrum channel We recently added the useGoogleMap hook that will return the map instasnce from anywhere in the tree. After successfully using it you can create a PR to add it to the documentation :-)

shobishani commented 5 years ago

@uriklar Thanks.. sure will submit PR

janklimo commented 5 years ago

Is anybody working on the implementation of InfoBox? We're working on migrating from react-google-maps so we could contribute to that guide once ready.

JustFly1984 commented 5 years ago

@janklimo we are not working on InfoBox currently, if you want to make a PR, it is always welcome. @uriklar do you have something to add?

danielkcz commented 5 years ago

@JustFly1984 @uriklar Hey, do you know that latest 1.2.3 build is actually broken? In package.json you are referring to UMD folder, but it's not included in the build. Perhaps you haven't intended to release it yet and it should have been another alpha?

https://unpkg.com/@react-google-maps/api@1.2.3/

I would highly recommend not to use .npmignore, it's hard to maintain that in the long run. Instead, the files field in package.json is much easier (be it a whitelist). I will do PR to fix that.

JustFly1984 commented 5 years ago

@FredyC Thank you!

joaoreynolds commented 5 years ago

@JustFly1984 @uriklar I'd like to take a crack at the OverlayView performance issues. But I don't have experience with lerna or storybook. I need some pointers on how to get started. I'm able to edit the storybook project and get it up and running, but what about making changes to /react-google-maps-api/src/components/dom/OverlayView.tsx - I'm not sure what to spin up so I can preview changes there as I develop.

Maybe add me to slack so I can talk with you there? I'll send an email to @uriklar

uriklar commented 5 years ago

Hi @joaoreynolds. Right now, the top issue preventing people from contributing is the lack of a good workflow for working on the library. I can give you my workflow, but it's really terrible :-) I think the solution needs to be setting up another package in the repo which will be a CRA app with typescript support. Then, this package could import directly from the react-google-maps-api folder instead of from the node_module. Or maybe using npm link or something like that.

Let me know if maybe you want to start from that. I'd be happy to assist on slack

joaoreynolds commented 5 years ago

I tried using npm link between storybook and react-google-maps-api, but that didn't seem to work. I'm comfortable setting up a CRA app. But what if we just changed the storybook project to import directly from the react-google-maps-api folder. It seems like a good place to do the development work. I suggest this because the storybook seems pretty empty right now, so I wonder what it's used for if not this...

Thoughts?

JustFly1984 commented 5 years ago

The problem is that storybook does not support typescript. You can’t import TD files from storybook.

The solution will be to create separate package react-google-maps-api-cra-example with typescript enabled. This way we could import TD files

Another way is to add typescript to react-google-maps-api-gatsby-example

joaoreynolds commented 5 years ago

Ahh gotcha - okay I can create the cra example. I'll keep you updated if I have more questions.

cuongdevjs commented 4 years ago

Does ImageMapType supported?

rbonigala commented 4 years ago

Hi, I am trying to develop a sample using the Next.Js, but the map is not getting rendered. Is there an example that I can refer to that has been implemented using NextJS framework.

JustFly1984 commented 4 years ago

@rbonigala we have only gatsby-example in the repo, but in general SSR with next js should not be different than static rendering with Gatsby. Inshort - you can't render it in SSR, cos it requires DOM and injecting script tag in runtime. for SSR you can render fallback instead.

mradenovic commented 3 years ago

Hi @joaoreynolds. Right now, the top issue preventing people from contributing is the lack of a good workflow for working on the library. I can give you my workflow, but it's really terrible :-) I think the solution needs to be setting up another package in the repo which will be a CRA app with typescript support. Then, this package could import directly from the react-google-maps-api folder instead of from the node_module. Or maybe using npm link or something like that.

Let me know if maybe you want to start from that. I'd be happy to assist on slack

I find this very thru! I would like to give it a try. If someone can help with any working workflow, no matter how terrible it is, it would be great. I feel like How to test changes locally is outdated).

So far I have noticed the following issues, that might need new issues created:

iuliiaostapenko commented 1 year ago

Hello guys. Maybe you know why documentation is down? https://react-google-maps-api-docs.netlify.app/ Maybe you could fix it, please :)

vijay-t13 commented 1 year ago

Is anyone making documentation site up ?

JustFly1984 commented 1 year ago

@vijay-t13 it is open source. If you want you can fix it. I will merge it and deploy. By now the config for docs is not updated to support latest current version, so docs build is broke. Somebody did a PR with downgrading docs to old version, and it is possible to run, but downgrading is not a good option. docs should be configured according new major version.

iuliiaostapenko commented 1 year ago

Heads up, guys

When i run npm run build on the server I got error:

[commonjs--resolver] Failed to resolve entry for package "/var/www/Project/node_modules/@react-google-maps/api". The package may have incorrect main/module/exports specified in its package.json. error during build: Error: Failed to resolve entry for package "/var/www/Project/node_modules/@react-google-maps/api". The package may have incorrect main/module/exports specified in its package.json. at packageEntryFailure (file:///var/www/Project/node_modules/vite/dist/node/chunks/dep-3bba9c7e.js:28730:11) at resolvePackageEntry (file:///var/www/Project/node_modules/vite/dist/node/chunks/dep-3bba9c7e.js:28727:5) at tryCleanFsResolve (file:///var/www/Project/node_modules/vite/dist/node/chunks/dep-3bba9c7e.js:28386:28) at tryFsResolve (file:///var/www/Project/node_modules/vite/dist/node/chunks/dep-3bba9c7e.js:28333:17) at Object.resolveId (file:///var/www/Project/node_modules/vite/dist/node/chunks/dep-3bba9c7e.js:28189:24) at file:///var/www/Project/node_modules/rollup/dist/es/shared/node-entry.js:25544:40 at processTicksAndRejections (node:internal/process/task_queues:96:5) at async PluginDriver.hookFirstAndGetPlugin (file:///var/www/Project/node_modules/rollup/dist/es/shared/node-entry.js:25444:28) at async resolveId (file:///var/www/Project/node_modules/rollup/dist/es/shared/node-entry.js:24117:26) at async ModuleLoader.resolveId (file:///var/www/Project/node_modules/rollup/dist/es/shared/node-entry.js:24531:15)

After I revert my version to previous in package.json all works just fine: "@react-google-maps/api": "2.18.1" My env-> react, ts, vite, inertia, laravel

JustFly1984 commented 1 year ago

@iuliiaostapenko Do you want to became a contributor?

JustFly1984 commented 1 year ago

@iuliiaostapenko since now, accepting issues only with PR to fix it. There is no guys, I'm here alone maintaining this project.