From 9bb25e1bbdf6115d18e0223b28a29dfd2004e1ae Mon Sep 17 00:00:00 2001 From: hungryc7 Date: Sat, 5 Mar 2016 03:27:05 +0000 Subject: [PATCH] Keep proxies in WeakMaps instead of static fields This prevents issues like https://github.com/reactjs/react-redux/issues/163#issuecomment-192556637 --- packages/react-proxy/package.json | 1 + packages/react-proxy/src/createClassProxy.js | 19 ++++++------- packages/react-proxy/test/consistency.js | 30 ++++++++++++++++++++ 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/packages/react-proxy/package.json b/packages/react-proxy/package.json index 99b8d11..4aa4422 100644 --- a/packages/react-proxy/package.json +++ b/packages/react-proxy/package.json @@ -65,6 +65,7 @@ "webpack": "1.4.8" }, "dependencies": { + "core-js": "^2.1.3", "lodash": "^3.7.0" } } diff --git a/packages/react-proxy/src/createClassProxy.js b/packages/react-proxy/src/createClassProxy.js index 2c0f9b9..719c179 100644 --- a/packages/react-proxy/src/createClassProxy.js +++ b/packages/react-proxy/src/createClassProxy.js @@ -1,3 +1,4 @@ +import WeakMap from 'core-js/library/es6/weak-map'; import createPrototypeProxy from './createPrototypeProxy'; import bindAutoBindMethods from './bindAutoBindMethods'; import deleteUnknownAutoBindMethods from './deleteUnknownAutoBindMethods'; @@ -27,11 +28,13 @@ function isEqualDescriptor(a, b) { return true; } +let allProxies = new WeakMap(); + function proxyClass(InitialComponent) { // Prevent double wrapping. // Given a proxy class, return the existing proxy managing it. - if (Object.prototype.hasOwnProperty.call(InitialComponent, '__reactPatchProxy')) { - return InitialComponent.__reactPatchProxy; + if (allProxies.has(InitialComponent)) { + return allProxies.get(InitialComponent); } let CurrentComponent; @@ -94,8 +97,8 @@ function proxyClass(InitialComponent) { } // Prevent proxy cycles - if (Object.prototype.hasOwnProperty.call(NextComponent, '__reactPatchProxy')) { - return update(NextComponent.__reactPatchProxy.__getCurrent()); + if (allProxies.has(NextComponent)) { + return update(allProxies.get(NextComponent).__getCurrent()); } // Save the next constructor so we call it @@ -173,6 +176,7 @@ function proxyClass(InitialComponent) { update(InitialComponent); const proxy = { get, update }; + allProxies.set(ProxyComponent, proxy); Object.defineProperty(proxy, '__getCurrent', { configurable: false, @@ -181,13 +185,6 @@ function proxyClass(InitialComponent) { value: getCurrent }); - Object.defineProperty(ProxyComponent, '__reactPatchProxy', { - configurable: false, - writable: false, - enumerable: false, - value: proxy - }); - return proxy; } diff --git a/packages/react-proxy/test/consistency.js b/packages/react-proxy/test/consistency.js index af648e8..6b3262f 100644 --- a/packages/react-proxy/test/consistency.js +++ b/packages/react-proxy/test/consistency.js @@ -117,6 +117,36 @@ describe('consistency', () => { expect(proxyTwice).toBe(proxy); }); + /* + * https://github.com/reactjs/react-redux/issues/163#issuecomment-192556637 + */ + it('avoid false positives when statics are hoisted', () => { + const fooProxy = createProxy(Foo); + const FooProxy = fooProxy.get(); + + class Stuff extends Component { + render() {} + } + + const KNOWN_STATICS = { + name: true, + length: true, + prototype: true, + caller: true, + arguments: true, + arity: true, + type: true + }; + Object.getOwnPropertyNames(FooProxy).forEach(key => { + if (!KNOWN_STATICS[key]) { + Stuff[key] = FooProxy[key]; + } + }); + + const stuffProxy = createProxy(Stuff); + expect(stuffProxy).toNotBe(fooProxy); + }); + it('prevents recursive proxy cycle', () => { const proxy = createProxy(Bar); const Proxy = proxy.get();