From 0a86704911f4bb68a4dc909edb0f90accdcc376a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sun, 17 Apr 2016 14:59:29 +0100 Subject: [PATCH 1/6] Rewrite with createElement()-type wrapping in mind --- README.md | 2 +- src/createClassProxy.js | 78 +++-- test/consistency.js | 385 +++++++++++----------- test/inheritance.js | 60 ++-- test/instance-descriptor.js | 311 +++++++++--------- test/instance-method-autobinding.js | 336 ++++++++++--------- test/instance-method.js | 492 +++++++++++++++------------- test/instance-property.js | 226 +++++++------ test/pure-component.js | 70 ++-- test/static-descriptor.js | 309 ++++++++--------- test/static-method.js | 298 +++++++++-------- test/static-property.js | 450 ++++++++++++++----------- test/unmounting.js | 246 +++++++------- 13 files changed, 1776 insertions(+), 1487 deletions(-) diff --git a/README.md b/README.md index 609a9f5..2e1f101 100644 --- a/README.md +++ b/README.md @@ -83,11 +83,11 @@ deepForceUpdate(rootInstance); * Replaces static getters and setters * Replaces unbound static methods * Replaces static properties unless they were overwritten by code +* Sets up `this.constructor` to match the most recent class ## Known Limitations * Does not replace ES7 instance properties -* Does not replace bound static methods * Replacing a method using [`autobind-decorator`](https://github.com/andreypopp/autobind-decorator) causes its identity to change ## Contributing diff --git a/src/createClassProxy.js b/src/createClassProxy.js index 2c46cc0..fbc63ed 100644 --- a/src/createClassProxy.js +++ b/src/createClassProxy.js @@ -6,6 +6,7 @@ import supportsProtoAssignment from './supportsProtoAssignment'; const RESERVED_STATICS = [ 'length', + 'displayName', 'name', 'arguments', 'caller', @@ -49,13 +50,7 @@ function proxyClass(InitialComponent) { let CurrentComponent; let ProxyComponent; - - let staticDescriptors = {}; - function wasStaticModifiedByUser(key) { - // Compare the descriptor with the one we previously set ourselves. - const currentDescriptor = Object.getOwnPropertyDescriptor(ProxyComponent, key); - return !isEqualDescriptor(staticDescriptors[key], currentDescriptor); - } + let savedDescriptors = {}; function instantiate(factory, context, params) { const component = factory(); @@ -105,6 +100,9 @@ function proxyClass(InitialComponent) { if (typeof NextComponent !== 'function') { throw new Error('Expected a constructor.'); } + if (NextComponent === CurrentComponent) { + return; + } // Prevent proxy cycles var existingProxy = findProxy(NextComponent); @@ -113,6 +111,7 @@ function proxyClass(InitialComponent) { } // Save the next constructor so we call it + const PreviousComponent = CurrentComponent; CurrentComponent = NextComponent; // Try to infer displayName @@ -121,46 +120,75 @@ function proxyClass(InitialComponent) { // Set up the same prototype for inherited statics ProxyComponent.__proto__ = NextComponent.__proto__; - // Copy static methods and properties + // Copy over static methods and properties added at runtime + if (PreviousComponent) { + Object.getOwnPropertyNames(PreviousComponent).forEach(key => { + if (RESERVED_STATICS.indexOf(key) > -1) { + return; + } + + const prevDescriptor = Object.getOwnPropertyDescriptor(PreviousComponent, key); + const savedDescriptor = savedDescriptors[key]; + + if (!isEqualDescriptor(prevDescriptor, savedDescriptor)) { + Object.defineProperty(NextComponent, key, prevDescriptor); + } + }); + } + + // Copy newly defined static methods and properties Object.getOwnPropertyNames(NextComponent).forEach(key => { if (RESERVED_STATICS.indexOf(key) > -1) { return; } - const staticDescriptor = { + const prevDescriptor = PreviousComponent && Object.getOwnPropertyDescriptor(PreviousComponent, key); + const savedDescriptor = savedDescriptors[key]; + + // Skip redefined descriptors + if (prevDescriptor && savedDescriptor && !isEqualDescriptor(savedDescriptor, prevDescriptor)) { + Object.defineProperty(NextComponent, key, prevDescriptor); + Object.defineProperty(ProxyComponent, key, prevDescriptor); + return; + } + + if (prevDescriptor && !savedDescriptor) { + Object.defineProperty(ProxyComponent, key, prevDescriptor); + return; + } + + const nextDescriptor = { ...Object.getOwnPropertyDescriptor(NextComponent, key), configurable: true }; - - // Copy static unless user has redefined it at runtime - if (!wasStaticModifiedByUser(key)) { - Object.defineProperty(ProxyComponent, key, staticDescriptor); - staticDescriptors[key] = staticDescriptor; - } + savedDescriptors[key] = nextDescriptor; + Object.defineProperty(ProxyComponent, key, nextDescriptor); }); - // Remove old static methods and properties + // Remove static methods and properties that are no longer defined Object.getOwnPropertyNames(ProxyComponent).forEach(key => { if (RESERVED_STATICS.indexOf(key) > -1) { return; } - // Skip statics that exist on the next class if (NextComponent.hasOwnProperty(key)) { return; } - // Skip non-configurable statics - const descriptor = Object.getOwnPropertyDescriptor(ProxyComponent, key); - if (descriptor && !descriptor.configurable) { + const proxyDescriptor = Object.getOwnPropertyDescriptor(ProxyComponent, key); + if (proxyDescriptor && !proxyDescriptor.configurable) { return; } - // Delete static unless user has redefined it at runtime - if (!wasStaticModifiedByUser(key)) { - delete ProxyComponent[key]; - delete staticDescriptors[key]; + const prevDescriptor = PreviousComponent && Object.getOwnPropertyDescriptor(PreviousComponent, key); + const savedDescriptor = savedDescriptors[key]; + + // Skip redefined descriptors + if (prevDescriptor && savedDescriptor && !isEqualDescriptor(savedDescriptor, prevDescriptor)) { + return; } + + delete ProxyComponent[key]; }); if (prototypeProxy) { @@ -168,7 +196,7 @@ function proxyClass(InitialComponent) { const mountedInstances = prototypeProxy.update(NextComponent.prototype); // Set up the constructor property so accessing the statics work - ProxyComponent.prototype.constructor = ProxyComponent; + ProxyComponent.prototype.constructor = NextComponent; // We might have added new methods that need to be auto-bound mountedInstances.forEach(bindAutoBindMethods); diff --git a/test/consistency.js b/test/consistency.js index 6b3262f..f753d74 100644 --- a/test/consistency.js +++ b/test/consistency.js @@ -3,81 +3,83 @@ import createShallowRenderer from './helpers/createShallowRenderer'; import expect from 'expect'; import createProxy from '../src'; -const fixtures = { - modern: { - Bar: class Bar extends Component { - componentWillUnmount() { - this.didUnmount = true; - } +function createModernFixtures() { + class Bar extends Component { + componentWillUnmount() { + this.didUnmount = true; + } - doNothing() { - } + doNothing() { + } - render() { - return
Bar
; - } - }, + render() { + return
Bar
; + } + } - Baz: class Baz extends Component { - componentWillUnmount() { - this.didUnmount = true; - } + class Baz extends Component { + componentWillUnmount() { + this.didUnmount = true; + } - render() { - return
Baz
; - } - }, + render() { + return
Baz
; + } + } - Foo: class Foo extends Component { - static displayName = 'Foo (Custom)'; + class Foo extends Component { + static displayName = 'Foo (Custom)'; - componentWillUnmount() { - this.didUnmount = true; - } + componentWillUnmount() { + this.didUnmount = true; + } - render() { - return
Foo
; - } + render() { + return
Foo
; } - }, + } - classic: { - Bar: React.createClass({ - componentWillUnmount() { - this.didUnmount = true; - }, + return { Bar, Baz, Foo }; +} - doNothing() { - }, +function createClassicFixtures() { + const Bar = React.createClass({ + componentWillUnmount() { + this.didUnmount = true; + }, - render() { - return
Bar
; - } - }), + doNothing() { + }, - Baz: React.createClass({ - componentWillUnmount() { - this.didUnmount = true; - }, + render() { + return
Bar
; + } + }); - render() { - return
Baz
; - } - }), + const Baz = React.createClass({ + componentWillUnmount() { + this.didUnmount = true; + }, - Foo: React.createClass({ - displayName: 'Foo (Custom)', + render() { + return
Baz
; + } + }); - componentWillUnmount() { - this.didUnmount = true; - }, + const Foo = React.createClass({ + displayName: 'Foo (Custom)', - render() { - return
Foo
; - } - }) - } -}; + componentWillUnmount() { + this.didUnmount = true; + }, + + render() { + return
Foo
; + } + }); + + return { Bar, Baz, Foo }; +} describe('consistency', () => { let renderer; @@ -93,155 +95,166 @@ describe('consistency', () => { expect(warnSpy.calls.length).toBe(0); }); - Object.keys(fixtures).forEach(type => { - describe(type, () => { - const { Bar, Baz, Foo } = fixtures[type]; - - it('does not overwrite the original class', () => { - const proxy = createProxy(Bar); - const Proxy = proxy.get(); - const barInstance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Bar'); - - proxy.update(Baz); - const realBarInstance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Bar'); - expect(barInstance).toNotEqual(realBarInstance); - expect(barInstance.didUnmount).toEqual(true); - }); + function runCommonTests(createFixtures) { + let Bar, Baz, Foo; + beforeEach(() => { + ({ Foo, Bar, Baz } = createFixtures()); + }); - it('returns an existing proxy when wrapped twice', () => { - const proxy = createProxy(Bar); - const Proxy = proxy.get(); - const proxyTwice = createProxy(Proxy); - expect(proxyTwice).toBe(proxy); - }); + it('does not overwrite the original class', () => { + const proxy = createProxy(Bar); + const Proxy = proxy.get(); + const barInstance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Bar'); - /* - * 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(); + proxy.update(Baz); + const realBarInstance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Bar'); + expect(barInstance).toNotEqual(realBarInstance); + expect(barInstance.didUnmount).toEqual(true); + }); - class Stuff extends Component { - render() {} - } + it('returns an existing proxy when wrapped twice', () => { + const proxy = createProxy(Bar); + const Proxy = proxy.get(); + const proxyTwice = createProxy(Proxy); + expect(proxyTwice).toBe(proxy); + }); - 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); - }); + /* + * 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(); - it('prevents recursive proxy cycle', () => { - const proxy = createProxy(Bar); - const Proxy = proxy.get(); - proxy.update(Proxy); - expect(proxy.get()).toEqual(Proxy); + 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]; + } }); - it('prevents mutually recursive proxy cycle', () => { - const barProxy = createProxy(Bar); - const BarProxy = barProxy.get(); + const stuffProxy = createProxy(Stuff); + expect(stuffProxy).toNotBe(fooProxy); + }); - const fooProxy = createProxy(Foo); - const FooProxy = fooProxy.get(); + it('prevents recursive proxy cycle', () => { + const proxy = createProxy(Bar); + const Proxy = proxy.get(); + proxy.update(Proxy); + expect(proxy.get()).toEqual(Proxy); + }); - barProxy.update(FooProxy); - fooProxy.update(BarProxy); - }); + it('prevents mutually recursive proxy cycle', () => { + const barProxy = createProxy(Bar); + const BarProxy = barProxy.get(); - it('sets up constructor to match the type', () => { - let proxy = createProxy(Bar); - const BarProxy = proxy.get(); - const barInstance = renderer.render(); - expect(barInstance.constructor).toEqual(BarProxy); - expect(barInstance instanceof BarProxy).toEqual(true); - - proxy.update(Baz); - const BazProxy = proxy.get(); - expect(BarProxy).toEqual(BazProxy); - expect(barInstance.constructor).toEqual(BazProxy); - expect(barInstance instanceof BazProxy).toEqual(true); - }); + const fooProxy = createProxy(Foo); + const FooProxy = fooProxy.get(); - it('sets up displayName from displayName or name', () => { - let proxy = createProxy(Bar); - const Proxy = proxy.get(); - const barInstance = renderer.render(); - expect(barInstance.constructor.displayName).toEqual('Bar'); + barProxy.update(FooProxy); + fooProxy.update(BarProxy); + }); - proxy.update(Baz); - expect(barInstance.constructor.displayName).toEqual('Baz'); + it('sets up constructor to match the most recent type', () => { + let proxy = createProxy(Bar); + const BarProxy = proxy.get(); + const barInstance = renderer.render(); + expect(barInstance.constructor).toEqual(Bar); + expect(barInstance instanceof BarProxy).toEqual(true); + expect(barInstance instanceof Bar).toEqual(true); - proxy.update(Foo); - expect(barInstance.constructor.displayName).toEqual('Foo (Custom)'); - }); + proxy.update(Baz); + const BazProxy = proxy.get(); + expect(BarProxy).toEqual(BazProxy); + expect(barInstance.constructor).toEqual(Baz); + expect(barInstance instanceof BazProxy).toEqual(true); + expect(barInstance instanceof Baz).toEqual(true); + }); + + it('sets up displayName from displayName or name', () => { + let proxy = createProxy(Bar); + const Proxy = proxy.get(); + expect(Proxy.displayName).toEqual('Bar'); + + proxy.update(Baz); + expect(Proxy.displayName).toEqual('Baz'); + + proxy.update(Foo); + expect(Proxy.displayName).toEqual('Foo (Custom)'); + }); + + it('keeps own methods on the prototype', () => { + let proxy = createProxy(Bar); + const Proxy = proxy.get(); + + const propertyNames = Object.getOwnPropertyNames(Proxy.prototype); + expect(propertyNames).toInclude('doNothing'); + }); + + it('preserves enumerability and writability of methods', () => { + let proxy = createProxy(Bar); + const Proxy = proxy.get(); - it('keeps own methods on the prototype', () => { - let proxy = createProxy(Bar); - const Proxy = proxy.get(); + ['doNothing', 'render', 'componentDidMount', 'componentWillUnmount'].forEach(name => { + const originalDescriptor = Object.getOwnPropertyDescriptor(Bar.prototype, name); + const proxyDescriptor = Object.getOwnPropertyDescriptor(Proxy.prototype, name); - const propertyNames = Object.getOwnPropertyNames(Proxy.prototype); - expect(propertyNames).toInclude('doNothing'); + if (originalDescriptor) { + expect(proxyDescriptor.enumerable).toEqual(originalDescriptor.enumerable, name); + expect(proxyDescriptor.writable).toEqual(originalDescriptor.writable, name); + } else { + expect(proxyDescriptor.enumerable).toEqual(false, name); + expect(proxyDescriptor.writable).toEqual(true, name); + } }); + }); - it('preserves enumerability and writability of methods', () => { - let proxy = createProxy(Bar); - const Proxy = proxy.get(); - - ['doNothing', 'render', 'componentDidMount', 'componentWillUnmount'].forEach(name => { - const originalDescriptor = Object.getOwnPropertyDescriptor(Bar.prototype, name); - const proxyDescriptor = Object.getOwnPropertyDescriptor(Proxy.prototype, name); - - if (originalDescriptor) { - expect(proxyDescriptor.enumerable).toEqual(originalDescriptor.enumerable, name); - expect(proxyDescriptor.writable).toEqual(originalDescriptor.writable, name); - } else { - expect(proxyDescriptor.enumerable).toEqual(false, name); - expect(proxyDescriptor.writable).toEqual(true, name); - } - }); + it('preserves toString() of methods', () => { + let proxy = createProxy(Bar); + + const Proxy = proxy.get(); + ['doNothing', 'render', 'componentWillUnmount', 'constructor'].forEach(name => { + const originalMethod = Bar.prototype[name]; + const proxyMethod = Proxy.prototype[name]; + expect(originalMethod.toString()).toEqual(proxyMethod.toString()); }); - it('preserves toString() of methods', () => { - let proxy = createProxy(Bar); - - const Proxy = proxy.get(); - ['doNothing', 'render', 'componentWillUnmount', 'constructor'].forEach(name => { - const originalMethod = Bar.prototype[name]; - const proxyMethod = Proxy.prototype[name]; - expect(originalMethod.toString()).toEqual(proxyMethod.toString()); - }); - - const doNothingBeforeItWasDeleted = Proxy.prototype.doNothing; - proxy.update(Baz); - ['render', 'componentWillUnmount', 'constructor'].forEach(name => { - const originalMethod = Baz.prototype[name]; - const proxyMethod = Proxy.prototype[name]; - expect(originalMethod.toString()).toEqual(proxyMethod.toString()); - }); - expect(doNothingBeforeItWasDeleted.toString()).toEqual(''); + const doNothingBeforeItWasDeleted = Proxy.prototype.doNothing; + proxy.update(Baz); + ['render', 'componentWillUnmount', 'constructor'].forEach(name => { + const originalMethod = Baz.prototype[name]; + const proxyMethod = Proxy.prototype[name]; + expect(originalMethod.toString()).toEqual(proxyMethod.toString()); }); + expect(doNothingBeforeItWasDeleted.toString()).toEqual(''); }); + } + + describe('classic', () => { + runCommonTests(createClassicFixtures); }); - describe('modern only', () => { - const { Bar, Baz } = fixtures.modern; + describe('modern', () => { + runCommonTests(createModernFixtures); + + let Bar, Baz, Foo; + beforeEach(() => { + ({ Bar, Baz, Foo } = createModernFixtures()); + }) it('sets up the constructor name from initial name', () => { let proxy = createProxy(Bar); @@ -269,7 +282,7 @@ describe('consistency', () => { const proxy = createProxy(Bar); const Proxy = proxy.get(); const barInstance = renderer.render(); - expect(barInstance.constructor).toEqual(Proxy); + expect(barInstance.constructor).toEqual(Bar); }).toNotThrow(); } finally { global.Function = oldFunction; diff --git a/test/inheritance.js b/test/inheritance.js index 16ed8b4..c82a0b5 100644 --- a/test/inheritance.js +++ b/test/inheritance.js @@ -3,41 +3,47 @@ import createShallowRenderer from './helpers/createShallowRenderer'; import expect from 'expect'; import createProxy from '../src'; -class Base1 extends Component { - static getY() { - return 42; +function createModernFixtures() { + class Base1 extends Component { + static getY() { + return 42; + } + + getX() { + return 42; + } + + render() { + return this.actuallyRender(); + } } - getX() { - return 42; - } + class Base2 extends Component { + static getY() { + return 43; + } - render() { - return this.actuallyRender(); - } -} - -class Base2 extends Component { - static getY() { - return 43; - } + getX() { + return 43; + } - getX() { - return 43; + render() { + return this.actuallyRender(); + } } - render() { - return this.actuallyRender(); - } + return { Base1, Base2 }; } describe('inheritance', () => { let renderer; let warnSpy; + let Base1, Base2; beforeEach(() => { renderer = createShallowRenderer(); warnSpy = expect.spyOn(console, 'error').andCallThrough(); + ({ Base1, Base2 } = createModernFixtures()); }); afterEach(() => { @@ -45,7 +51,7 @@ describe('inheritance', () => { expect(warnSpy.calls.length).toBe(0); }); - describe('modern only', () => { + describe('modern', () => { it('replaces a base instance method with proxied base and derived', () => { const baseProxy = createProxy(Base1); const BaseProxy = baseProxy.get(); @@ -336,18 +342,6 @@ describe('inheritance', () => { derivedProxy.update(Derived2); instance.forceUpdate(); expect(renderer.getRenderOutput().props.children).toEqual('4300 nice'); - - derivedProxy.update(Derived1); - instance.forceUpdate(); - expect(renderer.getRenderOutput().props.children).toEqual('4300 lol'); - - middleProxy.update(Middle1); - instance.forceUpdate(); - expect(renderer.getRenderOutput().props.children).toEqual('430 lol'); - - baseProxy.update(Base1); - instance.forceUpdate(); - expect(renderer.getRenderOutput().props.children).toEqual('420 lol'); }); }); }); diff --git a/test/instance-descriptor.js b/test/instance-descriptor.js index 83fb5ac..dfceabc 100644 --- a/test/instance-descriptor.js +++ b/test/instance-descriptor.js @@ -3,61 +3,78 @@ import createShallowRenderer from './helpers/createShallowRenderer'; import expect from 'expect'; import createProxy from '../src'; -const fixtures = { - modern: { - InstanceDescriptor: class InstanceDescriptor extends Component { - get answer() { - return this.props.base + 42; - } - - set something(value) { - this._something = value * 2; - } - - render() { - return
{this.answer}
; - } - }, - - InstanceDescriptorUpdate: class InstanceDescriptorUpdate extends Component { - get answer() { - return this.props.base + 43; - } - - set something(value) { - this._something = value * 3; - } - - render() { - return
{this.answer}
; - } - }, - - InstanceDescriptorRemoval: class InstanceDescriptorRemoval extends Component { - render() { - return
{this.answer}
; - } - }, - - ThrowingAccessors: class ThrowingAccessors extends Component { - get something() { - throw new Error(); - } - - set something(value) { - throw new Error(); - } +function createModernFixtures() { + class InstanceDescriptor extends Component { + get answer() { + return this.props.base + 42; + } + + set something(value) { + this._something = value * 2; + } + + render() { + return
{this.answer}
; + } + } + + class InstanceDescriptorUpdate extends Component { + get answer() { + return this.props.base + 43; + } + + set something(value) { + this._something = value * 3; + } + + render() { + return
{this.answer}
; + } + } + + class InstanceDescriptorRemoval extends Component { + render() { + return
{this.answer}
; } } -}; + + class ThrowingAccessors extends Component { + get something() { + throw new Error(); + } + + set something(value) { + throw new Error(); + } + } + + return { + InstanceDescriptor, + InstanceDescriptorUpdate, + InstanceDescriptorRemoval, + ThrowingAccessors + }; +} describe('instance descriptor', () => { let renderer; let warnSpy; + let InstanceDescriptor; + let InstanceDescriptorUpdate; + let InstanceDescriptorRemoval; + let ThrowingAccessors; + beforeEach(() => { renderer = createShallowRenderer(); warnSpy = expect.spyOn(console, 'error').andCallThrough(); + + ({ + InstanceDescriptor, + InstanceDescriptorUpdate, + InstanceDescriptorRemoval, + ThrowingAccessors + } = createModernFixtures()); }); afterEach(() => { @@ -65,128 +82,124 @@ describe('instance descriptor', () => { expect(warnSpy.calls.length).toBe(0); }); - Object.keys(fixtures).forEach(type => { - const { InstanceDescriptor, InstanceDescriptorUpdate, InstanceDescriptorRemoval, ThrowingAccessors } = fixtures[type]; + describe('modern', () => { + it('does not invoke accessors', () => { + const proxy = createProxy(InstanceDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(() => proxy.update(ThrowingAccessors)).toNotThrow(); + }); - describe(type, () => { - it('does not invoke accessors', () => { + describe('getter', () => { + it('is available on proxy class instance', () => { const proxy = createProxy(InstanceDescriptor); const Proxy = proxy.get(); - const instance = renderer.render(); - expect(() => proxy.update(ThrowingAccessors)).toNotThrow(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(142); + expect(instance.answer).toEqual(142); }); - describe('getter', () => { - it('is available on proxy class instance', () => { - const proxy = createProxy(InstanceDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(142); - expect(instance.answer).toEqual(142); - }); + it('gets added', () => { + const proxy = createProxy(InstanceDescriptorRemoval); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(undefined); + + proxy.update(InstanceDescriptor); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(142); + expect(instance.answer).toEqual(142); + }); - it('gets added', () => { - const proxy = createProxy(InstanceDescriptorRemoval); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(undefined); + it('gets replaced', () => { + const proxy = createProxy(InstanceDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(142); + + proxy.update(InstanceDescriptorUpdate); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(143); + expect(instance.answer).toEqual(143); + + proxy.update(InstanceDescriptorRemoval); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(undefined); + expect(instance.answer).toEqual(undefined); + }); - proxy.update(InstanceDescriptor); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(142); - expect(instance.answer).toEqual(142); - }); + it('gets redefined', () => { + const proxy = createProxy(InstanceDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(142); - it('gets replaced', () => { - const proxy = createProxy(InstanceDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(142); - - proxy.update(InstanceDescriptorUpdate); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(143); - expect(instance.answer).toEqual(143); - - proxy.update(InstanceDescriptorRemoval); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(undefined); - expect(instance.answer).toEqual(undefined); + Object.defineProperty(instance, 'answer', { + value: 7 }); - it('gets redefined', () => { - const proxy = createProxy(InstanceDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(142); - - Object.defineProperty(instance, 'answer', { - value: 7 - }); - - proxy.update(InstanceDescriptorUpdate); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(7); - expect(instance.answer).toEqual(7); - - proxy.update(InstanceDescriptorRemoval); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(7); - expect(instance.answer).toEqual(7); - }); - }); + proxy.update(InstanceDescriptorUpdate); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(7); + expect(instance.answer).toEqual(7); - describe('setter', () => { - it('is available on proxy class instance', () => { - const proxy = createProxy(InstanceDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - instance.something = 10; - expect(instance._something).toEqual(20); - }); + proxy.update(InstanceDescriptorRemoval); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(7); + expect(instance.answer).toEqual(7); + }); + }); - it('gets added', () => { - const proxy = createProxy(InstanceDescriptorRemoval); - const Proxy = proxy.get(); - const instance = renderer.render(); + describe('setter', () => { + it('is available on proxy class instance', () => { + const proxy = createProxy(InstanceDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + instance.something = 10; + expect(instance._something).toEqual(20); + }); - proxy.update(InstanceDescriptor); - instance.something = 10; - expect(instance._something).toEqual(20); - }); + it('gets added', () => { + const proxy = createProxy(InstanceDescriptorRemoval); + const Proxy = proxy.get(); + const instance = renderer.render(); - it('gets replaced', () => { - const proxy = createProxy(InstanceDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - instance.something = 10; - expect(instance._something).toEqual(20); - - proxy.update(InstanceDescriptorUpdate); - expect(instance._something).toEqual(20); - instance.something = 10; - expect(instance._something).toEqual(30); - - proxy.update(InstanceDescriptorRemoval); - expect(instance._something).toEqual(30); - instance.something = 7; - expect(instance.something).toEqual(7); - expect(instance._something).toEqual(30); - }); + proxy.update(InstanceDescriptor); + instance.something = 10; + expect(instance._something).toEqual(20); + }); - it('gets redefined', () => { - const proxy = createProxy(InstanceDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(142); + it('gets replaced', () => { + const proxy = createProxy(InstanceDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + instance.something = 10; + expect(instance._something).toEqual(20); + + proxy.update(InstanceDescriptorUpdate); + expect(instance._something).toEqual(20); + instance.something = 10; + expect(instance._something).toEqual(30); + + proxy.update(InstanceDescriptorRemoval); + expect(instance._something).toEqual(30); + instance.something = 7; + expect(instance.something).toEqual(7); + expect(instance._something).toEqual(30); + }); - Object.defineProperty(instance, 'something', { - value: 50 - }); + it('gets redefined', () => { + const proxy = createProxy(InstanceDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(142); - proxy.update(InstanceDescriptorUpdate); - expect(instance.something).toEqual(50); + Object.defineProperty(instance, 'something', { + value: 50 }); + + proxy.update(InstanceDescriptorUpdate); + expect(instance.something).toEqual(50); }); }); }); diff --git a/test/instance-method-autobinding.js b/test/instance-method-autobinding.js index 963e871..31cafa5 100644 --- a/test/instance-method-autobinding.js +++ b/test/instance-method-autobinding.js @@ -4,134 +4,146 @@ import autobind from './helpers/autobind'; import expect from 'expect'; import createProxy from '../src'; -const fixtures = { - classic: { - Counter1x: React.createClass({ - getInitialState() { - return { counter: 0 }; - }, - - increment() { - this.setState({ - counter: this.state.counter + 1 - }); - }, - - render() { - return {this.state.counter}; - } - }), - - Counter10x: React.createClass({ - getInitialState() { - return { counter: 0 }; - }, - - increment() { - this.setState({ - counter: this.state.counter + 10 - }); - }, - - render() { - return {this.state.counter}; - } - }), - - Counter100x: React.createClass({ - getInitialState() { - return { counter: 0 }; - }, - - increment() { - this.setState({ - counter: this.state.counter + 100 - }); - }, - - render() { - return {this.state.counter}; - } - }), - - CounterWithoutIncrementMethod: React.createClass({ - getInitialState() { - return { counter: 0 }; - }, - - render() { - return {this.state.counter}; - } - }) - }, - - modern: { - Counter1x: class Counter1x extends Component { - constructor(props) { - super(props); - this.state = { counter: 0 }; - } - - @autobind - increment() { - this.setState({ - counter: this.state.counter + 1 - }); - } - - render() { - return {this.state.counter}; - } +function createModernFixtures() { + class Counter1x extends Component { + constructor(props) { + super(props); + this.state = { counter: 0 }; + } + + @autobind + increment() { + this.setState({ + counter: this.state.counter + 1 + }); + } + + render() { + return {this.state.counter}; + } + } + + class Counter10x extends Component { + constructor(props) { + super(props); + this.state = { counter: 0 }; + } + + @autobind + increment() { + this.setState({ + counter: this.state.counter + 10 + }); + } + + render() { + return {this.state.counter}; + } + } + + class Counter100x extends Component { + constructor(props) { + super(props); + this.state = { counter: 0 }; + } + + @autobind + increment() { + this.setState({ + counter: this.state.counter + 100 + }); + } + + render() { + return {this.state.counter}; + } + } + + class CounterWithoutIncrementMethod extends Component { + constructor(props) { + super(props); + this.state = { counter: 0 }; + } + + render() { + return {this.state.counter}; + } + } + + return { + Counter1x, + Counter10x, + Counter100x, + CounterWithoutIncrementMethod + }; +} + +function createClassicFixtures() { + const Counter1x = React.createClass({ + getInitialState() { + return { counter: 0 }; + }, + + increment() { + this.setState({ + counter: this.state.counter + 1 + }); + }, + + render() { + return {this.state.counter}; + } + }); + + const Counter10x = React.createClass({ + getInitialState() { + return { counter: 0 }; }, - Counter10x: class Counter10x extends Component { - constructor(props) { - super(props); - this.state = { counter: 0 }; - } - - @autobind - increment() { - this.setState({ - counter: this.state.counter + 10 - }); - } - - render() { - return {this.state.counter}; - } + increment() { + this.setState({ + counter: this.state.counter + 10 + }); }, - Counter100x: class Counter100x extends Component { - constructor(props) { - super(props); - this.state = { counter: 0 }; - } - - @autobind - increment() { - this.setState({ - counter: this.state.counter + 100 - }); - } - - render() { - return {this.state.counter}; - } + render() { + return {this.state.counter}; + } + }); + + const Counter100x = React.createClass({ + getInitialState() { + return { counter: 0 }; }, - CounterWithoutIncrementMethod: class CounterWithoutIncrementMethod extends Component { - constructor(props) { - super(props); - this.state = { counter: 0 }; - } + increment() { + this.setState({ + counter: this.state.counter + 100 + }); + }, - render() { - return {this.state.counter}; - } + render() { + return {this.state.counter}; } - } -}; + }); + + const CounterWithoutIncrementMethod = React.createClass({ + getInitialState() { + return { counter: 0 }; + }, + + render() { + return {this.state.counter}; + } + }); + + return { + Counter1x, + Counter10x, + Counter100x, + CounterWithoutIncrementMethod + }; +} describe('autobound instance method', () => { let renderer; @@ -147,50 +159,61 @@ describe('autobound instance method', () => { expect(warnSpy.calls.length).toBe(0); }); - Object.keys(fixtures).forEach(type => { - describe(type, () => { - const { Counter1x, Counter10x, Counter100x, CounterWithoutIncrementMethod } = fixtures[type]; + function runCommonTests(createFixtures) { + let Counter1x; + let Counter10x; + let Counter100x; + let CounterWithoutIncrementMethod; + + beforeEach(() => { + ({ + Counter1x, + Counter10x, + Counter100x, + CounterWithoutIncrementMethod + } = createFixtures()); + }); - it('gets autobound', () => { - const proxy = createProxy(CounterWithoutIncrementMethod); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(0); + it('gets autobound', () => { + const proxy = createProxy(CounterWithoutIncrementMethod); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(0); - proxy.update(Counter1x); - instance.increment.call(null); - expect(renderer.getRenderOutput().props.children).toEqual(1); - }); + proxy.update(Counter1x); + instance.increment.call(null); + expect(renderer.getRenderOutput().props.children).toEqual(1); + }); - it('is autobound after getting replaced', () => { - const proxy = createProxy(Counter1x); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(0); - instance.increment.call(null); - expect(renderer.getRenderOutput().props.children).toEqual(1); - - proxy.update(Counter10x); - instance.increment.call(null); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(11); - - proxy.update(Counter100x); - instance.increment.call(null); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(111); - }); + it('is autobound after getting replaced', () => { + const proxy = createProxy(Counter1x); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(0); + instance.increment.call(null); + expect(renderer.getRenderOutput().props.children).toEqual(1); + + proxy.update(Counter10x); + instance.increment.call(null); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(11); + + proxy.update(Counter100x); + instance.increment.call(null); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(111); }); - }); + } - describe('classic only', () => { - const { Counter1x, Counter10x, Counter100x } = fixtures.classic; + describe('classic', () => { + runCommonTests(createClassicFixtures); /** * Important in case it's a subscription that * later needs to gets destroyed. */ it('preserves the reference', () => { + const { Counter1x, Counter10x, Counter100x } = createClassicFixtures(); const proxy = createProxy(Counter1x); const Proxy = proxy.get(); const instance = renderer.render(); @@ -202,17 +225,16 @@ describe('autobound instance method', () => { proxy.update(Counter100x); expect(instance.increment).toBe(savedIncrement); }); - }); - - describe('modern only', () => { - const { Counter1x, Counter10x, Counter100x } = fixtures.modern; + }) + describe('modern', () => { /** * There's nothing we can do here. * You can't use a lazy autobind with hot reloading * and expect function reference equality. */ it('does not preserve the reference (known limitation)', () => { + const { Counter1x, Counter10x, Counter100x } = createModernFixtures(); const proxy = createProxy(Counter1x); const Proxy = proxy.get(); const instance = renderer.render(); diff --git a/test/instance-method.js b/test/instance-method.js index 705e34b..1a470a9 100644 --- a/test/instance-method.js +++ b/test/instance-method.js @@ -3,134 +3,148 @@ import createShallowRenderer from './helpers/createShallowRenderer'; import expect from 'expect'; import createProxy from '../src'; -const fixtures = { - modern: { - shouldWarnOnBind: false, - - Counter1x: class Counter1x extends Component { - constructor(props) { - super(props); - this.state = { counter: 0 }; - } - - increment() { - this.setState({ - counter: this.state.counter + 1 - }); - } - - render() { - return {this.state.counter}; - } +function createModernFixtures() { + const shouldWarnOnBind = false; + + class Counter1x extends Component { + constructor(props) { + super(props); + this.state = { counter: 0 }; + } + + increment() { + this.setState({ + counter: this.state.counter + 1 + }); + } + + render() { + return {this.state.counter}; + } + } + + class Counter10x extends Component { + constructor(props) { + super(props); + this.state = { counter: 0 }; + } + + increment() { + this.setState({ + counter: this.state.counter + 10 + }); + } + + render() { + return {this.state.counter}; + } + } + + class Counter100x extends Component { + constructor(props) { + super(props); + this.state = { counter: 0 }; + } + + increment() { + this.setState({ + counter: this.state.counter + 100 + }); + } + + render() { + return {this.state.counter}; + } + } + + class CounterWithoutIncrementMethod extends Component { + constructor(props) { + super(props); + this.state = { counter: 0 }; + } + + render() { + return {this.state.counter}; + } + } + + return { + shouldWarnOnBind, + Counter1x, + Counter10x, + Counter100x, + CounterWithoutIncrementMethod + }; +} + +function createClassicFixtures() { + const shouldWarnOnBind = true; + + const Counter1x = React.createClass({ + getInitialState() { + return { counter: 0 }; }, - Counter10x: class Counter10x extends Component { - constructor(props) { - super(props); - this.state = { counter: 0 }; - } - - increment() { - this.setState({ - counter: this.state.counter + 10 - }); - } - - render() { - return {this.state.counter}; - } + increment() { + this.setState({ + counter: this.state.counter + 1 + }); }, - Counter100x: class Counter100x extends Component { - constructor(props) { - super(props); - this.state = { counter: 0 }; - } - - increment() { - this.setState({ - counter: this.state.counter + 100 - }); - } - - render() { - return {this.state.counter}; - } + render() { + return {this.state.counter}; + } + }); + + const Counter10x = React.createClass({ + getInitialState() { + return { counter: 0 }; }, - CounterWithoutIncrementMethod: class CounterWithoutIncrementMethod extends Component { - constructor(props) { - super(props); - this.state = { counter: 0 }; - } + increment() { + this.setState({ + counter: this.state.counter + 10 + }); + }, - render() { - return {this.state.counter}; - } + render() { + return {this.state.counter}; } - }, - - classic: { - shouldWarnOnBind: true, - - Counter1x: React.createClass({ - getInitialState() { - return { counter: 0 }; - }, - - increment() { - this.setState({ - counter: this.state.counter + 1 - }); - }, - - render() { - return {this.state.counter}; - } - }), - - Counter10x: React.createClass({ - getInitialState() { - return { counter: 0 }; - }, - - increment() { - this.setState({ - counter: this.state.counter + 10 - }); - }, - - render() { - return {this.state.counter}; - } - }), - - Counter100x: React.createClass({ - getInitialState() { - return { counter: 0 }; - }, - - increment() { - this.setState({ - counter: this.state.counter + 100 - }); - }, - - render() { - return {this.state.counter}; - } - }), - - CounterWithoutIncrementMethod: React.createClass({ - getInitialState() { - return { counter: 0 }; - }, - - render() { - return {this.state.counter}; - } - }) - } + }); + + const Counter100x = React.createClass({ + getInitialState() { + return { counter: 0 }; + }, + + increment() { + this.setState({ + counter: this.state.counter + 100 + }); + }, + + render() { + return {this.state.counter}; + } + }); + + const CounterWithoutIncrementMethod = React.createClass({ + getInitialState() { + return { counter: 0 }; + }, + + render() { + return {this.state.counter}; + } + }); + + return { + shouldWarnOnBind, + Counter1x, + Counter10x, + Counter100x, + CounterWithoutIncrementMethod + }; }; describe('instance method', () => { @@ -147,137 +161,157 @@ describe('instance method', () => { expect(warnSpy.calls.length).toBe(0); }); - Object.keys(fixtures).forEach(type => { - describe(type, () => { - const { Counter1x, Counter10x, Counter100x, CounterWithoutIncrementMethod, shouldWarnOnBind } = fixtures[type]; + function runCommonTests(createFixtures) { + let shouldWarnOnBind; + let Counter1x; + let Counter10x; + let Counter100x; + let CounterWithoutIncrementMethod; + + beforeEach(() => { + ({ + shouldWarnOnBind, + Counter1x, + Counter10x, + Counter100x, + CounterWithoutIncrementMethod + } = createFixtures()); + }); - it('gets added', () => { - const proxy = createProxy(CounterWithoutIncrementMethod); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(0); + it('gets added', () => { + const proxy = createProxy(CounterWithoutIncrementMethod); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(0); - proxy.update(Counter1x); - instance.increment(); - expect(renderer.getRenderOutput().props.children).toEqual(1); - }); + proxy.update(Counter1x); + instance.increment(); + expect(renderer.getRenderOutput().props.children).toEqual(1); + }); - it('gets replaced', () => { - const proxy = createProxy(Counter1x); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(0); - instance.increment(); - expect(renderer.getRenderOutput().props.children).toEqual(1); - - proxy.update(Counter10x); - instance.increment(); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(11); - - proxy.update(Counter100x); - instance.increment(); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(111); - }); + it('gets replaced', () => { + const proxy = createProxy(Counter1x); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(0); + instance.increment(); + expect(renderer.getRenderOutput().props.children).toEqual(1); + + proxy.update(Counter10x); + instance.increment(); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(11); + + proxy.update(Counter100x); + instance.increment(); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(111); + }); - it('gets replaced if bound by assignment', () => { - const proxy = createProxy(Counter1x); - const Proxy = proxy.get(); - const instance = renderer.render(); + it('gets replaced if bound by assignment', () => { + const proxy = createProxy(Counter1x); + const Proxy = proxy.get(); + const instance = renderer.render(); - warnSpy.destroy(); - const localWarnSpy = expect.spyOn(console, 'error'); + warnSpy.destroy(); + const localWarnSpy = expect.spyOn(console, 'error'); - instance.increment = instance.increment.bind(instance); + instance.increment = instance.increment.bind(instance); - expect(localWarnSpy.calls.length).toBe(shouldWarnOnBind ? 1 : 0); - localWarnSpy.destroy(); + expect(localWarnSpy.calls.length).toBe(shouldWarnOnBind ? 1 : 0); + localWarnSpy.destroy(); - expect(renderer.getRenderOutput().props.children).toEqual(0); - instance.increment(); - expect(renderer.getRenderOutput().props.children).toEqual(1); + expect(renderer.getRenderOutput().props.children).toEqual(0); + instance.increment(); + expect(renderer.getRenderOutput().props.children).toEqual(1); - proxy.update(Counter10x); - instance.increment(); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(11); + proxy.update(Counter10x); + instance.increment(); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(11); - proxy.update(Counter100x); - instance.increment(); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(111); - }); + proxy.update(Counter100x); + instance.increment(); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(111); + }); - it('gets replaced if bound by redefinition', () => { - const proxy = createProxy(Counter1x); - const Proxy = proxy.get(); - const instance = renderer.render(); + it('gets replaced if bound by redefinition', () => { + const proxy = createProxy(Counter1x); + const Proxy = proxy.get(); + const instance = renderer.render(); - warnSpy.destroy(); - const localWarnSpy = expect.spyOn(console, 'error'); + warnSpy.destroy(); + const localWarnSpy = expect.spyOn(console, 'error'); - Object.defineProperty(instance, 'increment', { - value: instance.increment.bind(instance) - }); + Object.defineProperty(instance, 'increment', { + value: instance.increment.bind(instance) + }); - expect(localWarnSpy.calls.length).toBe(shouldWarnOnBind ? 1 : 0); - localWarnSpy.destroy(); + expect(localWarnSpy.calls.length).toBe(shouldWarnOnBind ? 1 : 0); + localWarnSpy.destroy(); - expect(renderer.getRenderOutput().props.children).toEqual(0); - instance.increment(); - expect(renderer.getRenderOutput().props.children).toEqual(1); + expect(renderer.getRenderOutput().props.children).toEqual(0); + instance.increment(); + expect(renderer.getRenderOutput().props.children).toEqual(1); - proxy.update(Counter10x); - instance.increment(); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(11); + proxy.update(Counter10x); + instance.increment(); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(11); - proxy.update(Counter100x); - instance.increment(); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(111); - }); + proxy.update(Counter100x); + instance.increment(); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(111); + }); - /** - * It is important to make deleted methods no-ops - * so they don't crash if setTimeout-d or setInterval-d. - */ - it('is detached and acts as a no-op if not reassigned and deleted', () => { - const proxy = createProxy(Counter1x); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(0); - instance.increment(); - const savedIncrement = instance.increment; - expect(renderer.getRenderOutput().props.children).toEqual(1); - - proxy.update(CounterWithoutIncrementMethod); - expect(instance.increment).toEqual(undefined); - savedIncrement.call(instance); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(1); - }); + /** + * It is important to make deleted methods no-ops + * so they don't crash if setTimeout-d or setInterval-d. + */ + it('is detached and acts as a no-op if not reassigned and deleted', () => { + const proxy = createProxy(Counter1x); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(0); + instance.increment(); + const savedIncrement = instance.increment; + expect(renderer.getRenderOutput().props.children).toEqual(1); + + proxy.update(CounterWithoutIncrementMethod); + expect(instance.increment).toEqual(undefined); + savedIncrement.call(instance); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(1); + }); - it('is attached and acts as a no-op if reassigned and deleted', () => { - const proxy = createProxy(Counter1x); - const Proxy = proxy.get(); - const instance = renderer.render(); + it('is attached and acts as a no-op if reassigned and deleted', () => { + const proxy = createProxy(Counter1x); + const Proxy = proxy.get(); + const instance = renderer.render(); - // Pass an additional argument so that in classic mode, - // autobinding doesn't provide us a cached bound handler, - // and instead actually performs the bind (which is being tested). - instance.increment = instance.increment.bind(instance, 'lol'); + // Pass an additional argument so that in classic mode, + // autobinding doesn't provide us a cached bound handler, + // and instead actually performs the bind (which is being tested). + instance.increment = instance.increment.bind(instance, 'lol'); - expect(renderer.getRenderOutput().props.children).toEqual(0); - instance.increment(); - expect(renderer.getRenderOutput().props.children).toEqual(1); + expect(renderer.getRenderOutput().props.children).toEqual(0); + instance.increment(); + expect(renderer.getRenderOutput().props.children).toEqual(1); - proxy.update(CounterWithoutIncrementMethod); - instance.increment(); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(1); - }); + proxy.update(CounterWithoutIncrementMethod); + instance.increment(); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(1); }); + } + + describe('classic', () => { + runCommonTests(createClassicFixtures); + }); + + describe('modern', () => { + runCommonTests(createModernFixtures); }); }); \ No newline at end of file diff --git a/test/instance-property.js b/test/instance-property.js index f49e395..c26ae0e 100644 --- a/test/instance-property.js +++ b/test/instance-property.js @@ -3,59 +3,69 @@ import createShallowRenderer from './helpers/createShallowRenderer'; import expect from 'expect'; import createProxy from '../src'; -const fixtures = { - modern: { - InstanceProperty: class InstanceProperty extends Component { - answer = 42; - - render() { - return
{this.answer}
; - } +function createModernFixtures() { + class InstanceProperty extends Component { + answer = 42; + + render() { + return
{this.answer}
; + } + } + + class InstancePropertyUpdate extends Component { + answer = 43; + + render() { + return
{this.answer}
; + } + } + + class InstancePropertyRemoval extends Component { + render() { + return
{this.answer}
; + } + } + + return { + InstanceProperty, + InstancePropertyUpdate, + InstancePropertyRemoval + }; +} + +function createClassicFixtures() { + const InstanceProperty = React.createClass({ + componentWillMount() { + this.answer = 42; }, - InstancePropertyUpdate: class InstancePropertyUpdate extends Component { - answer = 43; + render() { + return
{this.answer}
; + } + }); - render() { - return
{this.answer}
; - } + const InstancePropertyUpdate = React.createClass({ + componentWillMount() { + this.answer = 43; }, - InstancePropertyRemoval: class InstancePropertyRemoval extends Component { - render() { - return
{this.answer}
; - } + render() { + return
{this.answer}
; } - }, - - classic: { - InstanceProperty: React.createClass({ - componentWillMount() { - this.answer = 42; - }, - - render() { - return
{this.answer}
; - } - }), - - InstancePropertyUpdate: React.createClass({ - componentWillMount() { - this.answer = 43; - }, - - render() { - return
{this.answer}
; - } - }), - - InstancePropertyRemoval: React.createClass({ - render() { - return
{this.answer}
; - } - }) - } -}; + }); + + const InstancePropertyRemoval = React.createClass({ + render() { + return
{this.answer}
; + } + }); + + return { + InstanceProperty, + InstancePropertyUpdate, + InstancePropertyRemoval + }; +} describe('instance property', () => { let renderer; @@ -71,59 +81,75 @@ describe('instance property', () => { expect(warnSpy.calls.length).toBe(0); }); - Object.keys(fixtures).forEach(type => { - describe(type, () => { - const { InstanceProperty, InstancePropertyUpdate, InstancePropertyRemoval } = fixtures[type]; - - it('is available on proxy class instance', () => { - const proxy = createProxy(InstanceProperty); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - expect(instance.answer).toEqual(42); - }); - - it('is left unchanged when reassigned', () => { - const proxy = createProxy(InstanceProperty); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - - instance.answer = 100; - - proxy.update(InstancePropertyUpdate); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(100); - expect(instance.answer).toEqual(100); - - proxy.update(InstancePropertyRemoval); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(100); - expect(instance.answer).toEqual(100); - }); - - /** - * I'm not aware of any way of retrieving their new values - * without calling the constructor, which seems like too much - * of a side effect. We also don't want to overwrite them - * in case they changed. - */ - it('is left unchanged even if not reassigned (known limitation)', () => { - const proxy = createProxy(InstanceProperty); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - - proxy.update(InstancePropertyUpdate); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - expect(instance.answer).toEqual(42); - - proxy.update(InstancePropertyRemoval); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - expect(instance.answer).toEqual(42); - }); + function runCommonTests(createFixtures) { + let InstanceProperty; + let InstancePropertyUpdate; + let InstancePropertyRemoval; + + beforeEach(() => { + ({ + InstanceProperty, + InstancePropertyUpdate, + InstancePropertyRemoval + } = createFixtures()); + }); + + it('is available on proxy class instance', () => { + const proxy = createProxy(InstanceProperty); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + expect(instance.answer).toEqual(42); + }); + + it('is left unchanged when reassigned', () => { + const proxy = createProxy(InstanceProperty); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + + instance.answer = 100; + + proxy.update(InstancePropertyUpdate); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(100); + expect(instance.answer).toEqual(100); + + proxy.update(InstancePropertyRemoval); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(100); + expect(instance.answer).toEqual(100); + }); + + /** + * I'm not aware of any way of retrieving their new values + * without calling the constructor, which seems like too much + * of a side effect. We also don't want to overwrite them + * in case they changed. + */ + it('is left unchanged even if not reassigned (known limitation)', () => { + const proxy = createProxy(InstanceProperty); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + + proxy.update(InstancePropertyUpdate); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + expect(instance.answer).toEqual(42); + + proxy.update(InstancePropertyRemoval); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + expect(instance.answer).toEqual(42); }); + } + + describe('classic', () => { + runCommonTests(createClassicFixtures); + }); + + describe('modern', () => { + runCommonTests(createModernFixtures); }); }); \ No newline at end of file diff --git a/test/pure-component.js b/test/pure-component.js index 4c2194c..36d747d 100644 --- a/test/pure-component.js +++ b/test/pure-component.js @@ -3,21 +3,25 @@ import createShallowRenderer from './helpers/createShallowRenderer'; import expect from 'expect'; import createProxy from '../src'; -const fixtures = { - pure: { - Bar(props) { - return
Bar
; - }, - - Baz(props) { - return
Baz
; - }, - - Foo(props) { - return
Foo
; - } +function createPureFixtures() { + function Bar(props) { + return
Bar
; } -}; + + function Baz(props) { + return
Baz
; + } + + function Foo(props) { + return
Foo
; + } + + return { + Bar, + Baz, + Foo + }; +} describe('pure component', () => { let renderer; @@ -33,24 +37,32 @@ describe('pure component', () => { expect(warnSpy.calls.length).toBe(0); }); - Object.keys(fixtures).forEach(type => { - describe(type, () => { - const { Bar, Baz, Foo } = fixtures[type]; + describe('pure', () => { + let Bar; + let Baz; + let Foo; + + beforeEach(() => { + ({ + Bar, + Baz, + Foo + } = createPureFixtures()); + }); - it('gets replaced', () => { - const proxy = createProxy(Bar); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Bar'); + it('gets replaced', () => { + const proxy = createProxy(Bar); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Bar'); - proxy.update(Baz); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Baz'); + proxy.update(Baz); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Baz'); - proxy.update(Foo); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Foo'); - }); + proxy.update(Foo); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Foo'); }); }); }); \ No newline at end of file diff --git a/test/static-descriptor.js b/test/static-descriptor.js index d6e694a..6eb9d6b 100644 --- a/test/static-descriptor.js +++ b/test/static-descriptor.js @@ -3,53 +3,58 @@ import createShallowRenderer from './helpers/createShallowRenderer'; import expect from 'expect'; import createProxy from '../src'; -const fixtures = { - modern: { - StaticDescriptor: class StaticDescriptor extends Component { - static get answer() { - return 42; - } - - static set something(value) { - this._something = value * 2; - } - - render() { - return
{this.constructor.answer}
; - } - }, - - StaticDescriptorUpdate: class StaticDescriptorUpdate extends Component { - static get answer() { - return 43; - } - - static set something(value) { - this._something = value * 3; - } - - render() { - return
{this.constructor.answer}
; - } - }, - - StaticDescriptorRemoval: class StaticDescriptorRemoval extends Component { - render() { - return
{this.constructor.answer}
; - } - }, - - ThrowingAccessors: class ThrowingAccessors extends Component { - static get something() { - throw new Error(); - } - - static set something(value) { - throw new Error(); - } +function createModernFixtures() { + class StaticDescriptor extends Component { + static get answer() { + return 42; + } + + static set something(value) { + this._something = value * 2; + } + + render() { + return
{this.constructor.answer}
; } } -}; + + class StaticDescriptorUpdate extends Component { + static get answer() { + return 43; + } + + static set something(value) { + this._something = value * 3; + } + + render() { + return
{this.constructor.answer}
; + } + } + + class StaticDescriptorRemoval extends Component { + render() { + return
{this.constructor.answer}
; + } + } + + class ThrowingAccessors extends Component { + static get something() { + throw new Error(); + } + + static set something(value) { + throw new Error(); + } + } + + return { + StaticDescriptor, + StaticDescriptorUpdate, + StaticDescriptorRemoval, + ThrowingAccessors + }; +} describe('static descriptor', () => { let renderer; @@ -65,128 +70,138 @@ describe('static descriptor', () => { expect(warnSpy.calls.length).toBe(0); }); - Object.keys(fixtures).forEach(type => { - const { StaticDescriptor, StaticDescriptorUpdate, StaticDescriptorRemoval, ThrowingAccessors } = fixtures[type]; + describe('modern', () => { + let StaticDescriptor; + let StaticDescriptorUpdate; + let StaticDescriptorRemoval; + let ThrowingAccessors; + + beforeEach(() => { + ({ + StaticDescriptor, + StaticDescriptorUpdate, + StaticDescriptorRemoval, + ThrowingAccessors + } = createModernFixtures()); + }); + + it('does not invoke accessors', () => { + const proxy = createProxy(StaticDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(() => proxy.update(ThrowingAccessors)).toNotThrow(); + }); - describe(type, () => { - it('does not invoke accessors', () => { + describe('getter', () => { + it('is available on proxy class', () => { const proxy = createProxy(StaticDescriptor); const Proxy = proxy.get(); const instance = renderer.render(); - expect(() => proxy.update(ThrowingAccessors)).toNotThrow(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + expect(instance.constructor.answer).toEqual(42); + expect(Proxy.answer).toEqual(42); }); - describe('getter', () => { - it('is available on proxy class', () => { - const proxy = createProxy(StaticDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - expect(instance.constructor.answer).toEqual(42); - expect(Proxy.answer).toEqual(42); - }); + it('gets added', () => { + const proxy = createProxy(StaticDescriptorRemoval); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(undefined); - it('gets added', () => { - const proxy = createProxy(StaticDescriptorRemoval); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(undefined); + proxy.update(StaticDescriptor); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + expect(instance.constructor.answer).toEqual(42); + }); - proxy.update(StaticDescriptor); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - expect(instance.constructor.answer).toEqual(42); - }); + it('gets replaced', () => { + const proxy = createProxy(StaticDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); - it('gets replaced', () => { - const proxy = createProxy(StaticDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - - proxy.update(StaticDescriptorUpdate); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(43); - expect(instance.constructor.answer).toEqual(43); - - proxy.update(StaticDescriptorRemoval); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(undefined); - expect(instance.answer).toEqual(undefined); - }); + proxy.update(StaticDescriptorUpdate); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(43); + expect(instance.constructor.answer).toEqual(43); - it('gets redefined', () => { - const proxy = createProxy(StaticDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - - Object.defineProperty(instance.constructor, 'answer', { - value: 7 - }); - - proxy.update(StaticDescriptorUpdate); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(7); - expect(instance.constructor.answer).toEqual(7); - - proxy.update(StaticDescriptorRemoval); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(7); - expect(instance.constructor.answer).toEqual(7); - }); + proxy.update(StaticDescriptorRemoval); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(undefined); + expect(instance.answer).toEqual(undefined); }); - describe('setter', () => { - it('is available on proxy class instance', () => { - const proxy = createProxy(StaticDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - instance.constructor.something = 10; + it('gets redefined', () => { + const proxy = createProxy(StaticDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + + Object.defineProperty(instance.constructor, 'answer', { + value: 7 }); - it('gets added', () => { - const proxy = createProxy(StaticDescriptorRemoval); - const Proxy = proxy.get(); - const instance = renderer.render(); + proxy.update(StaticDescriptorUpdate); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(7); + expect(instance.constructor.answer).toEqual(7); - proxy.update(StaticDescriptor); - instance.constructor.something = 10; - expect(instance.constructor._something).toEqual(20); - }); + proxy.update(StaticDescriptorRemoval); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(7); + expect(instance.constructor.answer).toEqual(7); + }); + }); - it('gets replaced', () => { - const proxy = createProxy(StaticDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - instance.constructor.something = 10; - expect(instance.constructor._something).toEqual(20); - - proxy.update(StaticDescriptorUpdate); - expect(instance.constructor._something).toEqual(20); - instance.constructor.something = 10; - expect(instance.constructor._something).toEqual(30); - - proxy.update(StaticDescriptorRemoval); - expect(instance.constructor._something).toEqual(30); - instance.constructor.something = 7; - expect(instance.constructor.something).toEqual(7); - expect(instance.constructor._something).toEqual(30); - }); + describe('setter', () => { + it('is available on proxy class instance', () => { + const proxy = createProxy(StaticDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + instance.constructor.something = 10; + }); + + it('gets added', () => { + const proxy = createProxy(StaticDescriptorRemoval); + const Proxy = proxy.get(); + const instance = renderer.render(); - it('gets redefined', () => { - const proxy = createProxy(StaticDescriptor); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); + proxy.update(StaticDescriptor); + instance.constructor.something = 10; + expect(instance.constructor._something).toEqual(20); + }); + + it('gets replaced', () => { + const proxy = createProxy(StaticDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + instance.constructor.something = 10; + expect(instance.constructor._something).toEqual(20); + + proxy.update(StaticDescriptorUpdate); + expect(instance.constructor._something).toEqual(20); + instance.constructor.something = 10; + expect(instance.constructor._something).toEqual(30); + + proxy.update(StaticDescriptorRemoval); + expect(instance.constructor._something).toEqual(30); + instance.constructor.something = 7; + expect(instance.constructor.something).toEqual(7); + expect(instance.constructor._something).toEqual(30); + }); - Object.defineProperty(instance.constructor, 'something', { - value: 50 - }); + it('gets redefined', () => { + const proxy = createProxy(StaticDescriptor); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); - proxy.update(StaticDescriptorUpdate); - expect(instance.constructor.something).toEqual(50); + Object.defineProperty(instance.constructor, 'something', { + value: 50 }); + + proxy.update(StaticDescriptorUpdate); + expect(instance.constructor.something).toEqual(50); }); }); }); diff --git a/test/static-method.js b/test/static-method.js index 059df5f..ac4be3a 100644 --- a/test/static-method.js +++ b/test/static-method.js @@ -3,79 +3,89 @@ import createShallowRenderer from './helpers/createShallowRenderer'; import expect from 'expect'; import createProxy from '../src'; -const fixtures = { - modern: { - StaticMethod: class StaticMethod extends Component { - static getAnswer() { - return 42; - }; +function createModernFixtures() { + class StaticMethod extends Component { + static getAnswer() { + return 42; + }; + + render() { + return ( +
{this.constructor.getAnswer()}
+ ); + } + } - render() { - return ( -
{this.constructor.getAnswer()}
- ); + class StaticMethodUpdate extends Component { + static getAnswer() { + return 43; + }; + + render() { + return ( +
{this.constructor.getAnswer()}
+ ); + } + } + + class StaticMethodRemoval extends Component { + render() { + return ( +
{this.constructor.getAnswer()}
+ ); + } + } + + return { + StaticMethod, + StaticMethodUpdate, + StaticMethodRemoval + }; +} + +function createClassicFixtures() { + const StaticMethod = React.createClass({ + statics: { + getAnswer() { + return 42; } }, - StaticMethodUpdate: class StaticMethodUpdate extends Component { - static getAnswer() { - return 43; - }; + render() { + return ( +
{this.constructor.getAnswer()}
+ ); + } + }); - render() { - return ( -
{this.constructor.getAnswer()}
- ); + const StaticMethodUpdate = React.createClass({ + statics: { + getAnswer() { + return 43; } }, - StaticMethodRemoval: class StaticMethodRemoval extends Component { - render() { - return ( -
{this.constructor.getAnswer()}
- ); - } + render() { + return ( +
{this.constructor.getAnswer()}
+ ); } - }, - - classic: { - StaticMethod: React.createClass({ - statics: { - getAnswer() { - return 42; - } - }, - - render() { - return ( -
{this.constructor.getAnswer()}
- ); - } - }), - - StaticMethodUpdate: React.createClass({ - statics: { - getAnswer() { - return 43; - } - }, - - render() { - return ( -
{this.constructor.getAnswer()}
- ); - } - }), + }); - StaticMethodRemoval: React.createClass({ - render() { - return ( -
{this.constructor.getAnswer()}
- ); - } - }) - } -}; + const StaticMethodRemoval = React.createClass({ + render() { + return ( +
{this.constructor.getAnswer()}
+ ); + } + }); + + return { + StaticMethod, + StaticMethodUpdate, + StaticMethodRemoval + }; +} describe('static method', () => { let renderer; @@ -91,80 +101,94 @@ describe('static method', () => { expect(warnSpy.calls.length).toBe(0); }); - Object.keys(fixtures).forEach(type => { - describe(type, () => { - const { StaticMethod, StaticMethodUpdate, StaticMethodRemoval } = fixtures[type]; - - it('is available on proxy class instance', () => { - const proxy = createProxy(StaticMethod); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - expect(Proxy.getAnswer()).toEqual(42); - }); - - it('is own on proxy class instance', () => { - const proxy = createProxy(StaticMethod); - const Proxy = proxy.get(); - expect(Proxy.hasOwnProperty('getAnswer')).toEqual(true); - }); - - it('gets added', () => { - const proxy = createProxy(StaticMethodRemoval); - const Proxy = proxy.get(); - expect(Proxy.getAnswer).toEqual(undefined); - - proxy.update(StaticMethod); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - expect(Proxy.getAnswer()).toEqual(42); - }); - - it('gets replaced', () => { - const proxy = createProxy(StaticMethod); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - expect(Proxy.getAnswer()).toEqual(42); - - proxy.update(StaticMethodUpdate); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(43); - expect(Proxy.getAnswer()).toEqual(43); - }); - - /** - * Known limitation. - * If you find a way around it without breaking other tests, let me know! - */ - it('does not get replaced if bound (known limitation)', () => { - const proxy = createProxy(StaticMethod); - const Proxy = proxy.get(); - const getAnswer = Proxy.getAnswer = Proxy.getAnswer.bind(Proxy); - - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - - proxy.update(StaticMethodUpdate); - renderer.render(); - - expect(renderer.getRenderOutput().props.children).toEqual(42); - expect(Proxy.getAnswer()).toEqual(42); - expect(getAnswer()).toEqual(42); - }); - - it('is detached if deleted', () => { - const proxy = createProxy(StaticMethod); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - expect(Proxy.getAnswer()).toEqual(42); - - proxy.update(StaticMethodRemoval); - expect(() => instance.forceUpdate()).toThrow(); - expect(() => renderer.render()).toThrow(); - expect(Proxy.getAnswer).toEqual(undefined); - }); + function runCommonTests(createFixtures) { + let StaticMethod; + let StaticMethodUpdate; + let StaticMethodRemoval; + let CounterWithoutIncrementMethod; + + beforeEach(() => { + ({ + StaticMethod, + StaticMethodUpdate, + StaticMethodRemoval, + } = createFixtures()); + }); + + it('is available on proxy class instance', () => { + const proxy = createProxy(StaticMethod); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + expect(Proxy.getAnswer()).toEqual(42); + }); + + it('is own on proxy class instance', () => { + const proxy = createProxy(StaticMethod); + const Proxy = proxy.get(); + expect(Proxy.hasOwnProperty('getAnswer')).toEqual(true); + }); + + it('gets added', () => { + const proxy = createProxy(StaticMethodRemoval); + const Proxy = proxy.get(); + expect(Proxy.getAnswer).toEqual(undefined); + + proxy.update(StaticMethod); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + expect(Proxy.getAnswer()).toEqual(42); + }); + + it('gets replaced', () => { + const proxy = createProxy(StaticMethod); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + expect(Proxy.getAnswer()).toEqual(42); + + proxy.update(StaticMethodUpdate); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(43); + expect(Proxy.getAnswer()).toEqual(43); }); + + it('gets replaced if bound', () => { + const proxy = createProxy(StaticMethod); + const Proxy = proxy.get(); + const getAnswer = Proxy.getAnswer = Proxy.getAnswer.bind(Proxy); + + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + + proxy.update(StaticMethodUpdate); + renderer.render(); + + expect(renderer.getRenderOutput().props.children).toEqual(43); + expect(Proxy.getAnswer()).toEqual(43); + // Can we make this work too? Probably isn't worth bothering: + expect(getAnswer()).toEqual(42); + }); + + it('is detached if deleted', () => { + const proxy = createProxy(StaticMethod); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual(42); + expect(Proxy.getAnswer()).toEqual(42); + + proxy.update(StaticMethodRemoval); + expect(() => instance.forceUpdate()).toThrow(); + expect(() => renderer.render()).toThrow(); + expect(Proxy.getAnswer).toEqual(undefined); + }); + } + + describe('classic', () => { + runCommonTests(createClassicFixtures); + }); + + describe('modern', () => { + runCommonTests(createModernFixtures); }); }); \ No newline at end of file diff --git a/test/static-property.js b/test/static-property.js index 4d6335a..bd121a3 100644 --- a/test/static-property.js +++ b/test/static-property.js @@ -3,131 +3,163 @@ import createShallowRenderer from './helpers/createShallowRenderer'; import expect from 'expect'; import createProxy from '../src'; -const fixtures = { - modern: { - StaticProperty: class StaticProperty extends Component { - static answer = 42; - - render() { - return ( -
{this.constructor.answer}
- ); - } - }, +function createModernFixtures() { + class StaticProperty extends Component { + static answer = 42; + + render() { + return ( +
+ {StaticProperty.answer} + {this.constructor.answer} +
+ ); + } + } + + class StaticPropertyUpdate extends Component { + static answer = 43; + + render() { + return ( +
+ {StaticPropertyUpdate.answer} + {this.constructor.answer} +
+ ); + } + } + + class StaticPropertyRemoval extends Component { + render() { + return ( +
+ {StaticPropertyRemoval.answer} + {this.constructor.answer} +
+ ); + } + } + + class PropTypes extends Component { + static propTypes = { + something: React.PropTypes.number + }; + + static contextTypes = { + something: React.PropTypes.number + }; + + static childContextTypes = { + something: React.PropTypes.number + }; + } + + class PropTypesUpdate extends Component { + static propTypes = { + something: React.PropTypes.string + }; - StaticPropertyUpdate: class StaticPropertyUpdate extends Component { - static answer = 43; + static contextTypes = { + something: React.PropTypes.string + }; + + static childContextTypes = { + something: React.PropTypes.string + }; + } - render() { - return ( -
{this.constructor.answer}
- ); - } + return { + StaticProperty, + StaticPropertyUpdate, + StaticPropertyRemoval, + PropTypes, + PropTypesUpdate + }; +} + +function createClassicFixtures() { + const StaticProperty = React.createClass({ + statics: { + answer: 42 }, - StaticPropertyRemoval: class StaticPropertyRemoval extends Component { - render() { - return ( -
{this.constructor.answer}
- ); - } + render() { + return ( +
+ {StaticProperty.answer} + {this.constructor.answer} +
+ ); + } + }); + + const StaticPropertyUpdate = React.createClass({ + statics: { + answer: 43 }, - PropTypes: class PropTypes extends Component { - static propTypes = { - something: React.PropTypes.number - }; + render() { + return ( +
+ {StaticPropertyUpdate.answer} + {this.constructor.answer} +
+ ); + } + }); + + const StaticPropertyRemoval = React.createClass({ + render() { + return ( +
+ {StaticPropertyRemoval.answer} + {this.constructor.answer} +
+ ); + } + }); + + const PropTypes = React.createClass({ + render() {}, - static contextTypes = { - something: React.PropTypes.number - }; + propTypes: { + something: React.PropTypes.number + }, - static childContextTypes = { - something: React.PropTypes.number - }; + contextTypes: { + something: React.PropTypes.number }, - PropTypesUpdate: class PropTypesUpdate extends Component { - static propTypes = { - something: React.PropTypes.string - }; + childContextTypes: { + something: React.PropTypes.number + } + }); + + const PropTypesUpdate = React.createClass({ + render() {}, - static contextTypes = { - something: React.PropTypes.string - }; + propTypes: { + something: React.PropTypes.string + }, + + contextTypes: { + something: React.PropTypes.string + }, - static childContextTypes = { - something: React.PropTypes.string - }; + childContextTypes: { + something: React.PropTypes.string } - }, - - classic: { - StaticProperty: React.createClass({ - statics: { - answer: 42 - }, - - render() { - return ( -
{this.constructor.answer}
- ); - } - }), - - StaticPropertyUpdate: React.createClass({ - statics: { - answer: 43 - }, - - render() { - return ( -
{this.constructor.answer}
- ); - } - }), - - StaticPropertyRemoval: React.createClass({ - render() { - return ( -
{this.constructor.answer}
- ); - } - }), - - PropTypes: React.createClass({ - render() {}, - - propTypes: { - something: React.PropTypes.number - }, - - contextTypes: { - something: React.PropTypes.number - }, - - childContextTypes: { - something: React.PropTypes.number - } - }), - - PropTypesUpdate: React.createClass({ - render() {}, - - propTypes: { - something: React.PropTypes.string - }, - - contextTypes: { - something: React.PropTypes.string - }, - - childContextTypes: { - something: React.PropTypes.string - } - }) - } -}; + }); + + return { + StaticProperty, + StaticPropertyUpdate, + StaticPropertyRemoval, + PropTypes, + PropTypesUpdate + }; +} describe('static property', () => { let renderer; @@ -143,78 +175,128 @@ describe('static property', () => { expect(warnSpy.calls.length).toBe(0); }); - Object.keys(fixtures).forEach(type => { - describe(type, () => { - const { - StaticProperty, StaticPropertyUpdate, StaticPropertyRemoval, - PropTypes, PropTypesUpdate - } = fixtures[type]; - - it('is available on proxy class instance', () => { - const proxy = createProxy(StaticProperty); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - expect(Proxy.answer).toEqual(42); - }); - - it('is own on proxy class instance', () => { - const proxy = createProxy(StaticProperty); - const Proxy = proxy.get(); - expect(Proxy.hasOwnProperty('answer')).toEqual(true); - }); - - it('is changed when not reassigned', () => { - const proxy = createProxy(StaticProperty); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - - proxy.update(StaticPropertyUpdate); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(43); - expect(Proxy.answer).toEqual(43); - - proxy.update(StaticPropertyRemoval); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(undefined); - expect(Proxy.answer).toEqual(undefined); - }); - - it('is changed for propTypes, contextTypes, childContextTypes', () => { - const proxy = createProxy(PropTypes); - const PropTypesProxy = proxy.get(); - expect(PropTypesProxy.propTypes.something).toEqual(React.PropTypes.number); - expect(PropTypesProxy.contextTypes.something).toEqual(React.PropTypes.number); - expect(PropTypesProxy.childContextTypes.something).toEqual(React.PropTypes.number); - - proxy.update(PropTypesUpdate); - expect(PropTypesProxy.propTypes.something).toEqual(React.PropTypes.string); - expect(PropTypesProxy.contextTypes.something).toEqual(React.PropTypes.string); - expect(PropTypesProxy.childContextTypes.something).toEqual(React.PropTypes.string); - }); - - /** - * Sometimes people dynamically store stuff on statics. - */ - it('is not changed when reassigned', () => { - const proxy = createProxy(StaticProperty); - const Proxy = proxy.get(); - const instance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(42); - - Proxy.answer = 100; - - proxy.update(StaticPropertyUpdate); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(100); - expect(Proxy.answer).toEqual(100); - - proxy.update(StaticPropertyRemoval); - renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual(100); - expect(Proxy.answer).toEqual(100); - }); + function runCommonTests(createFixtures) { + let StaticProperty; + let StaticPropertyUpdate; + let StaticPropertyRemoval; + let PropTypes; + let PropTypesUpdate; + + beforeEach(() => { + ({ + StaticProperty, + StaticPropertyUpdate, + StaticPropertyRemoval, + PropTypes, + PropTypesUpdate + } = createFixtures()); + }); + + it('is available on both classes', () => { + const proxy = createProxy(StaticProperty); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([42, 42]); + expect(StaticProperty.answer).toEqual(42); + expect(Proxy.answer).toEqual(42); + }); + + it('is own on both classes', () => { + const proxy = createProxy(StaticProperty); + const Proxy = proxy.get(); + expect(StaticProperty.hasOwnProperty('answer')).toEqual(true); + expect(Proxy.hasOwnProperty('answer')).toEqual(true); + }); + + it('is changed when not reassigned', () => { + const proxy = createProxy(StaticProperty); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([42, 42]); + expect(StaticProperty.answer).toEqual(42); + expect(Proxy.answer).toEqual(42); + + proxy.update(StaticPropertyUpdate); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([43, 43]); + expect(StaticPropertyUpdate.answer).toEqual(43); + expect(Proxy.answer).toEqual(43); + + proxy.update(StaticPropertyRemoval); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([undefined, undefined]); + expect(StaticPropertyRemoval.answer).toEqual(undefined); + expect(Proxy.answer).toEqual(undefined); + }); + + it('is changed for propTypes, contextTypes, childContextTypes', () => { + const proxy = createProxy(PropTypes); + const PropTypesProxy = proxy.get(); + expect(PropTypesProxy.propTypes.something).toEqual(React.PropTypes.number); + expect(PropTypesProxy.contextTypes.something).toEqual(React.PropTypes.number); + expect(PropTypesProxy.childContextTypes.something).toEqual(React.PropTypes.number); + + proxy.update(PropTypesUpdate); + expect(PropTypesProxy.propTypes.something).toEqual(React.PropTypes.string); + expect(PropTypesProxy.contextTypes.something).toEqual(React.PropTypes.string); + expect(PropTypesProxy.childContextTypes.something).toEqual(React.PropTypes.string); + }); + + it('is not changed when reassigned on initial class (declared)', () => { + const proxy = createProxy(StaticProperty); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([42, 42]); + + StaticProperty.answer = 100; + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([100, 100]); + expect(StaticProperty.answer).toEqual(100); + expect(Proxy.answer).toEqual(42); // Proxy gets synced on update() + + proxy.update(StaticPropertyUpdate); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([100, 100]); + expect(StaticPropertyUpdate.answer).toEqual(100); + expect(Proxy.answer).toEqual(100); + + proxy.update(StaticPropertyRemoval); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([100, 100]); + expect(StaticPropertyRemoval.answer).toEqual(100); + expect(Proxy.answer).toEqual(100); }); + + it('is not changed when reassigned on initial class (undeclared)', () => { + const proxy = createProxy(StaticPropertyRemoval); + const Proxy = proxy.get(); + const instance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([undefined, undefined]); + + StaticPropertyRemoval.answer = 100; + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([100, 100]); + expect(Proxy.answer).toEqual(undefined); // Proxy gets synced on update() + + proxy.update(StaticPropertyUpdate); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([100, 100]); + expect(StaticPropertyUpdate.answer).toEqual(100); + expect(Proxy.answer).toEqual(100); + + proxy.update(StaticProperty); + renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual([100, 100]); + expect(StaticPropertyRemoval.answer).toEqual(100); + expect(Proxy.answer).toEqual(100); + }); + } + + describe('classic', () => { + runCommonTests(createClassicFixtures); + }); + + describe('modern', () => { + runCommonTests(createModernFixtures); }); }); \ No newline at end of file diff --git a/test/unmounting.js b/test/unmounting.js index 7271433..82527d5 100644 --- a/test/unmounting.js +++ b/test/unmounting.js @@ -3,71 +3,81 @@ import createShallowRenderer from './helpers/createShallowRenderer'; import expect from 'expect'; import createProxy from '../src'; -const fixtures = { - modern: { - Bar: class Bar extends Component { - componentWillUnmount() { - this.didUnmount = true; - } - - render() { - return
Bar
; - } +function createModernFixtures() { + class Bar extends Component { + componentWillUnmount() { + this.didUnmount = true; + } + + render() { + return
Bar
; + } + } + + class Baz extends Component { + componentWillUnmount() { + this.didUnmount = true; + } + + render() { + return
Baz
; + } + } + + class Foo extends Component { + componentWillUnmount() { + this.didUnmount = true; + } + + render() { + return
Foo
; + } + } + + return { + Bar, + Baz, + Foo + }; +} + +function createClassicFixtures() { + const Bar = React.createClass({ + componentWillUnmount() { + this.didUnmount = true; }, - Baz: class Baz extends Component { - componentWillUnmount() { - this.didUnmount = true; - } + render() { + return
Bar
; + } + }); - render() { - return
Baz
; - } + const Baz = React.createClass({ + componentWillUnmount() { + this.didUnmount = true; }, - Foo: class Foo extends Component { - componentWillUnmount() { - this.didUnmount = true; - } + render() { + return
Baz
; + } + }); - render() { - return
Foo
; - } + const Foo = React.createClass({ + componentWillUnmount() { + this.didUnmount = true; + }, + + render() { + return
Foo
; } - }, - - classic: { - Bar: React.createClass({ - componentWillUnmount() { - this.didUnmount = true; - }, - - render() { - return
Bar
; - } - }), - - Baz: React.createClass({ - componentWillUnmount() { - this.didUnmount = true; - }, - - render() { - return
Baz
; - } - }), - - Foo: React.createClass({ - componentWillUnmount() { - this.didUnmount = true; - }, - - render() { - return
Foo
; - } - }) - } -}; + }); + + return { + Bar, + Baz, + Foo + }; +} describe('unmounting', () => { let renderer; @@ -83,58 +93,74 @@ describe('unmounting', () => { expect(warnSpy.calls.length).toBe(0); }); - Object.keys(fixtures).forEach(type => { - describe(type, () => { - const { Bar, Baz, Foo } = fixtures[type]; - - it('happens without proxy', () => { - const barInstance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Bar'); - const bazInstance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Baz'); - expect(barInstance).toNotEqual(bazInstance); - expect(barInstance.didUnmount).toEqual(true); - }); - - it('does not happen when rendering new proxied versions', () => { - const proxy = createProxy(Bar); - const BarProxy = proxy.get(); - const barInstance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Bar'); - - proxy.update(Baz); - const BazProxy = proxy.get(); - const bazInstance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Baz'); - expect(barInstance).toEqual(bazInstance); - expect(barInstance.didUnmount).toEqual(undefined); - - proxy.update(Foo); - const FooProxy = proxy.get(); - const fooInstance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Foo'); - expect(barInstance).toEqual(fooInstance); - expect(barInstance.didUnmount).toEqual(undefined); - }); - - it('does not happen when rendering old proxied versions', () => { - const proxy = createProxy(Bar); - const Proxy = proxy.get(); - const barInstance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Bar'); - - proxy.update(Baz); - const bazInstance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Baz'); - expect(barInstance).toEqual(bazInstance); - expect(barInstance.didUnmount).toEqual(undefined); - - proxy.update(Foo); - const fooInstance = renderer.render(); - expect(renderer.getRenderOutput().props.children).toEqual('Foo'); - expect(barInstance).toEqual(fooInstance); - expect(barInstance.didUnmount).toEqual(undefined); - }); + function runCommonTests(createFixtures) { + let Bar; + let Baz; + let Foo; + + beforeEach(() => { + ({ + Bar, + Baz, + Foo + } = createFixtures()); + }); + + it('happens without proxy', () => { + const barInstance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Bar'); + const bazInstance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Baz'); + expect(barInstance).toNotEqual(bazInstance); + expect(barInstance.didUnmount).toEqual(true); }); + + it('does not happen when rendering new proxied versions', () => { + const proxy = createProxy(Bar); + const BarProxy = proxy.get(); + const barInstance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Bar'); + + proxy.update(Baz); + const BazProxy = proxy.get(); + const bazInstance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Baz'); + expect(barInstance).toEqual(bazInstance); + expect(barInstance.didUnmount).toEqual(undefined); + + proxy.update(Foo); + const FooProxy = proxy.get(); + const fooInstance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Foo'); + expect(barInstance).toEqual(fooInstance); + expect(barInstance.didUnmount).toEqual(undefined); + }); + + it('does not happen when rendering old proxied versions', () => { + const proxy = createProxy(Bar); + const Proxy = proxy.get(); + const barInstance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Bar'); + + proxy.update(Baz); + const bazInstance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Baz'); + expect(barInstance).toEqual(bazInstance); + expect(barInstance.didUnmount).toEqual(undefined); + + proxy.update(Foo); + const fooInstance = renderer.render(); + expect(renderer.getRenderOutput().props.children).toEqual('Foo'); + expect(barInstance).toEqual(fooInstance); + expect(barInstance.didUnmount).toEqual(undefined); + }); + } + + describe('classic', () => { + runCommonTests(createClassicFixtures); + }); + + describe('modern', () => { + runCommonTests(createModernFixtures); }); }); \ No newline at end of file From 065d22c334bdc0a4198223f7d1fd35d14440e2dd Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sun, 17 Apr 2016 15:00:06 +0100 Subject: [PATCH 2/6] 3.0.0-alpha.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 892f387..61456dc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "react-proxy", - "version": "2.0.8", + "version": "3.0.0-alpha.0", "description": "Proxies React components without unmounting or losing their state.", "main": "modules/index.js", "scripts": { From e429f95fc15561562944f9fc8cc8ab0fd955be2e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 19 Apr 2016 01:46:23 +0100 Subject: [PATCH 3/6] Try to copy the method name --- src/createPrototypeProxy.js | 5 +++++ test/consistency.js | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/src/createPrototypeProxy.js b/src/createPrototypeProxy.js index 9c5dcf6..db2ed7d 100644 --- a/src/createPrototypeProxy.js +++ b/src/createPrototypeProxy.js @@ -34,6 +34,11 @@ export default function createPrototypeProxy() { // Copy properties of the original function, if any assign(proxiedMethod, current[name]); proxiedMethod.toString = proxyToString(name); + try { + Object.defineProperty(proxiedMethod, 'name', { + value: name + }); + } catch (err) { } return proxiedMethod; } diff --git a/test/consistency.js b/test/consistency.js index f753d74..2be6620 100644 --- a/test/consistency.js +++ b/test/consistency.js @@ -205,6 +205,12 @@ describe('consistency', () => { expect(propertyNames).toInclude('doNothing'); }); + it('copies name to new method', () => { + let proxy = createProxy(Bar); + const Proxy = proxy.get(); + expect(Proxy.prototype.doNothing.name).toBe('doNothing'); + }); + it('preserves enumerability and writability of methods', () => { let proxy = createProxy(Bar); const Proxy = proxy.get(); From 4cf5daba1f01084dba2d22f12e2abdc6a0f80f8c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 19 Apr 2016 02:18:23 +0100 Subject: [PATCH 4/6] Infer names more consistently --- src/createClassProxy.js | 23 ++++++++++++++++++++-- test/consistency.js | 43 ++++++++++++++++++++++++++--------------- 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/src/createClassProxy.js b/src/createClassProxy.js index fbc63ed..a769d90 100644 --- a/src/createClassProxy.js +++ b/src/createClassProxy.js @@ -29,6 +29,13 @@ function isEqualDescriptor(a, b) { return true; } +function getDisplayName(Component) { + const displayName = Component.displayName || Component.name; + return (displayName && displayName !== 'ReactComponent') ? + displayName : + 'Unknown'; +} + // This was originally a WeakMap but we had issues with React Native: // https://github.com/gaearon/react-proxy/issues/50#issuecomment-192928066 let allProxies = []; @@ -70,10 +77,11 @@ function proxyClass(InitialComponent) { } } + let displayName = getDisplayName(InitialComponent); try { // Create a proxy constructor with matching name ProxyComponent = new Function('factory', 'instantiate', - `return function ${InitialComponent.name || 'ProxyComponent'}() { + `return function ${displayName}() { return instantiate(factory, this, arguments); }` )(() => CurrentComponent, instantiate); @@ -83,6 +91,11 @@ function proxyClass(InitialComponent) { return instantiate(() => CurrentComponent, this, arguments); }; } + try { + Object.defineProperty(ProxyComponent, 'name', { + value: displayName + }); + } catch (err) { } // Proxy toString() to the current constructor ProxyComponent.toString = function toString() { @@ -115,7 +128,13 @@ function proxyClass(InitialComponent) { CurrentComponent = NextComponent; // Try to infer displayName - ProxyComponent.displayName = NextComponent.displayName || NextComponent.name; + displayName = getDisplayName(NextComponent); + ProxyComponent.displayName = displayName; + try { + Object.defineProperty(ProxyComponent, 'name', { + value: displayName + }); + } catch (err) { } // Set up the same prototype for inherited statics ProxyComponent.__proto__ = NextComponent.__proto__; diff --git a/test/consistency.js b/test/consistency.js index 2be6620..a010e1d 100644 --- a/test/consistency.js +++ b/test/consistency.js @@ -39,7 +39,14 @@ function createModernFixtures() { } } - return { Bar, Baz, Foo }; + class Anon extends React.Component { + render() { + return
Anon
; + } + } + delete Anon.name; + + return { Bar, Baz, Foo, Anon }; } function createClassicFixtures() { @@ -78,7 +85,14 @@ function createClassicFixtures() { } }); - return { Bar, Baz, Foo }; + const Anon = React.createClass({ + render() { + return
Anon
; + } + }); + delete Anon.displayName; + + return { Bar, Baz, Foo, Anon }; } describe('consistency', () => { @@ -96,9 +110,9 @@ describe('consistency', () => { }); function runCommonTests(createFixtures) { - let Bar, Baz, Foo; + let Bar, Baz, Foo, Anon; beforeEach(() => { - ({ Foo, Bar, Baz } = createFixtures()); + ({ Foo, Bar, Baz, Anon } = createFixtures()); }); it('does not overwrite the original class', () => { @@ -185,16 +199,23 @@ describe('consistency', () => { expect(barInstance instanceof Baz).toEqual(true); }); - it('sets up displayName from displayName or name', () => { + it('sets up name and displayName from displayName or name', () => { let proxy = createProxy(Bar); const Proxy = proxy.get(); + expect(Proxy.name).toEqual('Bar'); expect(Proxy.displayName).toEqual('Bar'); proxy.update(Baz); + expect(Proxy.name).toEqual('Baz'); expect(Proxy.displayName).toEqual('Baz'); proxy.update(Foo); + expect(Proxy.name).toEqual('Foo (Custom)'); expect(Proxy.displayName).toEqual('Foo (Custom)'); + + proxy.update(Anon); + expect(Proxy.name).toEqual('Unknown'); + expect(Proxy.displayName).toEqual('Unknown'); }); it('keeps own methods on the prototype', () => { @@ -205,7 +226,7 @@ describe('consistency', () => { expect(propertyNames).toInclude('doNothing'); }); - it('copies name to new method', () => { + it('preserves method names', () => { let proxy = createProxy(Bar); const Proxy = proxy.get(); expect(Proxy.prototype.doNothing.name).toBe('doNothing'); @@ -262,16 +283,6 @@ describe('consistency', () => { ({ Bar, Baz, Foo } = createModernFixtures()); }) - it('sets up the constructor name from initial name', () => { - let proxy = createProxy(Bar); - const Proxy = proxy.get(); - expect(Proxy.name).toEqual('Bar'); - - // Known limitation: name can't change - proxy.update(Baz); - expect(Proxy.name).toEqual('Bar'); - }); - it('should not crash if new Function() throws', () => { let oldFunction = global.Function; From 883084296e3cc9acb3077b7e61de13c9cb9f01b7 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 19 Apr 2016 02:18:39 +0100 Subject: [PATCH 5/6] 3.0.0-alpha.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 61456dc..6e0603b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "react-proxy", - "version": "3.0.0-alpha.0", + "version": "3.0.0-alpha.1", "description": "Proxies React components without unmounting or losing their state.", "main": "modules/index.js", "scripts": { From c019762362a4936ed86d6bb89ad6a57e13de6c06 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 19 Apr 2016 02:36:02 +0100 Subject: [PATCH 6/6] Add some tests for error swallowing --- src/createClassProxy.js | 2 -- test/consistency.js | 15 +++++++++++++++ test/pure-component.js | 27 +++++++++++++++++++++++++-- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/createClassProxy.js b/src/createClassProxy.js index a769d90..c94a2af 100644 --- a/src/createClassProxy.js +++ b/src/createClassProxy.js @@ -65,9 +65,7 @@ function proxyClass(InitialComponent) { try { return component.apply(context, params); } catch (err) { - // Native ES6 class instantiation const instance = new component(...params); - Object.keys(instance).forEach(key => { if (RESERVED_STATICS.indexOf(key) > -1) { return; diff --git a/test/consistency.js b/test/consistency.js index a010e1d..6f3a7d1 100644 --- a/test/consistency.js +++ b/test/consistency.js @@ -40,6 +40,11 @@ function createModernFixtures() { } class Anon extends React.Component { + constructor(props) { + super(props); + throw new Error('Oops.'); + } + render() { return
Anon
; } @@ -86,6 +91,10 @@ function createClassicFixtures() { }); const Anon = React.createClass({ + getInitialState() { + throw new Error('Oops.'); + }, + render() { return
Anon
; } @@ -269,6 +278,12 @@ describe('consistency', () => { }); expect(doNothingBeforeItWasDeleted.toString()).toEqual(''); }); + + it('does not swallow constructor errors', () => { + let proxy = createProxy(Anon); + const Proxy = proxy.get(); + expect(() => renderer.render()).toThrow('Oops'); + }); } describe('classic', () => { diff --git a/test/pure-component.js b/test/pure-component.js index 36d747d..f52094c 100644 --- a/test/pure-component.js +++ b/test/pure-component.js @@ -16,10 +16,20 @@ function createPureFixtures() { return
Foo
; } + function Qux() { + throw new Error('Oops.'); + } + + const Quy = () => { + throw new Error('Ouch.'); + } + return { Bar, Baz, - Foo + Foo, + Qux, + Quy }; } @@ -41,12 +51,16 @@ describe('pure component', () => { let Bar; let Baz; let Foo; + let Qux; + let Quy; beforeEach(() => { ({ Bar, Baz, - Foo + Foo, + Qux, + Quy } = createPureFixtures()); }); @@ -64,5 +78,14 @@ describe('pure component', () => { renderer.render(); expect(renderer.getRenderOutput().props.children).toEqual('Foo'); }); + + it('does not swallow errors', () => { + const proxy = createProxy(Qux); + const Proxy = proxy.get(); + expect(() => renderer.render()).toThrow('Oops.'); + + proxy.update(Quy); + expect(() => renderer.render()).toThrow('Ouch.'); + }); }); }); \ No newline at end of file