From f11b187d5abf5423ba75a2b7e796e6520681dd11 Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Thu, 10 May 2018 17:47:20 +1000 Subject: [PATCH] fix: construction sideEffect --- src/proxy/createClassProxy.js | 47 ++++++++++++++------ test/proxy/consistency.test.js | 80 +++++++++++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 15 deletions(-) diff --git a/src/proxy/createClassProxy.js b/src/proxy/createClassProxy.js index 037710967..ce8f6f407 100644 --- a/src/proxy/createClassProxy.js +++ b/src/proxy/createClassProxy.js @@ -108,6 +108,8 @@ function createClassProxy(InitialComponent, proxyKey, options) { let savedDescriptors = {} let injectedMembers = {} let proxyGeneration = 0 + let classUpdatePostponed = null + let instancesCount = 0 let isFunctionalComponent = !isReactClass(InitialComponent) let lastInstance = null @@ -115,11 +117,16 @@ function createClassProxy(InitialComponent, proxyKey, options) { function postConstructionAction() { this[GENERATION] = 0 + lastInstance = this + // is there is an update pending + if (classUpdatePostponed) { + const callUpdate = classUpdatePostponed + classUpdatePostponed = null + callUpdate() + } // As long we can't override constructor // every class shall evolve from a base class inject(this, proxyGeneration, injectedMembers) - - lastInstance = this } function proxiedUpdate() { @@ -162,6 +169,7 @@ function createClassProxy(InitialComponent, proxyKey, options) { 'componentDidMount', target => { target[PROXY_IS_MOUNTED] = true + instancesCount++ }, ) const componentDidUpdate = lifeCycleWrapperFactory( @@ -172,6 +180,7 @@ function createClassProxy(InitialComponent, proxyKey, options) { 'componentWillUnmount', target => { target[PROXY_IS_MOUNTED] = false + instancesCount-- }, ) @@ -308,9 +317,10 @@ function createClassProxy(InitialComponent, proxyKey, options) { return } + isFunctionalComponent = !isReactClass(NextComponent) + proxies.set(NextComponent, proxy) - isFunctionalComponent = !isReactClass(NextComponent) proxyGeneration++ // Save the next constructor so we call it @@ -343,17 +353,26 @@ function createClassProxy(InitialComponent, proxyKey, options) { if (isFunctionalComponent || !ProxyComponent) { // nothing } else { - checkLifeCycleMethods(ProxyComponent, NextComponent) - Object.setPrototypeOf(ProxyComponent.prototype, NextComponent.prototype) - defineProxyMethods(ProxyComponent, NextComponent.prototype) - if (proxyGeneration > 1) { - injectedMembers = mergeComponents( - ProxyComponent, - NextComponent, - InitialComponent, - lastInstance, - injectedMembers, - ) + const classHotReplacement = () => { + checkLifeCycleMethods(ProxyComponent, NextComponent) + Object.setPrototypeOf(ProxyComponent.prototype, NextComponent.prototype) + defineProxyMethods(ProxyComponent, NextComponent.prototype) + if (proxyGeneration > 1) { + injectedMembers = mergeComponents( + ProxyComponent, + NextComponent, + InitialComponent, + lastInstance, + injectedMembers, + ) + } + } + + // Was constructed once + if (instancesCount > 0) { + classHotReplacement() + } else { + classUpdatePostponed = classHotReplacement } } } diff --git a/test/proxy/consistency.test.js b/test/proxy/consistency.test.js index a30cdb863..a7f28e139 100644 --- a/test/proxy/consistency.test.js +++ b/test/proxy/consistency.test.js @@ -250,7 +250,6 @@ describe('consistency', () => { this[key] = eval(code) } } - /* eslint-enable */ const proxy = createProxy(BaseClass) const Proxy = proxy.get() @@ -258,10 +257,14 @@ describe('consistency', () => { expect(instance.render()).toBe(42) proxy.update(Update1Class) + new Proxy() // side effect expect(instance.render()).toBe(43) proxy.update(Update2Class) + new Proxy() // side effect + expect(instance.render()).toBe(42) + /* eslint-enable */ }) it('should reflect external dependencies', () => { @@ -298,6 +301,7 @@ describe('consistency', () => { } } proxy.update(Update1Class) + new Proxy() } /* eslint-enable */ @@ -363,6 +367,80 @@ describe('consistency', () => { mount().instance() expect(Proxy.isStatelessFunctionalProxy).toBe(false) }) + + it('should not update not constructed Proxies', () => { + const spy1 = jest.fn() + const spy2 = jest.fn() + class App extends React.Component { + constructor() { + super() + spy1() + } + render() { + return
+ } + } + + const proxy = createProxy(App) + expect(spy1).not.toHaveBeenCalled() + expect(spy1).not.toHaveBeenCalled() + { + class App extends React.Component { + constructor() { + super() + spy2() + } + render() { + return
+ } + } + proxy.update(App) + + expect(spy1).not.toHaveBeenCalled() + expect(spy1).not.toHaveBeenCalled() + + const Proxy = proxy.get() + mount() + + expect(spy1).toHaveBeenCalled() + expect(spy1).toHaveBeenCalled() + } + }) + + it('should update constructed Proxies', () => { + const spy1 = jest.fn() + const spy2 = jest.fn() + class App extends React.Component { + constructor() { + super() + spy1() + } + render() { + return
+ } + } + + const proxy = createProxy(App) + const Proxy = proxy.get() + mount() + expect(spy1).toHaveBeenCalled() + expect(spy2).not.toHaveBeenCalled() + { + class App extends React.Component { + constructor() { + super() + spy2() + } + render() { + return
+ } + } + proxy.update(App) + + expect(spy1).toHaveBeenCalled() + expect(spy2).toHaveBeenCalled() + } + }) }) describe('modern only', () => {