OverlayView re-rendered on map drag

[ninosaurus]

Hi, I refactored my app to use this library instead of react-google-maps, and I've found some problems with unnecessary rendering of OverlayView component.
I have events such as onMouseOver, onFocus, onMouseOut, onBlur, onClick on OverlayView children.

Environment

node --version: v10.15.1

Here is my package.json dependecies:

{
  "devDependencies": {
    "@babel/core": "^7.5.5",
    "@babel/plugin-proposal-class-properties": "^7.5.5",
    "@babel/preset-env": "^7.5.5",
    "@babel/preset-react": "^7.0.0",
    "babel-eslint": "^10.0.2",
    "babel-loader": "^8.0.6",
    "babel-plugin-lodash": "^3.3.4",
    "css-loader": "^3.1.0",
    "eslint-loader": "^2.2.1",
    "html-loader": "^0.5.5",
    "html-webpack-plugin": "^3.2.0",
    "mini-css-extract-plugin": "^0.8.0",
    "optimize-css-assets-webpack-plugin": "^5.0.3",
    "size-limit": "^1.3.8",
    "source-map-explorer": "^2.0.1",
    "terser-webpack-plugin": "^1.3.0",
    "webpack": "^4.38.0",
    "webpack-bundle-analyzer": "^3.4.1",
    "webpack-cli": "^3.3.6",
    "webpack-dashboard": "^3.0.7",
    "webpack-dev-server": "^3.7.2",
    "why-did-you-update": "^1.0.6"
  },
  "dependencies": {
    "@material-ui/core": "^3.9.3",
    "@react-google-maps/api": "^1.5.3",
    "axios": "^0.19.0",
    "babel-polyfill": "^6.26.0",
    "camelize": "^1.0.0",
    "classnames": "^2.2.6",
    "core-js": "^2.6.9",
    "immutability-helper": "^3.0.1",
    "js-cookie": "^2.2.0",
    "lodash": "^4.17.15",
    "prop-types": "^15.7.2",
    "react": "^16.8.6",
    "react-dom": "^16.8.6",
    "react-redux": "^7.1.0",
    "redux": "^4.0.4",
    "redux-devtools-extension": "^2.13.8",
    "redux-logger": "^3.0.6",
    "redux-thunk": "^2.3.0",
    "style-loader": "^0.23.1"
  }
}

I'm using OverlayView for custom html styled markers as shown below:

      <OverlayView
        mapPaneName={OverlayView.OVERLAY_MOUSE_TARGET}
        position={position}
      >
        <div
          className="map-price-container fade-scale-in"
          ref={ref => (
            ref
            // eslint-disable-next-line no-undef
            && google && google.maps && google.maps.OverlayView
            // eslint-disable-next-line no-undef
            && google.maps.OverlayView.preventMapHitsFrom(ref))
          }
        >
          {this.renderBody()}
        </div>
      </OverlayView>

How is it behave?

css: fade-scale-in is used for animating when element shows first time, by with every move/drag of map, I get rerendered that element and it trigger animation.
I turned on "why-did-you-update" and see that OverlayView portal is updated every time.

Is there any workaround or optimization needed in order to achieve this?

I tried with Marker component but I'm not able to do custom html inside it.

Thanks in advance.

[JustFly1984]

@ninosaurus can you please create minimal reproduction at codesandbox?

[JustFly1984]

@ninosaurus By now I think your issue is related to this.renderBody() call.
Please extract it to separate function component, and wrap into React.memo.

[JustFly1984]

Can you please also add eslint-plugin-react-perf for any case?

[ninosaurus]

@JustFly1984 I tried with memoizing renderBody but it's not fixed.
Here I created reproduction at the CodeSandbox, you may need to set your own API_KEY.

[JustFly1984]

@ninosaurus I have made some changes, please take a look
https://codesandbox.io/s/google-maps-issue-31ijp

[ninosaurus]

Thanks @JustFly1984 but I still have same issue, here is an illustration:

[JustFly1984]

I have changed it a bit more and found your issue - you have a className "fade-scale-in" on div
I have commented it out, and all works great
https://codesandbox.io/s/google-maps-issue-31ijp

[ninosaurus]

I know that, but that's the animation that we wanted, is there other way to do animation?

[JustFly1984]

I don't really know what is going on here
There is nothing re-rendered in React. @FredyC @uriklar can you look closer on this issue?

[ninosaurus]

I guess that's because map rerender the html element every time when map drags which then trigger css animation. I see that they provide API for animating Markers, but for OverlayView I had to do it with CSS. @JustFly1984

[JustFly1984]

@ninosaurus sorry, can't help you with issue, lets wait for other maintainers, maybe they have any ideas.

[danielkcz]

Well, it's really weird. Apparently componentDidUpdate of OverlayView is triggered on the move of map. Even though there is a PureComponent. React Devtools Profiles is also convinced that re-render is originating at OverlayView and not above it. I even tried to debug it and compared each prop and they do equal too. This should be fairly straightforward stuff and yet it baffles me as well. Sorry, I have no idea either what might be causing that.

Edit:

Oh, wait ... why is this in here? That means it will re-render on each draw :)

react-google-maps-api/packages/react-google-maps-api/src/components/dom/OverlayView.tsx

Line 128 in 4f47f5f

this.forceUpdate()

[JustFly1984]

As I see it was added by @uriklar
Can you please clear an intention?

[JustFly1984]

@ninosaurus it is clearly a bug on our side, please wait till we fix it and publish new version

[uriklar]

The force update is there because the container element is repositioned on every map moved. I guess we could update it's style via ref instead of rerendering the component. I'll try to get to it later today

[uriklar]

@ninosaurus I believe the https://github.com/JustFly1984/react-google-maps-api/tree/issue-198-overlayview-force-update fixes the issue but I don't have time to test it right now.
@JustFly1984 do we have a good way to let people test branches? Because of lerna it is not possible to install the package from a git branch (@FredyC maybe now with yarn workspaces this has changed?)

@ninosaurus you can clone the branch, run yarn build and copy the dist folder into your node_modules directory

[danielkcz]

Well, Yarn workspaces have nothing with that really. And even if there wouldn't be monorepo, without actually commiting dist it's not possible to just install it directly from GH.

I do like using https://github.com/whitecolor/yalc when I need to use a fork/branch of some project without polluting NPM with it, but it still needs to clone the project and build it. I am not aware of any other prettier way.

[JustFly1984]

I think the fastest way to test, is to add the code from codesandbox to gatsby-example in the same branch, and test.

PS: Honestly, It feels like lerna and yarn workspaces is making more trouble than solving issues. same is concerning tsdx.

I do not have time today, but if there will be PR, I'll find time to merge and publish.

[danielkcz]

Well, that's just a pitfall of modern tooling which ultimately makes sense. Without it you would need to write code in ES5, no ESM, no TypeScript. Also, there would be no UMD bundle and people would have run minification on their own.

I would say it's not worth it just for a few cases when you need to test things out. If it's really something worrisome for you, then set up git hook (with Husky) to run build on every commit and add dist folders to Git. Then it's possible to just install any commit directly from GH without additional steps.

[ninosaurus]

@uriklar I've created you example on codesandbox, it looks like it not working, can you check it, I pulled out reactgooglemapsapi.esm.js and loaded components from there
SandBox

[danielkcz]

@ninosaurus Well, it looks like that culprit is somewhere else after all. The componentDidUpdate of OverlayView is not firing anymore and the overlay content itself is not re-rendered either. So it seems that draw called by google maps is the source of the problem somehow.

Considering it was working for you with tomchentw version, it surely must be something on our side, but I just tried to compare the code and besides using React.portal there is no other significant difference.

[ninosaurus]

I guess google maps overlay animation is not supposed to be done with css. @FredyC

[JustFly1984]

@ninosaurus @FredyC @uriklar at least we have found a performance bug and eliminated it. @FredyC can you please look at the call stack of draw function?

[danielkcz]

@JustFly1984 It goes directly to minimized google maps, so that's pretty much impossible to get something useful from that, but it's clear it runs based on requestAnimationFrame so in general it should be suitable for animating stuff.

@ninosaurus I don't know enough about CSS animations, but are you actually sure that it's enough like that? I mean it feels like such animation is about to be executed on every DOM redraw, there is no indication it should happen on "div appearence" only. I have a vague awareness of https://github.com/reactjs/react-transition-group which should be used for animating on mount/unmount. Isn't it actually something you need?

[ninosaurus]

@JustFly1984 It goes directly to minimized google maps, so that's pretty much impossible to get something useful from that, but it's clear it runs based on requestAnimationFrame so in general it should be suitable for animating stuff.

@ninosaurus I don't know enough about CSS animations, but are you actually sure that it's enough like that? I mean it feels like such animation is about to be executed on every DOM redraw, there is no indication it should happen on "div appearence" only. I have a vague awareness of https://github.com/reactjs/react-transition-group which should be used for animating on mount/unmount. Isn't it actually something you need?

I'll try that later today, it make sense to be working, thanks a lot guys.

[JustFly1984]

@ninosaurus can we close an issue?

[JustFly1984]

I consider this issue cosed, please request reopen if you have more questions

[JustFly1984]

@uriklar

tomchentw/react-google-maps#817 (comment)

[danielkcz]

I don't think that calling appendChild in draw would have anything to do with this issue. It's performance boost, that's probably true, but this issue was about wrong way of doing animation. The OverlayView always have to re-render on map drag because it changes its position on the screen.

[JustFly1984]

@FredyC thank you for clarification. I just wanted to save this url as reference to OverlayView, so we can fix performance issue at some point.

[JustFly1984]

will leave this issue open, until we apply the fix.

[asdutoit]

Hi Guys, I have the same issue, but mine seems to be related to React 18 (and I assume it's due to the automatic batching).
When I downgrade to React 17, it works perfectly.

[AbdelrahmanTorki]

This issue was solved for me by using OverlayViewF instead of OverlayView