-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Changes from all commits
8eeb0b7
cff2ff5
9fc50dc
5dda4f8
765ec95
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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', () => { | ||
|
@@ -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'], | ||
]); | ||
}); | ||
|
||
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', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
|
There was a problem hiding this comment.
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 triggercomponentDidUpdate
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.
There was a problem hiding this comment.
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 arecomponentDidMount
andcomponentDidUpdate
insetProps
andsetContext
#318 (comment).In
setState
,componentDidUpdate
is always called because ShallowWrapper callsinstance.setState()
directly. 😅To be clear, if
disableLifecycleMethods
set to true,componentDidMount
andcomponentDidUpdate
(insetProps
andsetContext
) are not called on the component.