-
Notifications
You must be signed in to change notification settings - Fork 153
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
fix(TS): Cypress commands return jQuery objects #56
Conversation
cfb9952
to
7938750
Compare
Do we need users to install the jQuery types on their own projects? |
Travis is failing because of a Cypress dependency: [test] /home/travis/.cache/Cypress/3.3.1/Cypress/Cypress: error while loading shared libraries: libgconf-2.so.4: cannot open shared object file: No such file or directory |
I'll give this a look next week :-) thank you so much! |
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 using these types on any project currently, so even though this looks fine to me, I can't verify that it's correct. But I'm willing to take your word for it if you're willing to fix any bugs that people come up with upon its release 😅
expect(el.play).to.exist | ||
} | ||
}) | ||
test('includes proper TypeScript types', () => { |
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.
Does cypress support the test
global? 🤔
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.
good point! It uses Mocha, not Jest.
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.
Ah it looks like Mocha supports it, according to its types.
I think this is the solution: cypress-io/cypress#4069 I'll go ahead and fix this in master right now. |
Hi @kentcdodds! Thanks for the review! Yeah, I'm willing to get this fixed up as issues are raised. |
@kentcdodds I'll update the readme when I know if we require users to do anything extra. |
Sorry, I just introduced a merge conflict because that snapshot file was nothing but pain so I removed it. Could you rebase? Then I think we should be ready to go. |
Without this fix, the types are wrong using methods like `invoke` or using callbacks on commands.
7938750
to
6eadd6b
Compare
Yeah I've been removing snapshots in my projects too! I've just rebased and pushed |
@all-contributors please add @aaronmcadam for code and tests |
I've put up a pull request to add @aaronmcadam! 🎉 |
🎉 This PR is included in version 4.0.4 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Without this fix, the types are wrong using methods like
invoke
or using callbacks on commands.Fixes #39.
Supercedes #40.
Thanks to @simenbrekken for doing the initial work on this!
Checklist: