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.

Sign up for GitHub

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

Jump to bottom

fix(type): only return Promise with delay #491

Merged
merged 3 commits into from Nov 11, 2020
Merged

fix(type): only return Promise with delay #491

merged 3 commits into from Nov 11, 2020

Conversation

nickmccurdy
Copy link
Member

@nickmccurdy nickmccurdy commented Nov 11, 2020 •
edited

What: The TypeScript type for the type function will now only return a Promise when given a delay option.

Why: #480

How: Updated type declaration and added basic type tests.

Checklist:

  • Documentation N/A
  • Tests
  • Typings
  • Ready to be merged

@nickmccurdy nickmccurdy mentioned this pull request Nov 11, 2020
Closed
@nickmccurdy nickmccurdy linked an issue Nov 11, 2020 that may be closed by this pull request
[Typings] improve userEvent.type typings to only return a Promise when using delay is set #480
Closed
kentcdodds
kentcdodds approved these changes Nov 11, 2020
View reviewed changes
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Nice! This is great 👍 thank you!

@kentcdodds
Copy link
Member

Ugh, we had this build issue elsewhere: rollup/plugins#640

luistak
luistak approved these changes Nov 11, 2020
View reviewed changes
typings/test.ts
type NotVoid = string | number | boolean | object | null

userEvent.type(element, 'foo', {delay: 1}) as Promise<void>
// @ts-expect-error No delay returns void
Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome usage of typescript this was new to me many thanks! ❤️

@codecov
Copy link

codecov bot commented Nov 11, 2020 •
edited

Codecov Report

Merging #491 (8478406) into master (d1bcda4) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #491   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          615       615           
  Branches       200       200           
=========================================
  Hits           615       615

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1bcda4...8478406. Read the comment docs.

@kentcdodds
Copy link
Member

It looks like our testing code coveraged dipped. So we can merge this, but it won't get released until we fix the code coverage level. From the look of it, it's all around isContentEditable:

image

image

image

@kentcdodds kentcdodds merged commit 2f585fe into master Nov 11, 2020
@kentcdodds kentcdodds deleted the only-return-promise-with-delay branch November 11, 2020 03:28
@github-actions
Copy link

🎉 This PR is included in version 12.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nickmccurdy
Copy link
Member Author

That's strange, the codecov support says 100% coverage. Is that incorrect or only for the files in this PR?

@kentcdodds
Copy link
Member

Yeah, very odd. I just ran it locally out of curiosity and got that result. But this build succeeded and the library got published so 🤷‍♂️

@nickmccurdy
Copy link
Member Author

nickmccurdy commented Nov 13, 2020 •
edited

I just pulled, deleted node_modules, and ran npm install. Now I have 100% coverage locally. I guess some package update caused coverage to improve.

@svc-secops svc-secops mentioned this pull request Jul 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kentcdodds kentcdodds kentcdodds approved these changes

@luistak luistak luistak approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Typings] improve userEvent.type typings to only return a Promise when using delay is set
4 participants
@nickmccurdy @kentcdodds @luistak @ljosberinn