iTwin / viewer

Monorepo that contains the iTwin Viewer npm packages and their related packages
MIT License
24 stars 15 forks source link

Regression in how the React Viewer determines that it needs to re-render between versions 3.0.0 and 3.1.0. #241

Closed Alfonso-Martello closed 1 year ago

Alfonso-Martello commented 1 year ago

The Issue If the React Viewer component has a sibling component that causes the parent component to re-render, then the Viewer also re-renders even though no new props were provided to the Viewer.

My guess is that on each re-render, the Viewer thinks its props are being recalculated if those variables/functions are not memoized or constant.

The Version & Package This issue was observed with the package @itwin/web-viewer-react on version 3.1.2. I was able to reproduce the behavior in version 3.1.0 as well. However, the behavior is not present in versions 2.x nor version 3.0.0.

How to Reproduce Create a new iTwin Viewer application on version 3.1.x. In the top-level App component that renders the Viewer, add a sibling to the Viewer that causes some state change so that the parent re-renders (the state change should not change any state that the viewer consumes).

If you change the @itwin/web-viewer-react version to 3.0.0, you will notice that the Viewer application will no longer re-render when its sibling changes state.

What was Expected It is expected that the Viewer would behave the same as in previous versions (3.0.0 and 2.x) when determining if it should render. In version 3.1.x, it seems that all of the props to the Viewer need to be memoized. This was not the case in previous versions.

Examples I have put together 3 quick sandboxes to highlight the issue:

  1. A sandbox showing the unusual behavior in version 3.1.2: https://www.itwinjs.org/sandbox/AlfonsoMartello/3.x-Viewer-Rerender-Regression
  2. A sandbox with identical code showing different behavior in version 2.x (the expected behavior): https://www.itwinjs.org/sandbox/AlfonsoMartello/2.x-Viewer-Rerender-Regression
  3. A sandbox in version 3.1.2 showing a change that makes the behavior the same as in version 2.x: https://www.itwinjs.org/sandbox/AlfonsoMartello/3.x-Viewer-Rerender-Constant

Below is a gif of the expected behavior (this is from an app using version 3.0.0): 2e9be4e1-6b14-435b-9ac8-bb2561c47514

Below is a gif of the unexpected behavior (this is from an app using version 3.1.0): ac768f7a-8130-44ed-8508-0f7b3552589b