remove componentWillReceiveProps from proxies #918

examples/styled-components/package.json

@@ -20,7 +20,7 @@
     "react": "^16.2.0",
     "react-dom": "^16.2.0",
     "react-emotion": "^8.0.12",
-    "react-hot-loader": "next",
+    "react-hot-loader": "^4.0.1",
     "styled-components": "^2.4.0"
   }
 }

src/AppContainer.dev.js

@@ -15,20 +15,15 @@ class AppContainer extends React.Component {
     }
   }
 
-  componentWillReceiveProps() {
-    if (this.state.generation !== getGeneration()) {
+  static getDerivedStateFromProps(nextProps, prevState) {
+    if (prevState.generation !== getGeneration()) {
       // Hot reload is happening.
-
-      this.setState({
+      return {
         error: null,
         generation: getGeneration(),
-      })
-
-      // perform sandboxed render to find similarities between new and old code
-      renderReconciler(this, true)
-      // it is possible to flush update out of render cycle
-      flushScheduledUpdates()
+      }
     }
+    return null
   }
 
   shouldComponentUpdate(prevProps, prevState) {

src/proxy/createClassProxy.js

@@ -38,8 +38,8 @@ const blackListedClassMembers = [
 ]
 
 const defaultRenderOptions = {
-  componentWillReceiveProps: identity,
   componentWillRender: identity,
+  componentDidUpdate: result => result,
   componentDidRender: result => result,
 }
 
@@ -129,9 +129,9 @@ function createClassProxy(InitialComponent, proxyKey, options) {
       target[PROXY_IS_MOUNTED] = true
     },
   )
-  const componentWillReceiveProps = lifeCycleWrapperFactory(
-    'componentWillReceiveProps',
-    renderOptions.componentWillReceiveProps,
+  const componentDidUpdate = lifeCycleWrapperFactory(
+    'componentDidUpdate',
+    renderOptions.componentDidUpdate,
   )
   const componentWillUnmount = lifeCycleWrapperFactory(
     'componentWillUnmount',
@@ -157,7 +157,7 @@ function createClassProxy(InitialComponent, proxyKey, options) {
       result = CurrentComponent.prototype.render.call(this)
     }
 
-    return renderOptions.componentDidRender(result)
+    return renderOptions.componentDidRender.call(this, result)
   }
 
   function proxiedRender() {
@@ -171,7 +171,7 @@ function createClassProxy(InitialComponent, proxyKey, options) {
       render: proxiedRender,
       hotComponentRender,
       componentDidMount,
-      componentWillReceiveProps,
+      componentDidUpdate,
       componentWillUnmount,
     })
   }

src/reconciler/hotReplacementRender.js

@@ -221,6 +221,10 @@ export const flushScheduledUpdates = () => {
   )
 }
 
+export const unscheduleUpdate = instance => {
+  scheduledUpdates = scheduledUpdates.filter(inst => inst !== instance)
+}
+
 const scheduleInstanceUpdate = instance => {
   scheduledUpdates.push(instance)
   if (!scheduledUpdate) {

src/reconciler/index.js

@@ -1,11 +1,12 @@
 import getReactStack from '../internal/getReactStack'
 import hotReplacementRender, {
-  flushScheduledUpdates
+  flushScheduledUpdates,
+  unscheduleUpdate
 } from './hotReplacementRender'
 
 const reconcileHotReplacement = ReactInstance =>
   hotReplacementRender(ReactInstance, getReactStack(ReactInstance))
 
-export { flushScheduledUpdates }
+export { flushScheduledUpdates, unscheduleUpdate }
 
 export default reconcileHotReplacement

src/reconciler/proxyAdapter.js

@@ -1,7 +1,10 @@
 import reactHotLoader from '../reactHotLoader'
 import { get as getGeneration } from '../global/generation'
 import { getProxyByType, setStandInOptions } from './proxies'
-import reconcileHotReplacement, { flushScheduledUpdates } from './index'
+import reconcileHotReplacement, {
+  flushScheduledUpdates,
+  unscheduleUpdate,
+} from './index'
 
 export const RENDERED_GENERATION = 'REACT_HOT_LOADER_RENDERED_GENERATION'
 
@@ -28,14 +31,12 @@ function asyncReconciledRender(target) {
   renderReconciler(target, false)
 }
 
-function syncReconciledRender(target) {
-  if (renderReconciler(target, false)) {
-    flushScheduledUpdates()
+export function proxyWrapper(element) {
+  // post wrap on post render
+  if (!reactHotLoader.disableProxyCreation) {
+    unscheduleUpdate(this)
   }
-}
 
-export const proxyWrapper = element => {
-  // post wrap on post render
   if (!element) {
     return element
   }
@@ -55,7 +56,7 @@ export const proxyWrapper = element => {
 }
 
 setStandInOptions({
-  componentWillReceiveProps: syncReconciledRender,
   componentWillRender: asyncReconciledRender,
   componentDidRender: proxyWrapper,
+  componentDidUpdate: flushScheduledUpdates,
 })

test/AppContainer.dev.test.js

@@ -37,6 +37,7 @@ describe(`AppContainer (dev)`, () => {
 
     it('force updates the tree on receiving new children', () => {
       const spy = jest.fn()
+      const spy2 = jest.fn()
 
       class App extends Component {
         shouldComponentUpdate() {
@@ -66,6 +67,7 @@ describe(`AppContainer (dev)`, () => {
 
           render() {
             spy()
+            spy2()
             return <div>ho</div>
           }
         }
@@ -75,7 +77,9 @@ describe(`AppContainer (dev)`, () => {
       }
 
       expect(spy).toHaveBeenCalledTimes(3)
-      expect(wrapper.contains(<div>ho</div>)).toBe(true)
+      expect(spy2).toHaveBeenCalledTimes(2)
+
+      expect(wrapper.update().contains(<div>ho</div>)).toBe(true)
     })
 
     it('force updates the tree on receiving cached children', () => {
@@ -115,7 +119,7 @@ describe(`AppContainer (dev)`, () => {
       }
 
       expect(spy).toHaveBeenCalledTimes(3)
-      expect(wrapper.contains(<div>ho</div>)).toBe(true)
+      expect(wrapper.update().contains(<div>ho</div>)).toBe(true)
     })
 
     it('renders latest children on receiving cached never-rendered children', () => {
@@ -653,7 +657,7 @@ describe(`AppContainer (dev)`, () => {
       }
 
       expect(spy).toHaveBeenCalledTimes(1 + 2)
-      expect(wrapper.contains(<div>ho</div>)).toBe(true)
+      expect(wrapper.update().contains(<div>ho</div>)).toBe(true)
     })
 
     it('force updates the tree on receiving cached children', () => {
@@ -691,7 +695,7 @@ describe(`AppContainer (dev)`, () => {
       }
 
       expect(spy).toHaveBeenCalledTimes(1 + 2)
-      expect(wrapper.contains(<div>ho</div>)).toBe(true)
+      expect(wrapper.update().contains(<div>ho</div>)).toBe(true)
     })
 
     it('renders latest children on receiving cached never-rendered children', () => {
@@ -830,7 +834,7 @@ describe(`AppContainer (dev)`, () => {
       }
 
       expect(spy).toHaveBeenCalledTimes(1 + 2)
-      expect(wrapper.contains(<div>ho</div>)).toBe(true)
+      expect(wrapper.update().contains(<div>ho</div>)).toBe(true)
     })
 
     it('force updates the tree on receiving cached children', () => {
@@ -869,7 +873,7 @@ describe(`AppContainer (dev)`, () => {
       }
 
       expect(spy).toHaveBeenCalledTimes(1 + 2)
-      expect(wrapper.contains(<div>ho</div>)).toBe(true)
+      expect(wrapper.update().contains(<div>ho</div>)).toBe(true)
     })
 
     it('renders latest children on receiving cached never-rendered children', () => {

test/proxy/consistency.test.js

@@ -286,7 +286,7 @@ describe('consistency', () => {
           'render',
           'hotComponentRender',
           'componentDidMount',
-          'componentWillReceiveProps',
+          'componentDidUpdate',
           'componentWillUnmount',
         ])
       })

test/proxy/lifecycle-method.test.js

@@ -50,7 +50,7 @@ describe('lifecycle method', () => {
       }
 
       render() {
-        return <div>{this.superSecret * 2}</div>
+        return <div>!{this.superSecret * 5}</div>
       }
     }
     return { App1, App2 }
@@ -63,7 +63,7 @@ describe('lifecycle method', () => {
     return testFabric(methodName)(Component, patchedRender, spy)
   }
 
-  it('handle componentWillMount', () => {
+  it('handle componentWillMount', done => {
     const spy = jest.fn()
     const { App1, App2 } = getTestClass('componentWillMount', spy)
 
@@ -78,7 +78,11 @@ describe('lifecycle method', () => {
     proxy.update(App2)
     wrapper.instance().forceUpdate()
     expect(spy).toHaveBeenCalledTimes(1)
-    expect(wrapper.text()).toContain('PATCHED + 6')
+    // first render before hot render
+    expect(wrapper.text()).toContain('PATCHED + !10')
+    wrapper.instance().forceUpdate()
+    expect(wrapper.text()).toContain('PATCHED + !15')
+    done()
   })
 
   it('handle componentDidMount', () => {
@@ -96,6 +100,9 @@ describe('lifecycle method', () => {
     proxy.update(App2)
     wrapper.instance().forceUpdate()
     expect(spy).toHaveBeenCalledTimes(1)
-    expect(wrapper.text()).toContain('PATCHED + 6')
+    // first render before hot render
+    expect(wrapper.text()).toContain('PATCHED + !10')
+    wrapper.instance().forceUpdate()
+    expect(wrapper.text()).toContain('PATCHED + !15')
   })
 })

test/reconciler.test.js

@@ -146,7 +146,7 @@ describe('reconciler', () => {
       incrementGeneration()
       wrapper.setProps({ update: 'now' })
 
-      expect(wrapper.find(<third.Component />.type).length).toBe(1)
+      expect(wrapper.update().find(<third.Component />.type).length).toBe(1)
       // first will never be unmounted
       expect(first.unmounted).toHaveBeenCalledTimes(0)
       expect(second.unmounted).toHaveBeenCalledTimes(1)
@@ -231,7 +231,7 @@ describe('reconciler', () => {
 
       incrementGeneration()
       wrapper.setProps({ second: false })
-      expect(First.rendered).toHaveBeenCalledTimes(6)
+      expect(First.rendered).toHaveBeenCalledTimes(5)
       expect(Second.rendered).toHaveBeenCalledTimes(3)
 
       expect(First.unmounted).toHaveBeenCalledTimes(0)
@@ -265,29 +265,29 @@ describe('reconciler', () => {
 
       incrementGeneration()
       wrapper.setProps({ update: 'now' })
-      expect(First.rendered).toHaveBeenCalledTimes(4)
-      expect(Second.rendered).toHaveBeenCalledTimes(4)
+      expect(First.rendered).toHaveBeenCalledTimes(3)
+      expect(Second.rendered).toHaveBeenCalledTimes(3)
 
       incrementGeneration()
       wrapper.setProps({ second: false })
-      expect(First.rendered).toHaveBeenCalledTimes(7)
-      expect(Second.rendered).toHaveBeenCalledTimes(4)
+      expect(First.rendered).toHaveBeenCalledTimes(5)
+      expect(Second.rendered).toHaveBeenCalledTimes(3)
 
       expect(First.unmounted).toHaveBeenCalledTimes(0)
       expect(Second.unmounted).toHaveBeenCalledTimes(1)
     })
 
     it('should handle child mounting', () => {
-      const First = spyComponent(() => <u>1</u>, 'test', '1')
-      const Second = spyComponent(() => <u>2</u>, 'test', '2')
-      const Third = spyComponent(() => <u>2</u>, 'test', '2')
+      const First = spyComponent(() => <u>test1</u>, 'test1', '1')
+      const Second = spyComponent(() => <u>test2</u>, 'test2', '2')
+      const Third = spyComponent(() => <u>test3</u>, 'test3', '3')
       const App = ({ first, second, third }) => (
         <div>
           {first && <First.Component />}
           {second && [
-            <div key="1" />,
+            <div key="1">start</div>,
             <Second.Component key="2" />,
-            <div key="3" />,
+            <div key="3">middle</div>,
             third && <Third.Component key="4" />,
           ]}
         </div>
@@ -304,13 +304,15 @@ describe('reconciler', () => {
 
       incrementGeneration()
       wrapper.setProps({ second: true })
-      expect(First.rendered).toHaveBeenCalledTimes(4) // +3 (reconcile + update + render)
-      expect(Second.rendered).toHaveBeenCalledTimes(2) // (update from first + render)
+      expect(First.rendered).toHaveBeenCalledTimes(3) // +3 (reconcile + update + render)
+      expect(Second.rendered).toHaveBeenCalledTimes(1) // (update from first + render)
 
       wrapper.setProps({ third: true })
-      expect(First.rendered).toHaveBeenCalledTimes(5)
-      expect(Second.rendered).toHaveBeenCalledTimes(3)
+      expect(First.rendered).toHaveBeenCalledTimes(4)
+      expect(Second.rendered).toHaveBeenCalledTimes(2)
       expect(Third.rendered).toHaveBeenCalledTimes(1)
+
+      expect(wrapper.update().html()).toMatch(/test3/)
     })
 
     it('should handle function as a child', () => {

test/reconciler/proxyAdapter.test.js

@@ -1,8 +1,11 @@
 /* eslint-env browser */
 import { proxyWrapper } from '../../src/reconciler/proxyAdapter'
 import * as proxies from '../../src/reconciler/proxies'
+import reactHotLoader from '../../src/reactHotLoader'
+import { unscheduleUpdate } from '../../src/reconciler/hotReplacementRender'
 
 jest.mock('../../src/reconciler/proxies')
+jest.mock('../../src/reconciler/hotReplacementRender')
 
 proxies.getProxyByType.mockReturnValue({ get: () => 'proxy' })
 
@@ -27,4 +30,16 @@ describe(`proxyAdapter`, () => {
       prop: 42,
     })
   })
+
+  it('should remove rendered proxy', () => {
+    const object = {}
+    unscheduleUpdate.mockClear()
+    reactHotLoader.disableProxyCreation = 1
+    proxyWrapper.call(object)
+    expect(unscheduleUpdate).not.toHaveBeenCalled()
+
+    reactHotLoader.disableProxyCreation = 0
+    proxyWrapper.call(object)
+    expect(unscheduleUpdate).toHaveBeenCalledWith(object)
+  })
 })