From a18cae61669d0257c50369f2409c3edad54c8de8 Mon Sep 17 00:00:00 2001 From: vzaidman Date: Fri, 13 Sep 2019 12:00:19 +0300 Subject: [PATCH] fix #50 - support memoization of class components --- src/patches/patchForwardRefComponent.js | 39 ++------- src/patches/patchForwardRefComponent.test.js | 92 ++++++++++---------- src/patches/patchFunctionalComponent.js | 21 +++-- src/patches/patchMemoComponent.js | 47 ++++------ src/patches/patchMemoComponent.test.js | 1 + src/utils.js | 14 +++ src/whyDidYouRender.js | 14 +-- 7 files changed, 106 insertions(+), 122 deletions(-) diff --git a/src/patches/patchForwardRefComponent.js b/src/patches/patchForwardRefComponent.js index 3412826..c5381d2 100644 --- 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 index 8917c8a..d390aab 100644 --- 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
{content}
- // }, () => true)) - // - // MyComponent.whyDidYouRender = true - // - // let componentContentFromRef = null - // let timesRefWasCalled = 0 - // - // const handleRef = ref => { - // if(!ref){ - // return - // } - // timesRefWasCalled++ - // componentContentFromRef = ref.innerHTML - // } - // - // const {rerender} = rtl.render( - // - // ) - // - // rerender( - // - // ) - // - // 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
{content}
+ }, () => true)) + + MyComponent.whyDidYouRender = true + + let componentContentFromRef = null + let timesRefWasCalled = 0 + + const handleRef = ref => { + if(!ref){ + return + } + timesRefWasCalled++ + componentContentFromRef = ref.innerHTML + } + + const {rerender} = rtl.render( + + ) + + rerender( + + ) + + 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 index e583b0b..a53ee15 100644 --- a/src/patches/patchFunctionalComponent.js +++ b/src/patches/patchFunctionalComponent.js @@ -2,15 +2,16 @@ 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, @@ -18,14 +19,18 @@ export default function patchFunctionalComponent(FunctionalComponent, displayNam 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 index 2081ded..57c03dc 100644 --- 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 index f59435a..3e9de9c 100644 --- a/src/patches/patchMemoComponent.test.js +++ b/src/patches/patchMemoComponent.test.js @@ -218,4 +218,5 @@ test('memo a pure class component', () => { stateDifferences: false, hookDifferences: false }) + global.flushConsoleOutput() }) diff --git a/src/utils.js b/src/utils.js index 40506a5..0cb03e0 100644 --- 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 +} diff --git a/src/whyDidYouRender.js b/src/whyDidYouRender.js index 2c2bafa..b831000 100644 --- a/src/whyDidYouRender.js +++ b/src/whyDidYouRender.js @@ -10,7 +10,7 @@ import patchFunctionalComponent from './patches/patchFunctionalComponent' import patchMemoComponent from './patches/patchMemoComponent' import patchForwardRefComponent from './patches/patchForwardRefComponent' -import {REACT_MEMO_TYPE, REACT_FORWARD_REF_TYPE} from './consts' +import {isForwardRefComponent, isMemoComponent, isReactClassComponent} from './utils' function trackHookChanges(hookName, {path: hookPath}, hookResult, React, options){ const nextHook = hookResult @@ -52,19 +52,19 @@ function trackHookChanges(hookName, {path: hookPath}, hookResult, React, options } function createPatchedComponent(componentsMap, Component, displayName, React, options){ - if(Component.$$typeof === REACT_MEMO_TYPE){ + if(isMemoComponent(Component)){ return patchMemoComponent(Component, displayName, React, options) } - if(Component.$$typeof === REACT_FORWARD_REF_TYPE){ + if(isForwardRefComponent(Component)){ return patchForwardRefComponent(Component, displayName, React, options) } - if(Component.prototype && Component.prototype.isReactComponent){ + if(isReactClassComponent(Component)){ return patchClassComponent(Component, displayName, React, options) } - return patchFunctionalComponent(Component, displayName, React, options) + return patchFunctionalComponent(Component, false, displayName, React, options) } function getPatchedComponent(componentsMap, Component, displayName, React, options){ @@ -102,8 +102,8 @@ export default function whyDidYouRender(React, userOptions){ isShouldTrack = ( ( typeof componentNameOrComponent === 'function' || - componentNameOrComponent.$$typeof === REACT_MEMO_TYPE || - componentNameOrComponent.$$typeof === REACT_FORWARD_REF_TYPE + isMemoComponent(componentNameOrComponent) || + isForwardRefComponent(componentNameOrComponent) ) && shouldTrack(componentNameOrComponent, getDisplayName(componentNameOrComponent), options) )