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

Bad practice in OverlayView #817

Closed
zolero opened this issue Apr 6, 2018 · 4 comments
Closed

Bad practice in OverlayView #817

zolero opened this issue Apr 6, 2018 · 4 comments

Comments

@zolero
Copy link

zolero commented Apr 6, 2018

Really bad practice in the OverlayView component for performance reasons. Why?

In the draw method every map movement it will re-append the this.containerElement and re-render the react component.

The appendChild() function should be in onAdd() and the unstable_renderSubtreeIntoContainer() function should be in render() method of the React Component.

This resulted in a huge performance different on our projects. Maybe someone could change this class so everyone can enjoy of the performance boost! I'm having issues to create a pull request because of styling problems.

For more information: https://developers.google.com/maps/documentation/javascript/customoverlays

draw() {
const { mapPaneName } = this.props
invariant(
!!mapPaneName,
`OverlayView requires either props.mapPaneName or props.defaultMapPaneName but got %s`,
mapPaneName
)
// https://developers.google.com/maps/documentation/javascript/3.exp/reference#MapPanes
const mapPanes = this.state[OVERLAY_VIEW].getPanes()
mapPanes[mapPaneName].appendChild(this.containerElement)
ReactDOM.unstable_renderSubtreeIntoContainer(
this,
React.Children.only(this.props.children),
this.containerElement,
this.onPositionElement
)
}

@nickpolet
Copy link

I've noticed the OverlayView having particularly bad performance as well. Works fine with the native google maps, but the same number of OverlayViews in this component results in drastically poor performance.

@molimauro
Copy link

@zolero Great hint thanks. The performance of my map drastically increased just by moving the appenChild in the onAdd function. However I'm facing some issues in moving the unstable_renderSubtreeIntoContainer in the render function. How did you manage to do this? Thanks a lot.

@JustFly1984
Copy link

@molimauro
Copy link

@JustFly1984 Yeah I'm aware of it (thanks for your work) and I'm planning the switch in the future, now unfortunately I can't.
Regarding this problem, I've noticed that also in your library you call 'appendChild' in the 'draw' function of the OverlayView, so please consider moving it in the 'add' function instead. It'll give you a boost in terms of performance, and me another reason to switch to your library :)
Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants