Popup interaction re-renders web RootView and all children

[ngburke]

When interacting with a popup (e.g. mouse in/out, re-rendering on modification), the entire application web root view and all children are re-rendered. This is due to state variables such as isMouseInPopup in RootViewState in web/RootView.tsx being changed during popup to interaction which triggers a whole-app render call. For apps with a large number of components this is not desirable.

[erictraut]

The state change will cause the RootView to re-render, but it shouldn't cause the entire app to be re-rendered. React is smart about not calling render on lower levels of the view hierarchy if there were no changes further up the hierarchy.

For example, let's say the view hierarchy consists of RootView => ViewA => ViewB => ViewC. (I'm using "=>" to mean "contains".) If RootView render method is called and the props and context it uses to render ViewA did not change since the last time it was rendered, React will not call the render method of ViewA. Neither will it call the render method for ViewB or ViewC.

What behavior are you seeing that concerns you? If you're just noting that the RootView render method is being called, that's to be expected, and it's not a problem. This call is very lightweight and fast. If you're seeing evidence that render is being called for all mounted components in the app or that the DOM is changing, then we should investigate further because those shouldn't be happening.

[ngburke]

I wasn't clear - yes I'm seeing mouseover (for example) trigger a re-render of the RootView and all children. Below is a small app to demonstrate the issue. After clicking the button, when mousing in and out of the popup, I see the following console logs:

Render View A
Render View B
Render Button

class Button extends RX.Component {
  private _buttonAnchor: RX.Button
  private readonly _selectBoxId = 'box'

  private _onPress() {
    RX.Popup.show({
      positionPriorities: ['bottom', 'right'],
      getAnchor: () => { return this._buttonAnchor },
      renderPopup: () => <RX.View>Hello!</RX.View>,
      cacheable: true,
    }, this._selectBoxId)
  }

  render() {
    console.log('Render Button')
    return (
      <RX.Button
        onPress={ () => this._onPress() }
        ref={ (comp: RX.Button) => this._buttonAnchor = comp }
      >
        Press Me
      </RX.Button>
    )
  }
}

function ViewB() {
  console.log('Render ViewB')
  return (
    <RX.View>
      <Button/>
    </RX.View>
  )
}

function ViewA() {
  console.log('Render ViewA')
  return (
    <RX.View>
      <ViewB/>
    </RX.View>
  )
}

export class App extends RX.Component<any, any> {
  render() {
    return <ViewA/>
  }
}

[erictraut]

Interesting. It looks like recent versions of ReactJS changed its behavior with respect to components that use the legacy context mechanism (getChildContext). Earlier versions of ReactJS did not force a complete re-render of the view hierarchy unless the context changed. Now it appears to do so every time that a context-owning component renders. I suspect that switching to the new context APIs would solve the problem, but that would create compatibility issues with older versions of React. We will eventually need to adopt the new APIs, but I don't want to do it until they are fully removed (which will be in React 17.x).

As a workaround, I've removed the use of setState within RootView to track mouse hovers. It wasn't needed anyway.

I just published ReactXP 1.4.0, so this fix won't appear until we accumulate a few more bug fixes and 1.4.1 is published.

Thanks for reporting the problem and providing the above code.

[ngburke]

No problem - thanks for the in-depth explanation of the issue and the workaround. I also noticed that in addition to mouseovers, calls to RX.Popup.show an already shown popup (for example to re-render items in the combo box) touch the RootView state tracking the popup absolute location and dimensions. This also causes a full RootView and children re-render per your explanation above. I didn't dive in enough to understand if a workaround is possible.

[mshoho]

@erictraut I have checked it, and the initial assumption about the context API changes doesn't seem to be correct. Older versions behave the same. React always rerenders the whole subtree (even when the props didn't change), unless shouldComponentUpdate() presents and returns false.

[ngburke]

@mshoho - Ok, if that is the case, is it possible to encapsulate the popup details elsewhere so say, re-rendering the popup with different text, would not trigger a whole app update by default? Or do you recommend that I make some custom logic in my app's shouldComponentUpdate()?

[erictraut]

@ngburke, does it really matter if the app is re-rendered when the popup is initially displayed? I can see why you'd be concerned about re-renders on every mouse over, but a popup is shown only once, so the perf impact should not be immaterial.

The correct workaround is to adopt the new context mechanism, which we will eventually do in ReactXP. I'd prefer not to invest in other short-term workarounds unless they're truly important.

[ngburke]

Per my current implementation of the combo box, each time an arrow is used to traverse the list, RX.Popup.show will be invoked again to update the list (to highlight the selected item). Similarly when typing in the combo box, the list is modified based on what is typed, thus invoking RX.Popup.show again. These cause re-renders of everything. Maybe there is another way to approach it?

In any case, I'm perfectly fine waiting for the correct fix you mention and can live with this right now, so don't sweat another workaround!

[mshoho]

@ngburke you can definitely use shouldComponentUpdate(). Skype uses https://github.com/Microsoft/ReSub for the internal components, which has a default implementation for shouldComponentUpdate(). That is likely why we don't suffer from that rerendering problem.

[ngburke]

@mshoho Thanks for the pointer - I'll dive into that this weekend.