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 default events object to shallow wrapper simulate #329

Closed
wants to merge 1 commit into from
Closed

Add default events object to shallow wrapper simulate #329

wants to merge 1 commit into from

Conversation

danielhusar
Copy link

@danielhusar danielhusar commented Apr 18, 2016

Hey,

I had to pass preventDefault mock lot of times to simulate event in shallow rendering, so I thought it would be nice to have it by default there.

Fixes: #277

@danielhusar
Copy link
Author

It failed on old node's because of jsdom not working there :(

// TODO(lmr): emulate React's event propagation
if (!args.length && window) {
args.push(new window.Event(event));
Copy link
Member

Choose a reason for hiding this comment

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

window is not available when shallow-wrapping.

Copy link
Author

Choose a reason for hiding this comment

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

Don't most users have it available when running tests ?
I know its not requirement for shallow rendering, but are there many people that use only shallow rendering ?

Optionally I was thinking to add jsdom there but introducing this as dependency is probably not good idea.

Maybe just hardcode some event object with most used properties ?

Copy link
Member

Choose a reason for hiding this comment

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

No. Many users do not use mount at all and do not have the DOM available during tests.

shallow must not require or assume a browser environment.

Copy link
Author

Choose a reason for hiding this comment

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

👍 So what would be better approach ? Check for global.window and add it only for users that have it exposed ? (Leads to inconsistency)
Or create manually event object ? (Don't think so we can mock properly all properties)

Copy link
Member

Choose a reason for hiding this comment

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

Manually create an event object. The behavior of shallow should not differ based on the presence or absence of browser globals.

@blainekasten
Copy link
Contributor

I feel like something I would rather see is a mocking event object in enzyme that I can pass to simulate.

Something like this:

wrapper.simulate('click', enzyme.mockEvent);

And that is effectively an object that represents the shape of a typical event.

@lelandrichardson
Copy link
Collaborator

We definitely cannot rely on browser APIs for shallow.

I would love to see a well built event mocking system built for enzyme that would remove some of the boilerplate, but I hesitate to add something that only implements it half-way - which we would then have to have a potential additional breaking change for in the future.

@lelandrichardson
Copy link
Collaborator

I'm going to close this in favor of #368 which I think is going more closely in the direction that we want, but I believe will accomplish everything you are wanting.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants