JustFly1984 / react-google-maps-api

React Google Maps API
MIT License
1.79k stars 438 forks source link

[Fix][Ezra] Directions Docs Callback Fix #3269

Closed ezerssss closed 1 year ago

ezerssss commented 1 year ago

Please explain PR reason

Fix the Directions Callback in the Docs

Related to: https://github.com/JustFly1984/react-google-maps-api/pull/3266, https://github.com/JustFly1984/react-google-maps-api/issues/3256, https://github.com/JustFly1984/react-google-maps-api/issues/3261

JustFly1984 commented 1 year ago

Well, this looks like google had changed an underlying API, which caused old code to fail with new API version. Since Docs build is broken for 2 years, I had no ability to even test it locally, not even talking about deploying updated docs version. I personally haven't used google maps api for any project for more than 3 years and maintaining it just for the sake of Open Source spirit.

ezerssss commented 1 year ago

hmmm found something weird, I basically copied the component in this pr: https://github.com/JustFly1984/react-google-maps-api/pull/3266

@JustFly1984 @IndianBoyBR

and it worked, no more rerenders and just called the callback function once

BUT

as soon as I added the onLoad or the onUnmount functions in the DirectionService component, the issue exists (infinite rerenders)

https://github.com/JustFly1984/react-google-maps-api/assets/72904036/3455efda-774a-4bbc-b038-39abe282310c

Check the stackblitz code here:

Note Add your API Key in the index.tsx file

JustFly1984 commented 1 year ago

@ezerssss , You are creating new anonymous arrow function, and causing re-renders. This is ABC of react performance. This library is all about abiding props referential transparency at every step. you can't just willy-nilly pass new object to props. Use eslint with eslint-plugin-react-perf to track those cases. you should cache any object/array/function before passing it into the prop always without exceptions across your whole application, and wrap functional components exports with memo() to make them pure, and update only if prop reference changed.

ezerssss commented 1 year ago

@JustFly1984 Could that be one of the reasons why it was causing re-renders in the original code?

Code in the docs: image

<DirectionsService
                  // required
                  options={{ // eslint-disable-line react-perf/jsx-no-new-object-as-prop
                    destination: this.state.destination,
                    origin: this.state.origin,
                    travelMode: this.state.travelMode
                  }}
                  // required
                  callback={this.directionsCallback}
                  // optional
                  onLoad={directionsService => {
                    console.log('DirectionsService onLoad directionsService: ', directionsService)
                  }}
                  // optional
                  onUnmount={directionsService => {
                    console.log('DirectionsService onUnmount directionsService: ', directionsService)
                  }}
                />
JustFly1984 commented 1 year ago

yes, it is just docs with simplified examples. Everybody is expected to kind of understand referential transparency and not just blindly copy the code from docs. // eslint-disable-line react-perf/jsx-no-new-object-as-prop kind of explains that something is odd here)

JustFly1984 commented 1 year ago

btw you can see all examples everywhere in most of the docs around react passing objects willy-nilly. This is for obvious reason of simplicity and reducing cognitive load. If you got bitten with referential transparency thing in javascript once, you understand the concept once and for all, and never make same mistake again. There is no tab vs spaces argument. It is just a feature of javascript, employed by react library to cause re-renders in pure (memo) functional components. Unpure functional components re-renders all the time without exception anyway, even without referential transparency. So we have to have all of them pure, and use referential transparency without exceptions.

ezerssss commented 1 year ago

btw you can see all examples everywhere in most of the docs around react passing objects willy-nilly. This is for obvious reason of simplicity and reducing cognitive load. If you got bitten with referential transparency thing in javascript once, you understand the concept once and for all, and never make same mistake again. There is no tab vs spaces argument. It is just a feature of javascript, employed by react library to cause re-renders in pure (memo) functional components. Unpure functional components re-renders all the time without exception anyway, even without referential transparency. So we have to have all of them pure, and use referential transparency without exceptions.

got it! i guess this is the time I kinda understand that concept. thank u so much for being patient!

ezerssss commented 1 year ago

@JustFly1984 i just removed the remaining optional parameters