fix: double proxy registration (#915)

Could break all the things in case of code splitting - something "old" could be "registered" by a new name (it is just a variable name), obtain a new proxy and unmount old instances.

Was fixed in v3. We broke it during migration, and had no tests around it.

Fix #912

Author: theKashey

src/proxy/createClassProxy.js

@@ -19,7 +19,11 @@ import { inject, checkLifeCycleMethods, mergeComponents } from './inject'
 
 const has = Object.prototype.hasOwnProperty
 
-const proxies = new WeakMap()
+let proxies = new WeakMap()
+
+export const resetClassProxies = () => {
+  proxies = new WeakMap()
+}
 
 const blackListedClassMembers = [
   'constructor',
@@ -174,6 +178,7 @@ function createClassProxy(InitialComponent, proxyKey, options) {
 
   let ProxyFacade
   let ProxyComponent = null
+  let proxy
 
   if (!isFunctionalComponent) {
     ProxyComponent = proxyClassCreator(InitialComponent, postConstructionAction)
@@ -255,10 +260,11 @@ function createClassProxy(InitialComponent, proxyKey, options) {
     // Prevent proxy cycles
     const existingProxy = proxies.get(NextComponent)
     if (existingProxy) {
-      update(existingProxy[UNWRAP_PROXY]())
       return
     }
 
+    proxies.set(NextComponent, proxy)
+
     isFunctionalComponent = !isReactClass(NextComponent)
     proxyGeneration++
 
@@ -309,7 +315,9 @@ function createClassProxy(InitialComponent, proxyKey, options) {
 
   update(InitialComponent)
 
-  const proxy = { get, update }
+  proxy = { get, update }
+
+  proxies.set(InitialComponent, proxy)
   proxies.set(ProxyFacade, proxy)
 
   safeDefineProperty(proxy, UNWRAP_PROXY, {

src/reconciler/proxies.js

@@ -1,4 +1,5 @@
 import createProxy from '../proxy'
+import { resetClassProxies } from '../proxy/createClassProxy'
 
 let proxiesByID
 let idsByType
@@ -35,6 +36,7 @@ export const createProxyForType = type =>
 export const resetProxies = () => {
   proxiesByID = {}
   idsByType = new WeakMap()
+  resetClassProxies()
 }
 
 resetProxies()

test/AppContainer.dev.test.js

@@ -1202,6 +1202,55 @@ describe(`AppContainer (dev)`, () => {
     expect(wrapper.text()).toBe('TESTnew childnew child')
   })
 
+  it('renders children with chunked re-register', () => {
+    const spy = jest.fn()
+
+    class App extends Component {
+      componentWillUnmount() {
+        spy()
+      }
+
+      render() {
+        return <div>I AM CHILD</div>
+      }
+    }
+
+    RHL.reset()
+    RHL.register(App, 'App1', 'test.js')
+    RHL.register(App, 'App2', 'test.js')
+
+    const wrapper = mount(
+      <AppContainer>
+        <App />
+      </AppContainer>,
+    )
+    expect(spy).not.toHaveBeenCalled()
+    wrapper.setProps({ children: <App /> })
+    expect(spy).not.toHaveBeenCalled()
+    RHL.register(App, 'App3', 'test.js')
+    wrapper.setProps({ children: <App /> })
+    expect(spy).not.toHaveBeenCalled()
+
+    {
+      class App extends Component {
+        componentWillUnmount() {
+          spy()
+        }
+
+        render() {
+          return <div>I AM NEW CHILD</div>
+        }
+      }
+      RHL.register(App, 'App3', 'test.js')
+      wrapper.setProps({ children: <App /> })
+      expect(spy).not.toHaveBeenCalled()
+
+      RHL.register(App, 'App4', 'test.js')
+      wrapper.setProps({ children: <App /> })
+      expect(spy).not.toHaveBeenCalled()
+    }
+  })
+
   describe('with HOC-wrapped root', () => {
     it('renders children', () => {
       const spy = jest.fn()

test/proxy/consistency.test.js

@@ -149,6 +149,12 @@ describe('consistency', () => {
         expect(proxy.get()).toBe(Proxy)
       })
 
+      it('prevents double proxy creation', () => {
+        const proxy1 = createProxy(Bar)
+        const proxy2 = createProxy(Bar)
+        expect(proxy1.get()).toBe(proxy2.get())
+      })
+
       it('prevents mutually recursive proxy cycle', () => {
         const barProxy = createProxy(Bar)
         const BarProxy = barProxy.get()

test/proxy/instance-property.test.js

@@ -3,7 +3,7 @@ import React from 'react'
 import { ensureNoWarnings, createMounter } from './helper'
 import createProxy from '../../src/proxy'
 
-const fixtures = {
+const fixtures = () => ({
   modern: {
     InstanceProperty: class InstanceProperty extends React.Component {
       answer = 42
@@ -79,7 +79,7 @@ const fixtures = {
       }
     },
   },
-}
+})
 
 describe('instance property', () => {
   ensureNoWarnings()
@@ -91,7 +91,7 @@ describe('instance property', () => {
         InstanceProperty,
         InstancePropertyUpdate,
         InstancePropertyRemoval,
-      } = fixtures[type]
+      } = fixtures()[type]
 
       it('is available on proxy class instance', () => {
         const proxy = createProxy(InstanceProperty)
@@ -158,7 +158,7 @@ describe('instance property', () => {
     // })
 
     it('show use the underlayer top value', () => {
-      const proxy = createProxy(fixtures.modern.InstancePropertyFromContext)
+      const proxy = createProxy(fixtures().modern.InstancePropertyFromContext)
       const Proxy = proxy.get()
       const wrapper = mount(<Proxy />)
       expect(wrapper.text()).toBe('42')

test/proxy/static-method.test.js

@@ -2,8 +2,9 @@
 import React from 'react'
 import { ensureNoWarnings, createMounter } from './helper'
 import createProxy from '../../src/proxy'
+import reactHotLoader from '../../src/reactHotLoader'
 
-const fixtures = {
+const fixtures = () => ({
   modern: {
     StaticMethod: class StaticMethod extends React.Component {
       static getAnswer() {
@@ -31,19 +32,21 @@ const fixtures = {
       }
     },
   },
-}
+})
 
 describe('static method', () => {
   ensureNoWarnings()
   const { mount } = createMounter()
 
-  Object.keys(fixtures).forEach(type => {
+  Object.keys(fixtures()).forEach(type => {
     describe(type, () => {
       const {
         StaticMethod,
         StaticMethodUpdate,
         StaticMethodRemoval,
-      } = fixtures[type]
+      } = fixtures()[type]
+
+      beforeEach(() => reactHotLoader.reset())
 
       it('is available on proxy class instance', () => {
         const proxy = createProxy(StaticMethod)

test/proxy/static-property.test.js

@@ -4,8 +4,9 @@ import React from 'react'
 import PropTypes from 'prop-types'
 import { ensureNoWarnings, createMounter } from './helper'
 import createProxy from '../../src/proxy'
+import reactHotLoader from '../../src/reactHotLoader'
 
-const fixtures = {
+const fixtures = () => ({
   modern: {
     StaticProperty: class StaticProperty extends React.Component {
       static answer = 42
@@ -65,21 +66,23 @@ const fixtures = {
       }
     },
   },
-}
+})
 
 describe('static property', () => {
   ensureNoWarnings()
   const { mount } = createMounter()
 
-  Object.keys(fixtures).forEach(type => {
+  Object.keys(fixtures()).forEach(type => {
     describe(type, () => {
       const {
         StaticProperty,
         StaticPropertyUpdate,
         StaticPropertyRemoval,
         WithPropTypes,
         WithPropTypesUpdate,
-      } = fixtures[type]
+      } = fixtures()[type]
+
+      beforeEach(() => reactHotLoader.reset())
 
       it('is available on proxy class instance', () => {
         const proxy = createProxy(StaticProperty)