-
-
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
Breaking: lifecycleExperimental by default #1140
Breaking: lifecycleExperimental by default #1140
Conversation
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.
This LGTM - please rebase, and we can get this in for v3.
I'm working on rebasing and fixing a test. |
e8296d7
to
9930f3e
Compare
89f3322
to
bb4c6b6
Compare
Thanks! |
constructor(nodes, root, options = {}) { | ||
validateOptions(options); | ||
constructor(nodes, root, passedOptions = {}) { | ||
validateOptions(passedOptions); |
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.
How come ReactWrapper doesn't validate its options?
enzyme calls componentDidMount() by default as of v3. * https://github.com/airbnb/enzyme/blob/master/CHANGELOG.md#breaking-changes * enzymejs/enzyme#1140
enzyme calls componentDidMount() by default as of v3. * https://github.com/airbnb/enzyme/blob/master/CHANGELOG.md#breaking-changes * enzymejs/enzyme#1140
There is a discussion in #678 about this.
If enzyme chooses this option, the time has come.
I think this PR needs more works, which are tests and documentation.
Does enzyme enable lifecyleExperimental by default at v3?