Add support for simulating events on Components #211
Conversation
andrewiggins
force-pushed
the
simulate-events-on-components
branch
2 times, most recently
from
4eb3914 to
4a5c388
Compare
src/ShallowRenderer.ts
Outdated
| this._mountRenderer.simulateEvent(node, eventName, args); | ||
| const handler = node.props[propFromEvent(eventName)]; | ||
| if (handler) { | ||
| handler(...args); |
There was a problem hiding this comment.
This looks like a significant change to how event simulation works with shallow rendering. Can you provide some more context here? This isn't going to cause a problem for the company I work for as we don't use shallow rendering, but it could potentially be a surprise for other users. If we do decide to do this, it ought to be marked as a breaking change.
There was a problem hiding this comment.
This change was just to bring our adapter into alignment with the React adapter. See my other comment for the source.
Yes, good call. We should release this as a breaking change. Should I retarget this PR to a "v5" branch we can release after gathering a couple of changes?
To be clear, my end goal here is to bring our enzyme adapter and react's enzyme adapter a bit closer in feature parity to make migrating easier.
There was a problem hiding this comment.
Should I retarget this PR to a "v5" branch we can release after gathering a couple of changes?
Are there other breaking changes that you expect may be needed soon?
There was a problem hiding this comment.
I'm not sure yet. I'm still exploring/discovering what behaviors of the React 16 adapter our existing tests rely on, so it is possible. I imagine within a week or two I'll have a good grasp of what they are and if they are breaking or not.
There was a problem hiding this comment.
One possibility to avoid the need for a separate branch would be to add a feature flag in the adapter's configuration to control which behavior is used. The flag can be set to preserve the current behavior and then flipped in the next major release.
There was a problem hiding this comment.
Ooh nice! I've hidden the new behavior behind a feature flag passed into the Adapter constructor.
|
@robertknight Responded to your comments. Let me know what you think! |
src/ShallowRenderer.ts
Outdated
| this._mountRenderer.simulateEvent(node, eventName, args); | ||
| const handler = node.props[propFromEvent(eventName)]; | ||
| if (handler) { | ||
| handler(...args); |
There was a problem hiding this comment.
One possibility to avoid the need for a separate branch would be to add a feature flag in the adapter's configuration to control which behavior is used. The flag can be set to preserve the current behavior and then flipped in the next major release.
There was a problem hiding this comment.
@robertknight Let me know if these changes match what you had in mind :)
src/ShallowRenderer.ts
Outdated
| this._mountRenderer.simulateEvent(node, eventName, args); | ||
| const handler = node.props[propFromEvent(eventName)]; | ||
| if (handler) { | ||
| handler(...args); |
There was a problem hiding this comment.
Ooh nice! I've hidden the new behavior behind a feature flag passed into the Adapter constructor.
andrewiggins
force-pushed
the
simulate-events-on-components
branch
from
6f0b62b to
d3bf0fc
Compare
No description provided.