Skip to content

Commit

Permalink
Keep proxies in WeakMaps instead of static fields
Browse files Browse the repository at this point in the history
This prevents issues like reduxjs/react-redux#163 (comment)
  • Loading branch information
hungry-7 committed Oct 29, 2017
1 parent a9b6191 commit 9bb25e1
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 11 deletions.
1 change: 1 addition & 0 deletions packages/react-proxy/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
"webpack": "1.4.8"
},
"dependencies": {
"core-js": "^2.1.3",
"lodash": "^3.7.0"
}
}
19 changes: 8 additions & 11 deletions packages/react-proxy/src/createClassProxy.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -173,6 +176,7 @@ function proxyClass(InitialComponent) {
update(InitialComponent);

const proxy = { get, update };
allProxies.set(ProxyComponent, proxy);

Object.defineProperty(proxy, '__getCurrent', {
configurable: false,
Expand All @@ -181,13 +185,6 @@ function proxyClass(InitialComponent) {
value: getCurrent
});

Object.defineProperty(ProxyComponent, '__reactPatchProxy', {
configurable: false,
writable: false,
enumerable: false,
value: proxy
});

return proxy;
}

Expand Down
30 changes: 30 additions & 0 deletions packages/react-proxy/test/consistency.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 9bb25e1

Please sign in to comment.