Skip to content

Commit

Permalink
Fix UNSAFE_* lifecycles being overwritten
Browse files Browse the repository at this point in the history
This was caused by transpilers overwriting our patched property.
  • Loading branch information
marvinhagemeister committed Sep 24, 2019
1 parent 8c6ac43 commit 46c8f9b
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 20 deletions.
34 changes: 21 additions & 13 deletions compat/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,19 +364,6 @@ function memo(c, comparer) {
return Memoed;
}

// Patch in `UNSAFE_*` lifecycle hooks
function setUnsafeDescriptor(obj, key) {
Object.defineProperty(obj.prototype, 'UNSAFE_' + key, {
configurable: true,
get() { return this[key]; },
set(v) { this[key] = v; }
});
}

setUnsafeDescriptor(Component, 'componentWillMount');
setUnsafeDescriptor(Component, 'componentWillReceiveProps');
setUnsafeDescriptor(Component, 'componentWillUpdate');

/**
* Pass ref down to a child. This is mainly used in libraries with HOCs that
* wrap components. Using `forwardRef` there is an easy way to get a reference
Expand All @@ -396,6 +383,17 @@ function forwardRef(fn) {
return Forwarded;
}

// Patch in `UNSAFE_*` lifecycle hooks
function setSafeDescriptor(obj, key) {
if (obj.prototype['UNSAFE_'+key] && !obj.prototype[key]) {
Object.defineProperty(obj.prototype, key, {
configurable: false,
get() { return this['UNSAFE_' + key]; },
set(v) { this['UNSAFE_' + key] = v; }
});
}
}

let oldVNodeHook = options.vnode;
options.vnode = vnode => {
vnode.$$typeof = REACT_ELEMENT_TYPE;
Expand All @@ -406,6 +404,16 @@ options.vnode = vnode => {
vnode.props.ref = vnode.ref;
vnode.ref = null;
}

// We can't just patch the base component class, because components that use
// inheritance and are transpiled down to ES5 will overwrite our patched
// getters and setters. See #1941
if (typeof type === 'function' && !type._patchedLifecycles && type.prototype) {
setSafeDescriptor(type, 'componentWillMount');
setSafeDescriptor(type, 'componentWillReceiveProps');
setSafeDescriptor(type, 'componentWillUpdate');
type._patchedLifecycles = true;
}
/* istanbul ignore next */
if (oldVNodeHook) oldVNodeHook(vnode);
};
Expand Down
56 changes: 56 additions & 0 deletions compat/test/browser/component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,23 @@ describe('components', () => {
expect(spy).to.be.calledOnce;
});

it('should support UNSAFE_componentWillMount #2', () => {
let spy = sinon.spy();

class Foo extends React.Component {
render() {
return <h1>foo</h1>;
}
}

Object.defineProperty(Foo.prototype, 'UNSAFE_componentWillMount', {
value: spy
});

React.render(<Foo />, scratch);
expect(spy).to.be.calledOnce;
});

it('should support UNSAFE_componentWillReceiveProps', () => {
let spy = sinon.spy();

Expand All @@ -186,6 +203,25 @@ describe('components', () => {
expect(spy).to.be.calledOnce;
});

it('should support UNSAFE_componentWillReceiveProps #2', () => {
let spy = sinon.spy();

class Foo extends React.Component {
render() {
return <h1>foo</h1>;
}
}

Object.defineProperty(Foo.prototype, 'UNSAFE_componentWillReceiveProps', {
value: spy
});

React.render(<Foo />, scratch);
// Trigger an update
React.render(<Foo />, scratch);
expect(spy).to.be.calledOnce;
});

it('should support UNSAFE_componentWillUpdate', () => {
let spy = sinon.spy();

Expand All @@ -206,6 +242,26 @@ describe('components', () => {
expect(spy).to.be.calledOnce;
});

it('should support UNSAFE_componentWillUpdate #2', () => {
let spy = sinon.spy();

class Foo extends React.Component {
render() {
return <h1>foo</h1>;
}
}


Object.defineProperty(Foo.prototype, 'UNSAFE_componentWillUpdate', {
value: spy
});

React.render(<Foo />, scratch);
// Trigger an update
React.render(<Foo />, scratch);
expect(spy).to.be.calledOnce;
});

it('should alias UNSAFE_* method to non-prefixed variant', () => {
let inst;
class Foo extends React.Component {
Expand Down
13 changes: 6 additions & 7 deletions compat/test/browser/suspense.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,19 +201,18 @@ describe('suspense', () => {
});

it('should not call lifecycle methods when suspending', () => {
let componentWillMount = sinon.spy();
let componentDidMount = sinon.spy();
let componentWillUnmount = sinon.spy();
class LifecycleLogger extends Component {
render() {
return <div>Lifecycle</div>;
}
componentWillMount() {}
componentDidMount() {}
componentWillUnmount() {}
componentWillMount() { componentWillMount(); }
componentDidMount() { componentDidMount(); }
componentWillUnmount() { componentWillUnmount(); }
}

const componentWillMount = sinon.spy(LifecycleLogger.prototype, 'componentWillMount');
const componentDidMount = sinon.spy(LifecycleLogger.prototype, 'componentDidMount');
const componentWillUnmount = sinon.spy(LifecycleLogger.prototype, 'componentWillUnmount');

const [Suspender, suspend] = createSuspender(() => <div>Suspense</div>);

render((
Expand Down

0 comments on commit 46c8f9b

Please sign in to comment.