Merge branch '#50-memo-a-class-component'

welldone-software · Sep 13, 2019 · a82d25e · a82d25e
2 parents 5ea9777 + a18cae6
commit a82d25e
Show file tree

Hide file tree

Showing 11 changed files with 181 additions and 159 deletions.
diff --git a/jestSetup.js b/jestSetup.js
@@ -1,3 +1,3 @@
-import errorOnConsoleOutput from './testUtils/errorOnConsoleOutput'
+import {errorOnConsoleOutput} from '@welldone-software/jest-console-handler'
 
 global.flushConsoleOutput = errorOnConsoleOutput()
diff --git a/package.json b/package.json
@@ -68,6 +68,7 @@
     "@testing-library/jest-dom": "^4.1.0",
     "@testing-library/react": "^9.1.4",
     "@types/react": "^16.9.2",
+    "@welldone-software/jest-console-handler": "^0.1.0",
     "acorn-walk": "^7.0.0",
     "astring": "^1.4.1",
     "babel-core": "^7.0.0-bridge.0",

diff --git a/src/patches/patchForwardRefComponent.js b/src/patches/patchForwardRefComponent.js
@@ -1,40 +1,19 @@
 import {defaults} from 'lodash'
 
-import getUpdateInfo from '../getUpdateInfo'
 import getDisplayName from '../getDisplayName'
-import {REACT_MEMO_TYPE} from '../consts'
+import {isMemoComponent} from '../utils'
+import patchFunctionalComponent from './patchFunctionalComponent'
 
 export default function patchForwardRefComponent(ForwardRefComponent, displayName, React, options){
   const {render: InnerForwardRefComponent} = ForwardRefComponent
 
-  const isInnerComponentMemoized = InnerForwardRefComponent.$$typeof === REACT_MEMO_TYPE
-  const WrappedFunctionalComponent = isInnerComponentMemoized ? InnerForwardRefComponent.type : InnerForwardRefComponent
-
-  function WDYRWrappedByReactForwardRefFunctionalComponent(){
-    const nextProps = arguments[0]
-    const ref = React.useRef()
-
-    const prevProps = ref.current
-    ref.current = nextProps
-
-    if(prevProps){
-      const notification = getUpdateInfo({
-        Component: ForwardRefComponent,
-        displayName,
-        prevProps,
-        nextProps,
-        options
-      })
-
-      // if a memoized functional component re-rendered without props change / prop values change
-      // it was probably caused by a hook and we should not care about it
-      if(notification.reason.propsDifferences && notification.reason.propsDifferences.length > 0){
-        options.notifier(notification)
-      }
-    }
-
-    return WrappedFunctionalComponent(...arguments)
-  }
+  const isInnerComponentMemoized = isMemoComponent(InnerForwardRefComponent)
+  const WrappedFunctionalComponent = isInnerComponentMemoized ?
+    InnerForwardRefComponent.type : InnerForwardRefComponent
+
+  const WDYRWrappedByReactForwardRefFunctionalComponent = (
+    patchFunctionalComponent(WrappedFunctionalComponent, isInnerComponentMemoized, displayName, React, options)
+  )
 
   WDYRWrappedByReactForwardRefFunctionalComponent.displayName = getDisplayName(WrappedFunctionalComponent)
   WDYRWrappedByReactForwardRefFunctionalComponent.ComponentForHooksTracking = WrappedFunctionalComponent

diff --git a/src/patches/patchForwardRefComponent.test.js b/src/patches/patchForwardRefComponent.test.js
@@ -63,49 +63,51 @@ test('forward ref', () => {
 })
 
 test('forward ref a memo component', () => {
-  /* turns out this is not supported by react at this point. */
-
-  //   const content = 'My component!!!'
-  //
-  //   const MyComponent = React.forwardRef(React.memo((props, ref) => {
-  //     return <div ref={ref}>{content}</div>
-  //   }, () => true))
-  //
-  //   MyComponent.whyDidYouRender = true
-  //
-  //   let componentContentFromRef = null
-  //   let timesRefWasCalled = 0
-  //
-  //   const handleRef = ref => {
-  //     if(!ref){
-  //       return
-  //     }
-  //     timesRefWasCalled++
-  //     componentContentFromRef = ref.innerHTML
-  //   }
-  //
-  //   const {rerender} = rtl.render(
-  //     <MyComponent a={[]} ref={handleRef}/>
-  //   )
-  //
-  //   rerender(
-  //     <MyComponent a={[]} ref={handleRef}/>
-  //   )
-  //
-  //   expect(componentContentFromRef).toBe(content)
-  //   expect(timesRefWasCalled).toBe(1)
-  //
-  //   expect(updateInfos).toHaveLength(1)
-  //   expect(updateInfos[0].reason).toEqual({
-  //     propsDifferences: [
-  //       {
-  //         pathString: 'a',
-  //         diffType: diffTypes.deepEquals,
-  //         prevValue: [],
-  //         nextValue: []
-  //       }
-  //     ],
-  //     stateDifferences: false,
-  //     hookDifferences: false
-  //   })
+  // This is not supported by React 16.9
+  expect(() => {
+    const content = 'My component!!!'
+
+    const MyComponent = React.forwardRef(React.memo((props, ref) => {
+      return <div ref={ref}>{content}</div>
+    }, () => true))
+
+    MyComponent.whyDidYouRender = true
+
+    let componentContentFromRef = null
+    let timesRefWasCalled = 0
+
+    const handleRef = ref => {
+      if(!ref){
+        return
+      }
+      timesRefWasCalled++
+      componentContentFromRef = ref.innerHTML
+    }
+
+    const {rerender} = rtl.render(
+      <MyComponent a={[]} ref={handleRef}/>
+    )
+
+    rerender(
+      <MyComponent a={[]} ref={handleRef}/>
+    )
+
+    expect(componentContentFromRef).toBe(content)
+    expect(timesRefWasCalled).toBe(1)
+
+    expect(updateInfos).toHaveLength(1)
+    expect(updateInfos[0].reason).toEqual({
+      propsDifferences: [
+        {
+          pathString: 'a',
+          diffType: diffTypes.deepEquals,
+          prevValue: [],
+          nextValue: []
+        }
+      ],
+      stateDifferences: false,
+      hookDifferences: false
+    })
+  }).toThrow()
+  global.flushConsoleOutput()
 })
diff --git a/src/patches/patchFunctionalComponent.js b/src/patches/patchFunctionalComponent.js
@@ -2,30 +2,35 @@ import {defaults} from 'lodash'
 
 import getUpdateInfo from '../getUpdateInfo'
 
-export default function patchFunctionalComponent(FunctionalComponent, displayName, React, options){
-  function WDYRFunctionalComponent(nextProps){
+export default function patchFunctionalComponent(FunctionalComponent, isPure, displayName, React, options){
+  function WDYRFunctionalComponent(){
+    const nextProps = arguments[0]
     const ref = React.useRef()
 
     const prevProps = ref.current
     ref.current = nextProps
 
     if(prevProps){
-      const notification = getUpdateInfo({
+      const updateInfo = getUpdateInfo({
         Component: FunctionalComponent,
         displayName,
         prevProps,
         nextProps,
         options
       })
 
-      // if a functional component re-rendered without a props change
-      // it was probably caused by a hook and we should not care about it
-      if(notification.reason.propsDifferences){
-        options.notifier(notification)
+      const shouldNotify = (
+        updateInfo.reason.propsDifferences && (
+          !(isPure && updateInfo.reason.propsDifferences.length === 0)
+        )
+      )
+
+      if(shouldNotify){
+        options.notifier(updateInfo)
       }
     }
 
-    return FunctionalComponent(nextProps)
+    return FunctionalComponent(...arguments)
   }
 
   WDYRFunctionalComponent.displayName = displayName

diff --git a/src/patches/patchMemoComponent.js b/src/patches/patchMemoComponent.js
@@ -1,48 +1,31 @@
 import {defaults} from 'lodash'
 
-import getUpdateInfo from '../getUpdateInfo'
 import getDisplayName from '../getDisplayName'
 
-import {REACT_FORWARD_REF_TYPE} from '../consts'
+import {isForwardRefComponent, isReactClassComponent} from '../utils'
+import patchClassComponent from './patchClassComponent'
+import patchFunctionalComponent from './patchFunctionalComponent'
 
 export default function patchMemoComponent(MemoComponent, displayName, React, options){
   const {type: InnerMemoComponent} = MemoComponent
 
-  const isInnerMemoComponentForwardRefs = InnerMemoComponent.$$typeof === REACT_FORWARD_REF_TYPE
-  const WrappedFunctionalComponent = isInnerMemoComponentForwardRefs ? InnerMemoComponent.render : InnerMemoComponent
+  const isInnerMemoComponentAClassComponent = isReactClassComponent(InnerMemoComponent)
+  const isInnerMemoComponentForwardRefs = isForwardRefComponent(InnerMemoComponent)
 
-  function WDYRWrappedByMemoFunctionalComponent(){
-    const nextProps = arguments[0]
-    const ref = React.useRef()
+  const WrappedFunctionalComponent = isInnerMemoComponentForwardRefs ?
+    InnerMemoComponent.render :
+    InnerMemoComponent
 
-    const prevProps = ref.current
-    ref.current = nextProps
+  const PatchedInnerComponent = isInnerMemoComponentAClassComponent ?
+    patchClassComponent(WrappedFunctionalComponent, displayName, React, options) :
+    patchFunctionalComponent(WrappedFunctionalComponent, true, displayName, React, options)
 
-    if(prevProps){
-      const notification = getUpdateInfo({
-        Component: MemoComponent,
-        displayName,
-        prevProps,
-        nextProps,
-        options
-      })
-
-      // if a memoized functional component re-rendered without props change / prop values change
-      // it was probably caused by a hook and we should not care about it
-      if(notification.reason.propsDifferences && notification.reason.propsDifferences.length > 0){
-        options.notifier(notification)
-      }
-    }
-
-    return WrappedFunctionalComponent(...arguments)
-  }
-
-  WDYRWrappedByMemoFunctionalComponent.displayName = getDisplayName(WrappedFunctionalComponent)
-  WDYRWrappedByMemoFunctionalComponent.ComponentForHooksTracking = MemoComponent
-  defaults(WDYRWrappedByMemoFunctionalComponent, WrappedFunctionalComponent)
+  PatchedInnerComponent.displayName = getDisplayName(WrappedFunctionalComponent)
+  PatchedInnerComponent.ComponentForHooksTracking = MemoComponent
+  defaults(PatchedInnerComponent, WrappedFunctionalComponent)
 
   const WDYRMemoizedFunctionalComponent = React.memo(
-    isInnerMemoComponentForwardRefs ? React.forwardRef(WDYRWrappedByMemoFunctionalComponent) : WDYRWrappedByMemoFunctionalComponent,
+    isInnerMemoComponentForwardRefs ? React.forwardRef(PatchedInnerComponent) : PatchedInnerComponent,
     MemoComponent.compare
   )
 

diff --git a/src/patches/patchMemoComponent.test.js b/src/patches/patchMemoComponent.test.js
@@ -151,3 +151,72 @@ test('memo a forward ref component', () => {
     hookDifferences: false
   })
 })
+
+test('memo a class component', () => {
+  class ClassComponent extends React.Component{
+    render(){
+      return <div>hi!</div>
+    }
+  }
+
+  const MyComponent = React.memo(ClassComponent)
+
+  MyComponent.whyDidYouRender = true
+
+  const {rerender} = rtl.render(
+    <MyComponent a={[]}/>
+  )
+
+  rerender(
+    <MyComponent a={[]}/>
+  )
+
+  expect(updateInfos).toHaveLength(1)
+  expect(updateInfos[0].reason).toEqual({
+    propsDifferences: [
+      {
+        pathString: 'a',
+        diffType: diffTypes.deepEquals,
+        prevValue: [],
+        nextValue: []
+      }
+    ],
+    stateDifferences: false,
+    hookDifferences: false
+  })
+})
+
+test('memo a pure class component', () => {
+  class ClassComponent extends React.PureComponent{
+    render(){
+      return <div>hi!</div>
+    }
+  }
+
+  const MyComponent = React.memo(ClassComponent)
+
+  MyComponent.whyDidYouRender = true
+
+  const {rerender} = rtl.render(
+    <MyComponent a={[]}/>
+  )
+
+  rerender(
+    <MyComponent a={[]}/>
+  )
+
+  expect(updateInfos).toHaveLength(1)
+  expect(updateInfos[0].reason).toEqual({
+    propsDifferences: [
+      {
+        pathString: 'a',
+        diffType: diffTypes.deepEquals,
+        prevValue: [],
+        nextValue: []
+      }
+    ],
+    stateDifferences: false,
+    hookDifferences: false
+  })
+  global.flushConsoleOutput()
+})
diff --git a/src/utils.js b/src/utils.js
@@ -1,4 +1,6 @@
 // copied from https://github.com/facebook/react/blob/master/packages/react-reconciler/src/ReactTypeOfMode.js
+import {REACT_FORWARD_REF_TYPE, REACT_MEMO_TYPE} from './consts'
+
 const StrictMode = 0b0001
 
 // based on "findStrictRoot" from https://github.com/facebook/react/blob/master/packages/react-reconciler/src/ReactStrictModeWarnings.js
@@ -13,3 +15,15 @@ export function checkIfInsideAStrictModeTree(reactComponentInstance){
   }
   return false
 }
+
+export function isReactClassComponent(Component){
+  return Component.prototype && !!Component.prototype.isReactComponent
+}
+
+export function isMemoComponent(Component){
+  return Component.$$typeof === REACT_MEMO_TYPE
+}
+
+export function isForwardRefComponent(Component){
+  return Component.$$typeof === REACT_FORWARD_REF_TYPE
+}