Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add disableLifecycleMethods for shallow #789

Merged
merged 5 commits into from
Mar 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion docs/api/shallow.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ describe('<MyComponent />', () => {

1. `node` (`ReactElement`): The node to render
2. `options` (`Object` [optional]):
- `options.context`: (`Object` [optional]): Context to be passed into the component
- `options.context`: (`Object` [optional]): Context to be passed into the component
- `options.disableLifecycleMethods`: (`Boolean` [optional]): If set to true, `componentDidMount`
is not called on the component, and `componentDidUpdate` is not called after
[`setProps`](ShallowWrapper/setProps.md) and [`setContext`](ShallowWrapper/setContext.md).

#### Returns

Expand Down
36 changes: 36 additions & 0 deletions src/ShallowWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,48 @@ function filterWhereUnwrapped(wrapper, predicate) {
return wrapper.wrap(compact(wrapper.getNodes().filter(predicate)));
}

/**
* Ensure options passed to ShallowWrapper are valid. Throws otherwise.
* @param {Object} options
*/
function validateOptions(options) {
const { lifecycleExperimental, disableLifecycleMethods } = options;
if (
typeof lifecycleExperimental !== 'undefined' &&
typeof lifecycleExperimental !== 'boolean'
) {
throw new Error(
'lifecycleExperimental must be either true or false if provided',
);
}

if (
typeof disableLifecycleMethods !== 'undefined' &&
typeof disableLifecycleMethods !== 'boolean'
) {
throw new Error(
'disableLifecycleMethods must be either true or false if provided',
);
}

if (
lifecycleExperimental != null &&
disableLifecycleMethods != null &&
lifecycleExperimental === disableLifecycleMethods
) {
throw new Error(
'lifecycleExperimental and disableLifecycleMethods cannot be set to the same value',
);
}
}

/**
* @class ShallowWrapper
*/
class ShallowWrapper {

constructor(nodes, root, options = {}) {
validateOptions(options);
if (!root) {
this.root = this;
this.unrendered = nodes;
Expand Down
156 changes: 155 additions & 1 deletion test/ShallowWrapper-spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import sinon from 'sinon';
import { shallow, render, ShallowWrapper } from '../src/';
import { describeIf, itIf, itWithData, generateEmptyRenderData } from './_helpers';
import { ITERATOR_SYMBOL, withSetStateAllowed } from '../src/Utils';
import { REACT013, REACT15 } from '../src/version';
import { REACT013, REACT014, REACT15 } from '../src/version';

describe('shallow', () => {
describe('context', () => {
Expand Down Expand Up @@ -2683,7 +2683,161 @@ describe('shallow', () => {
});
});

describe('disableLifecycleMethods', () => {
describe('validation', () => {
it('throws for a non-boolean value', () => {
['value', 42, null].forEach((value) => {
expect(() => shallow(<div />, {
disableLifecycleMethods: value,
})).to.throw(/true or false/);
});
});

it('does not throw for a boolean value or undefined', () => {
[true, false, undefined].forEach((value) => {
expect(() => shallow(<div />, {
disableLifecycleMethods: value,
})).not.to.throw();
});
});

it('does not throw when no lifecycle flags are provided in options', () => {
expect(() => shallow(<div />, {})).not.to.throw();
});

it('throws when used with lifecycleExperimental in invalid combinations', () => {
[true, false].forEach((value) => {
expect(() => shallow(<div />, {
lifecycleExperimental: value,
disableLifecycleMethods: value,
})).to.throw(/same value/);
});
});
});

describe('when set to true', () => {
let wrapper;
const spy = sinon.spy();
class Foo extends React.Component {
componentWillMount() {
spy('componentWillMount');
}
componentDidMount() {
spy('componentDidMount');
}
componentWillReceiveProps() {
spy('componentWillReceiveProps');
}
shouldComponentUpdate() {
spy('shouldComponentUpdate');
return true;
}
componentWillUpdate() {
spy('componentWillUpdate');
}
componentDidUpdate() {
spy('componentDidUpdate');
}
componentWillUnmount() {
spy('componentWillUnmount');
}
render() {
spy('render');
return <div>foo</div>;
}
}

const options = {
disableLifecycleMethods: true,
context: {
foo: 'foo',
},
};

beforeEach(() => {
wrapper = shallow(<Foo />, options);
spy.reset();
});

it('does not call componentDidMount when mounting', () => {
wrapper = shallow(<Foo />, options);
expect(spy.args).to.deep.equal([
['componentWillMount'],
['render'],
]);
});

it('calls expected methods when receiving new props', () => {
wrapper.setProps({ foo: 'foo' });
expect(spy.args).to.deep.equal([
['componentWillReceiveProps'],
['shouldComponentUpdate'],
['componentWillUpdate'],
['render'],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether componentDidUpdate is supposed to be called here, but it's not currently. If this behavior is correct it's going to make this flag + documentation pretty confusing since, per next test down, setState does trigger componentDidUpdate for shallow rendered components (even without enzyme in the picture).

The flag is looking more like it should be disableDidMountAllTheTimeAndDidUpdateSomeOfTheTime. 😅

cc @koba04 any thoughts on correctness here? Note this is all without lifecycleExperimental set to true.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jwbay
Currently, lifecycleExperimental does complement lifecycle methods that ShallowRenderer is missing, which are componentDidMount and componentDidUpdate in setProps and setContext #318 (comment).

In setState, componentDidUpdate is always called because ShallowWrapper calls instance.setState() directly. 😅

To be clear, if disableLifecycleMethods set to true, componentDidMount and componentDidUpdate(in setProps and setContext) are not called on the component.

]);
});

describeIf(REACT013 || REACT15, 'setContext', () => {
it('calls expected methods when receiving new context', () => {
wrapper.setContext({ foo: 'foo' });
expect(spy.args).to.deep.equal([
['componentWillReceiveProps'],
['shouldComponentUpdate'],
['componentWillUpdate'],
['render'],
]);
});
});

describeIf(REACT014, 'setContext', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm glad we're testing this on all React versions - could you elaborate on why componentWillReceiveProps fires in 13 and 15, but not 14?

Copy link
Contributor Author

@jwbay jwbay Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In React 13 and 14, the check for whether willReceiveProps is called is just a strict equality check in ReactCompositeComponent between the previous and next elements. In React 15, they added an additional equality check between previous and next context.

willReceiveProps is called in 13 because Enzyme clones the element on each render in react-compat's createRendererCompatible, failing the equality check.

It's not called in 14 because the elements are equal. createRendererCompatible is only used for React 13.

It's called in 15 because of the new check for context.

it('calls expected methods when receiving new context', () => {
wrapper.setContext({ foo: 'foo' });
expect(spy.args).to.deep.equal([
['shouldComponentUpdate'],
['componentWillUpdate'],
['render'],
]);
});
});

it('calls expected methods for setState', () => {
wrapper.setState({ bar: 'bar' });
expect(spy.args).to.deep.equal([
['shouldComponentUpdate'],
['componentWillUpdate'],
['render'],
['componentDidUpdate'],
]);
});

it('calls expected methods when unmounting', () => {
wrapper.unmount();
expect(spy.args).to.deep.equal([
['componentWillUnmount'],
]);
});
});
});

describe('lifecycleExperimental', () => {
describe('validation', () => {
it('throws for a non-boolean value', () => {
['value', 42, null].forEach((value) => {
expect(() => shallow(<div />, {
lifecycleExperimental: value,
})).to.throw(/true or false/);
});
});

it('does not throw for a boolean value or when not provided', () => {
[true, false, undefined].forEach((value) => {
expect(() => shallow(<div />, {
lifecycleExperimental: value,
})).not.to.throw();
});
});
});

context('mounting phase', () => {
it('should call componentWillMount and componentDidMount', () => {
const spy = sinon.spy();
Expand Down