-
-
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
Invoke() api #1856
Invoke() api #1856
Conversation
I haven't had time to work on my PR, so thanks for this! |
@woutervanvliet Thank you for your reviews! I've fixed them. |
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 will need a full rebase - not just a merge - on latest master.
@koba04 ping, still interested in this? |
@ljharb OK, I'll work on it again! |
I've recreated the branch from |
1eecdaf
to
5e28ce4
Compare
@ljharb I’ve addressed you comment and rebased this! |
} | ||
return (...args) => { | ||
const response = handler.apply(this, args); | ||
this[ROOT].update(); |
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.
why is this needed?
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.
@ljharb
The purpose is to reflect the updates into the ShallowWrapper instance if the handler
calls setState
.
It's the same as what simulate()
is doing.
https://github.com/airbnb/enzyme/blob/master/packages/enzyme/src/ShallowWrapper.js#L999
But I've covered the most case by this PR so this might be unnecessary.
} | ||
return (...args) => { | ||
const response = handler.apply(this, args); | ||
this[ROOT].update(); |
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.
ditto
This reverts commit eb14926cbc5de867745ca7c892bda533ddff043b.
I'm not sure what's going on with coverage there; I'm going to merge anyways. |
Thanks! |
This PR is to pick up from #945
If @milesj is still working on #945, feel free to close this!