Improvements to AutoSizer:

* No longer re-renders nor calls onResize callback unless width and/or height have changed (depending on which properties are being watched). * No longer renders content before collecting initial measurements. This prevents Grid (and others) from being rendered initially with a width or height of 0.
bvaughn · Mar 26, 2017 · 2c48093 · 2c48093
1 parent 978ec14
commit 2c48093
Show file tree

Hide file tree

Showing 3 changed files with 104 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,8 @@ Changelog
 * ✨ Replaced inline `require` statement with header `import` in `Grid` for better integration with the Rollup module bundler. ([@odogono](https://github.com/odogono) - [#617](https://github.com/bvaughn/react-virtualized/pull/617))
 * 🐛 Improved guard for edge-case scrolling issue with rubberband scrolling in iOS. ([@dtoddtarsi](https://github.com/offsky) - [#616](https://github.com/bvaughn/react-virtualized/pull/616))
 * ✨ Replaced `getBoundingClientRect()` with slightly faster `offsetWidth` and `offsetHeight` inside of `AutoSizer`.
+* ✨ `AutoSizer` no longer re-renders nor calls `onResize` callback unless `width` and/or `height` have changed (depending on which properties are being watched).
+* ✨ `AutoSizer` no longer renders content before collecting initial measurements. This prevents `Grid` (and others) from being rendered initially with a `width` or `height` of 0.
 
 ##### 9.3.0
 * 🎉 Added `resetLoadMoreRowsCache` method to `InfiniteLoader` to reset any cached data about loaded rows. This method should be called if any/all loaded data needs to be refetched (eg a filtered list where the search criteria changes). ([#612](https://github.com/bvaughn/react-virtualized/issues/612))

diff --git a/source/AutoSizer/AutoSizer.jest.js b/source/AutoSizer/AutoSizer.jest.js
@@ -5,7 +5,7 @@ import { findDOMNode } from 'react-dom'
 import { render } from '../TestUtils'
 import AutoSizer from './AutoSizer'
 
-function ChildComponent ({ height, width, foo, bar }) {
+function DefaultChildComponent ({ height, width, foo, bar }) {
   return (
     <div>{`width:${width}, height:${height}, foo:${foo}, bar:${bar}`}</div>
   )
@@ -14,10 +14,12 @@ function ChildComponent ({ height, width, foo, bar }) {
 describe('AutoSizer', () => {
   function getMarkup ({
     bar = 123,
+    ChildComponent = DefaultChildComponent,
     disableHeight = false,
     disableWidth = false,
     foo = 456,
     height = 100,
+    onResize,
     paddingBottom = 0,
     paddingLeft = 0,
     paddingRight = 0,
@@ -38,7 +40,11 @@ describe('AutoSizer', () => {
 
     return (
       <div style={style}>
-        <AutoSizer>
+        <AutoSizer
+          disableHeight={disableHeight}
+          disableWidth={disableWidth}
+          onResize={onResize}
+        >
           {({ height, width }) => (
             <ChildComponent
               width={disableWidth ? undefined : width}
@@ -124,4 +130,65 @@ describe('AutoSizer', () => {
     expect(rendered.textContent).toContain('width:300')
     done()
   })
+
+  describe('onResize and (re)render', () => {
+    it('should trigger when size changes', async (done) => {
+      const onResize = jest.fn()
+      const ChildComponent = jest.fn().mockImplementation(DefaultChildComponent)
+      const rendered = findDOMNode(render(getMarkup({
+        ChildComponent,
+        height: 100,
+        onResize,
+        width: 200
+      })))
+      expect(ChildComponent).toHaveBeenCalledTimes(1)
+      expect(onResize).toHaveBeenCalledTimes(1)
+      await simulateResize({ element: rendered, height: 400, width: 300 })
+      expect(ChildComponent).toHaveBeenCalledTimes(2)
+      expect(onResize).toHaveBeenCalledTimes(2)
+      done()
+    })
+
+    it('should only trigger when height changes for disableWidth == true', async (done) => {
+      const onResize = jest.fn()
+      const ChildComponent = jest.fn().mockImplementation(DefaultChildComponent)
+      const rendered = findDOMNode(render(getMarkup({
+        ChildComponent,
+        disableWidth: true,
+        height: 100,
+        onResize,
+        width: 200
+      })))
+      expect(ChildComponent).toHaveBeenCalledTimes(1)
+      expect(onResize).toHaveBeenCalledTimes(1)
+      await simulateResize({ element: rendered, height: 100, width: 300 })
+      expect(ChildComponent).toHaveBeenCalledTimes(1)
+      expect(onResize).toHaveBeenCalledTimes(1)
+      await simulateResize({ element: rendered, height: 200, width: 300 })
+      expect(ChildComponent).toHaveBeenCalledTimes(2)
+      expect(onResize).toHaveBeenCalledTimes(2)
+      done()
+    })
+
+    it('should only trigger when width changes for disableHeight == true', async (done) => {
+      const onResize = jest.fn()
+      const ChildComponent = jest.fn().mockImplementation(DefaultChildComponent)
+      const rendered = findDOMNode(render(getMarkup({
+        ChildComponent,
+        disableHeight: true,
+        height: 100,
+        onResize,
+        width: 200
+      })))
+      expect(ChildComponent).toHaveBeenCalledTimes(1)
+      expect(onResize).toHaveBeenCalledTimes(1)
+      await simulateResize({ element: rendered, height: 200, width: 200 })
+      expect(ChildComponent).toHaveBeenCalledTimes(1)
+      expect(onResize).toHaveBeenCalledTimes(1)
+      await simulateResize({ element: rendered, height: 200, width: 300 })
+      expect(ChildComponent).toHaveBeenCalledTimes(2)
+      expect(onResize).toHaveBeenCalledTimes(2)
+      done()
+    })
+  })
 })
diff --git a/source/AutoSizer/AutoSizer.js b/source/AutoSizer/AutoSizer.js
@@ -79,18 +79,33 @@ export default class AutoSizer extends PureComponent {
       outerStyle.width = 0
     }
 
+    let child
+
+    // Avoid rendering children before the initial measurements have been collected.
+    // At best this would just be wasting cycles.
+    if (
+      height !== 0 &&
+      width !== 0
+    ) {
+      child = children({ height, width })
+    }
+
     return (
       <div
         ref={this._setRef}
         style={outerStyle}
       >
-        {children({ height, width })}
+        {child}
       </div>
     )
   }
 
   _onResize () {
-    const { onResize } = this.props
+    const {
+      disableHeight,
+      disableWidth,
+      onResize
+    } = this.props
 
     // Guard against AutoSizer component being removed from the DOM immediately after being added.
     // This can result in invalid style values which can result in NaN values if we don't handle them.
@@ -105,12 +120,22 @@ export default class AutoSizer extends PureComponent {
     const paddingTop = parseInt(style.paddingTop, 10) || 0
     const paddingBottom = parseInt(style.paddingBottom, 10) || 0
 
-    this.setState({
-      height: height - paddingTop - paddingBottom,
-      width: width - paddingLeft - paddingRight
-    })
-
-    onResize({ height, width })
+    const newHeight = height - paddingTop - paddingBottom
+    const newWidth = width - paddingLeft - paddingRight
+
+    if (
+      !disableHeight &&
+      this.state.height !== newHeight ||
+      !disableWidth &&
+      this.state.width !== newWidth
+    ) {
+      this.setState({
+        height: height - paddingTop - paddingBottom,
+        width: width - paddingLeft - paddingRight
+      })
+
+      onResize({ height, width })
+    }
   }
 
   _setRef (autoSizer) {