remove global listener

renatorib · Sep 15, 2017 · f3ec0da · f3ec0da
1 parent 01eebdb
commit f3ec0da
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 63 deletions.
diff --git a/README.md b/README.md
@@ -169,23 +169,6 @@ const mapSizesToProps = sizes => ({
 ```
 > `sizes` argument is an object with `width` and `height` properties and represents DOM window width and height.
 
-## Performance Notes
-
-#### Global listener
-Window resize event listener is grouped at one global listener only.
-So you can have as many components as you want using Sizes hoc, which will not have multiple resize listeners,
-but only one that will dispatch for all instances.  
-
-Don't worry, Sizes handles `componentWillUnmount` to remove unnecessary callbacks.
-When each component unmounted, it unsubscribe for global dispatches, and when last component is unmounted,
-the listener is removed.
-
-#### Throttle
-By now the listener callback is hardcoded as [throttle](https://css-tricks.com/debouncing-throttling-explained-examples/)
-function of 200ms. Because of having a global listener, we have a limitation on changing this behavior.
-I don't see it as tradeoff, and I think it can have good workarounds.
-Then, for the future, I intend to work to bring a solution to this important customization.
-
 ## Guide
 
 #### mapSizesToProps(sizes)
@@ -215,6 +198,11 @@ const mapSizesToProps = ({ width }) => ({
 });
 ```
 
+## Performance Notes
+
+#### Shallow Compare
+React Sizes do a shallow compare in props generated from `mapSizesToProps` (called `propsToPass`), so it will only rerender when they really change. If you create a deep data sctructure, this can generate false positives. In these cases, we recommend using immutable for a more reliable shallow compare result. Or just don't use deep data structures, if possible.
+
 ## Contribute
 
 You can help improving this project sending PRs and helping with issues.  

diff --git a/package.json b/package.json
@@ -8,7 +8,7 @@
     "lint": "eslint src",
     "build": "cross-env NODE_ENV=production npm run lint && npm run clear && babel src -d lib --source-maps",
     "prepublish": "npm run lint && npm run clear && npm run build",
-    "storybook": "start-storybook -p 6006",
+    "storybook": "cross-env NODE_ENV=debug start-storybook -p 6006",
     "build-storybook": "build-storybook"
   },
   "repository": {

diff --git a/src/withSizes.js b/src/withSizes.js
@@ -1,8 +1,6 @@
 /* eslint-disable no-console */
 
 import React, { Component } from 'react'
-import { v4 } from 'uuid'
-import keys from 'lodash.keys'
 import throttle from 'lodash.throttle'
 
 import * as presets from './presets'
@@ -11,67 +9,51 @@ import getDisplayName from './utils/getDisplayName'
 import shallowDiff from './utils/shallowDiff'
 import getWindowSizes from './utils/getWindowSizes'
 
-const debug = process.env.NODE_ENV !== 'production'
-
-let resizeListener
-const listeners = {}
+const debug = process && process.env &&
+  process.env.NODE_ENV === 'debug'
 
 const withSizes = (...mappedSizesToProps) => (WrappedComponent) => {
-  const parseMappedSizesToProps = (dimensions, props) => {
-    const propsToPass = mappedSizesToProps
+  const parseMappedSizesToProps = (dimensions, props) =>
+    mappedSizesToProps
       .map(check => check(dimensions, props))
-      .reduce((acc, props) => ({...acc, ...props}), {})
-
-    return propsToPass
-  }
+      .reduce((acc, props) => ({ ...acc, ...props }), {})
 
-  return class extends Component {
-    static displayName = `withSizes(${getDisplayName(WrappedComponent)})`;
+  return class ComponentWithSizes extends Component {
+    static displayName = `withSizes(${getDisplayName(WrappedComponent)})`
 
     state = {
-      id: `A${v4()}`,
+      initialSizes: getWindowSizes(window),
       propsToPass: parseMappedSizesToProps(getWindowSizes(window), this.props),
-    };
-
-    componentDidMount() {
-      if (!resizeListener) {
-        resizeListener = window.addEventListener('resize', this.throttledWindowResize)
-      }
-
-      listeners[this.state.id] = this.listenerCallback
-
-      this.dispatchSizes()
     }
 
-    componentWillUnmount() {
-      delete listeners[this.state.id]
-      if (keys(listeners).length < 1) {
-        window.removeEventListener('resize', this.throttledWindowResize)
-        resizeListener = null
-      }
-    }
+    /* Dispatching & Throttling */
 
-    listenerCallback = (sizes) => {
-      const propsToPass = parseMappedSizesToProps(sizes, this.props)
+    dispatchSizes = () => {
+      const propsToPass = parseMappedSizesToProps(getWindowSizes(window), this.props)
 
       if (shallowDiff(propsToPass, this.state.propsToPass)) {
         this.setState({ propsToPass })
       }
     }
 
-    dispatchSizes = () => {
-      keys(listeners).forEach(key => {
-        const callback = listeners[key]
+    throttledDispatchSizes = (
+      throttle(this.dispatchSizes, 200)
+    )
 
-        if (typeof callback === 'function') {
-          callback(getWindowSizes(window))
-        }
-      })
-    };
+    /* Lifecycles */
 
-    throttledWindowResize = (
-      throttle(this.dispatchSizes, 200)
-    );
+    componentDidMount() {
+      window.addEventListener('resize', this.throttledDispatchSizes)
+
+      /* dispatch if aren't computed on first render */
+      if (!this.state.initialSizes.canUseDOM) {
+        this.dispatchSizes()
+      }
+    }
+
+    componentWillUnmount() {
+      window.removeEventListener('resize', this.throttledDispatchSizes)
+    }
 
     render() {
       if (debug) console.log('render', this.state.propsToPass)