feat(overmind-react): pass new object of overmind prop only when a state change has occured

christianalfoni · christianalfoni · commit 601d48fe953b · 2018-12-31T13:57:34.000+01:00
BREAKING CHANGE:
HOC no longer uses PureComponent
diff --git a/packages/node_modules/overmind-react/src/index.test.tsx b/packages/node_modules/overmind-react/src/index.test.tsx
@@ -134,4 +134,55 @@ describe('React', () => {
     expect(ConnectedFoo.name).toBe('ConnectFooComponent')
     expect(ConnectedBar.name).toBe('ConnectBarComponent')
   })
+
+  test('should allow using shouldComponentUpdate', () => {
+    let renderCount = 0
+
+    const doThis: Action = ({ state }) => {
+      state.foo = 'bar2'
+    }
+    const config = {
+      state: {
+        foo: 'bar',
+      },
+      actions: {
+        doThis,
+      },
+    }
+
+    type IConfig = {
+      state: {
+        foo: typeof config.state.foo
+      }
+      actions: {
+        doThis: typeof doThis
+      }
+    }
+
+    const app = new Overmind(config)
+
+    type Action<Input = void> = TAction<IConfig, Input>
+
+    const connect = createConnect(app)
+
+    class FooComponent extends React.Component<TConnect<typeof config>> {
+      shouldComponentUpdate(nextProps) {
+        return this.props.overmind !== nextProps.overmind
+      }
+      render() {
+        renderCount++
+        return <h1>{this.props.overmind.state.foo}</h1>
+      }
+    }
+
+    const ConnectedFoo = connect(FooComponent)
+
+    const tree = renderer.create(<ConnectedFoo />).toJSON()
+
+    expect(renderCount).toBe(1)
+
+    app.actions.doThis()
+
+    expect(renderCount).toBe(2)
+  })
 })
diff --git a/packages/node_modules/overmind-react/src/index.ts b/packages/node_modules/overmind-react/src/index.ts
@@ -3,9 +3,9 @@ import {
   // @ts-ignore
   __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED,
   ClassicComponentClass,
+  Component,
   ComponentClass,
   createElement,
-  PureComponent,
   StatelessComponent,
   // @ts-ignore
   useEffect,
@@ -160,72 +160,137 @@ export const createConnect = <A extends Overmind<Configuration>>(
       }
     }
 
-    class HOC extends PureComponent {
-      tree = (overmind as any).proxyStateTree.getTrackStateTree()
-      componentInstanceId = componentInstanceId++
-      currentFlushId = 0
-      componentDidMount() {
-        overmind.eventHub.emitAsync(EventType.COMPONENT_ADD, {
-          componentId: populatedComponent.__componentId,
-          componentInstanceId: this.componentInstanceId,
-          name,
-          paths: Array.from(this.tree.pathDependencies) as any,
-        })
-      }
-      componentDidUpdate() {
-        overmind.eventHub.emitAsync(EventType.COMPONENT_UPDATE, {
-          componentId: populatedComponent.__componentId,
-          componentInstanceId: this.componentInstanceId,
-          name,
-          flushId: this.currentFlushId,
-          paths: Array.from(this.tree.pathDependencies as Set<string>),
-        })
-      }
-      componentWillUnmount() {
-        ;(overmind as any).proxyStateTree.disposeTree(this.tree)
-        overmind.eventHub.emitAsync(EventType.COMPONENT_REMOVE, {
-          componentId: populatedComponent.__componentId,
-          componentInstanceId: this.componentInstanceId,
-          name,
-        })
-      }
-      onUpdate = (mutatons, paths, flushId) => {
-        this.currentFlushId = flushId
-        this.forceUpdate()
-      }
-      render() {
-        if (isClassComponent) {
+    if (IS_PRODUCTION) {
+      class HOC extends Component {
+        tree = (overmind as any).proxyStateTree.getTrackStateTree()
+        state: {
+          overmind: any
+        }
+        constructor(props) {
+          super(props)
+          this.state = {
+            overmind: {
+              state: this.tree.state,
+              actions: overmind.actions,
+              addMutationListener: overmind.addMutationListener,
+              onUpdate: this.onUpdate,
+              tree: this.tree,
+            },
+          }
+        }
+        componentWillUnmount() {
+          ;(overmind as any).proxyStateTree.disposeTree(this.tree)
+        }
+        onUpdate = () => {
+          this.setState({
+            overmind: {
+              state: this.tree.state,
+              actions: overmind.actions,
+              addMutationListener: overmind.addMutationListener,
+              onUpdate: this.onUpdate,
+              tree: this.tree,
+            },
+          })
+        }
+        render() {
+          if (isClassComponent) {
+            return createElement(component, {
+              ...this.props,
+              overmind: this.state.overmind,
+            } as any)
+          }
+
+          this.tree.track(this.onUpdate)
+
           return createElement(component, {
             ...this.props,
+            overmind: this.state.overmind,
+          } as any)
+        }
+      }
+
+      return HOC as any
+    } else {
+      class HOC extends Component {
+        tree = (overmind as any).proxyStateTree.getTrackStateTree()
+        componentInstanceId = componentInstanceId++
+        currentFlushId = 0
+        state: {
+          overmind: any
+        }
+        constructor(props) {
+          super(props)
+          this.state = {
             overmind: {
               state: this.tree.state,
               actions: overmind.actions,
               addMutationListener: overmind.addMutationListener,
               onUpdate: this.onUpdate,
               tree: this.tree,
             },
-          } as any)
+          }
+        }
+        componentDidMount() {
+          overmind.eventHub.emitAsync(EventType.COMPONENT_ADD, {
+            componentId: populatedComponent.__componentId,
+            componentInstanceId: this.componentInstanceId,
+            name,
+            paths: Array.from(this.tree.pathDependencies) as any,
+          })
+        }
+        componentDidUpdate() {
+          overmind.eventHub.emitAsync(EventType.COMPONENT_UPDATE, {
+            componentId: populatedComponent.__componentId,
+            componentInstanceId: this.componentInstanceId,
+            name,
+            flushId: this.currentFlushId,
+            paths: Array.from(this.tree.pathDependencies as Set<string>),
+          })
+        }
+        componentWillUnmount() {
+          ;(overmind as any).proxyStateTree.disposeTree(this.tree)
+          overmind.eventHub.emitAsync(EventType.COMPONENT_REMOVE, {
+            componentId: populatedComponent.__componentId,
+            componentInstanceId: this.componentInstanceId,
+            name,
+          })
         }
+        onUpdate = (mutatons, paths, flushId) => {
+          this.currentFlushId = flushId
+          this.setState({
+            overmind: {
+              state: this.tree.state,
+              actions: overmind.actions,
+              addMutationListener: overmind.addMutationListener,
+              onUpdate: this.onUpdate,
+              tree: this.tree,
+            },
+          })
+        }
+        render() {
+          if (isClassComponent) {
+            return createElement(component, {
+              ...this.props,
+              overmind: this.state.overmind,
+            } as any)
+          }
 
-        this.tree.track(this.onUpdate)
+          this.tree.track(this.onUpdate)
 
-        return createElement(component, {
-          ...this.props,
-          overmind: {
-            state: this.tree.state,
-            actions: overmind.actions,
-            addMutationListener: overmind.addMutationListener,
-          },
-        } as any)
+          return createElement(component, {
+            ...this.props,
+            overmind: this.state.overmind,
+          } as any)
+        }
       }
-    }
 
-    Object.defineProperties(HOC, {
-      name: {
-        value: 'Connect' + (component.displayName || component.name || ''),
-      },
-    })
+      Object.defineProperties(HOC, {
+        name: {
+          value: 'Connect' + (component.displayName || component.name || ''),
+        },
+      })
 
-    return HOC as any
+      return HOC as any
+    }
   }
 }
diff --git a/packages/overmind-website/examples/guide/usingovermindwithreact/hoc_compareprop.ts b/packages/overmind-website/examples/guide/usingovermindwithreact/hoc_compareprop.ts
@@ -0,0 +1,48 @@
+export default (ts) =>
+  ts
+    ? [
+        {
+          fileName: 'components/App.tsx',
+          code: `
+import * as React from 'react'
+import { connect, Connect } from '../overmind'
+
+type Props = {} & Connect
+
+class App extends React.Component<Props> {
+  shouldComponentUpdate(nextProps: Props) {
+    return this.props.overmind !== nextProps.overmind
+  }
+  render() {
+    const { state, actions } = this.props.overmind
+
+    return <div />
+  }
+}
+
+export default connect(App)
+`,
+        },
+      ]
+    : [
+        {
+          fileName: 'components/App.jsx',
+          code: `
+import React from 'react'
+import { connect } from '../overmind'
+
+class App extends React.Component {
+  shouldComponentUpdate(nextProps) {
+    return this.props.overmind !== nextProps.overmind
+  }
+  render() {
+    const { state, actions } = this.props.overmind
+
+    return <div />
+  }
+}
+
+export default connect(App)
+`,
+        },
+      ]
diff --git a/packages/overmind-website/guides/beginner/06_usingovermindwithreact.md b/packages/overmind-website/guides/beginner/06_usingovermindwithreact.md
@@ -2,7 +2,7 @@
 
 There are two different ways to connect Overmind to React. You can either use a traditional **Higher Order Component** or you can use the new **hooks** api to expose state and actions.
 
-When you connect Overmind to a component you ensure that whenever any tracked state changes only components interested in that state will rerender and they will rerender "at their point in the UI structure". That means we remove a lot of unnecessary work from React. There is no reason for the whole React component tree to rerender when only one component is interested in a change.
+When you connect Overmind to a component you ensure that whenever any tracked state changes only components interested in that state will rerender and they will rerender "at their point in the component tree". That means we remove a lot of unnecessary work from React. There is no reason for the whole React component tree to rerender when only one component is interested in a change.
 
 ## With HOC
 ```marksy
@@ -11,7 +11,15 @@ h(Example, { name: "guide/usingovermindwithreact/hoc" })
 
 ### Rendering
 
-When you connect a component with the **connect** HOC it will automatically optimize with **PureComponent**, meaning that the only way your connected component can render is if the parent passes a changed prop or the state that is tracked changes.
+When you connect a component with the **connect HOC** it will be responsible for tracking and trigger a render when the tracked state is updated. The **overmind** prop passed to the component you defined holds the state and actions. If you want to detect inside your component that it was indeed an Overmind state change causing the render you can compare the **overmind** prop itself.
+
+```marksy
+h(Example, { name: "guide/usingovermindwithreact/hoc_compareprop" })
+```
+
+You will not be able to compare a previous state value in Overmind with the new. That is simply because Overmind is not immutable and it should not be. You will not use **shouldComponentUpdate** to compare state in Overmind, though you can of course still use it to compare props from a parent. This is a bit of a mindshift if you come from Redux, but it actually removes the mental burden of doing this stuff.
+
+If you previously used **componentDidUpdate** to trigger an effect, that is no longer necessary either. You rather listen to state changes in Overmind using **addMutationListener** specified below in *effects*.
 
 ### Passing state as props
 
@@ -23,7 +31,7 @@ h(Example, { name: "guide/usingovermindwithreact/hoc_passprop" })
 
 ### Effects
 
-To run effects in components based on changes to state you use the **subscribe** function in the lifecycle hooks of React.
+To run effects in components based on changes to state you use the **addMutationListener** function in the lifecycle hooks of React.
 
 ```marksy
 h(Example, { name: "guide/usingovermindwithreact/hoc_effect" })
@@ -36,7 +44,7 @@ h(Example, { name: "guide/usingovermindwithreact/hook" })
 
 ### Rendering
 
-When you use the Overmind hook it will ensure that the component will render when any tracked state changes. It will not optimize related to checking if parent props has changed. That means whenever the parent renders, this component renders as well. You will need to wrap your component with [**React.memo**](https://reactjs.org/docs/react-api.html#reactmemo).
+When you use the Overmind hook it will ensure that the component will render when any tracked state changes. It will not do anything related to the props passed to the component. That means whenever the parent renders, this component renders as well. You will need to wrap your component with [**React.memo**](https://reactjs.org/docs/react-api.html#reactmemo) to optimize rendering caused by a parent.
 
 ### Passing state as props
 
