Type configured userEvent field for easier testing DX #1421
Conversation
|
Size Change: +240 B (+0.03%) Total Size: 854 kB
ℹ️ View Unchanged
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1421 +/- ##
==========================================
+ Coverage 69.37% 70.22% +0.85%
==========================================
Files 504 508 +4
Lines 105315 105698 +383
Branches 7565 10799 +3234
==========================================
+ Hits 73062 74229 +1167
+ Misses 32071 31469 -602
+ Partials 182 0 -182 see 150 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
| @@ -80,8 +80,9 @@ describe("Editor", () => { | |||
| render(<Harnessed onChange={onChangeMock} />); | |||
|
|
|||
| // Act | |||
| screen.getByRole("button", {name: "Remove image widget"}), | |||
| await userEvent.click(); | |||
| await userEvent.click( | |||
| @@ -80,7 +80,7 @@ function ConnectedMathInput({keypadConfiguration = defaultConfiguration}) { | |||
| } | |||
|
|
|||
| describe("math input integration", () => { | |||
| let userEvent; | |||
| let userEvent: ReturnType<typeof userEventLib.setup>; | |||
There was a problem hiding this comment.
I'm wondering if it'd be useful to alias this so it's clearer.
type UserEvent = ReturnType<typeof userEventLib.setup>;
Then
let userEvent: UserEvent;
Not a blocker, I'll defer to you.
There was a problem hiding this comment.
Nice. The library even provides an exported type for this: UserEvent!
There was a problem hiding this comment.
Was able to go into an affected test and see options after typing userEvent. into it! Yay code completion~ Added a couple questions just for learning purposes, but this looks pretty straightforward to me :) I'm curious though, why didn't it get a type after it was redeclared?
| @@ -80,7 +80,7 @@ function ConnectedMathInput({keypadConfiguration = defaultConfiguration}) { | |||
| } | |||
|
|
|||
| describe("math input integration", () => { | |||
| let userEvent; | |||
| let userEvent: ReturnType<typeof userEventLib.setup>; | |||
There was a problem hiding this comment.
question, nb: So this means whatever the return type is for the userEventLib.setup function is the type of this userEvent variable, right?
There was a problem hiding this comment.
That's exactly correct.
| await userEvent.click( | ||
| screen.getByRole("button", {name: "Remove image widget"}), | ||
| ); |
There was a problem hiding this comment.
question, nb: Did the syntax that was previously here not work? Or is this change mostly for readability?
There was a problem hiding this comment.
It was incorrect. It looks like deep in the library there's some defaulting to use the previous target as the target when click() is called, but the API types definitely don't allow for a missing element. I think it was sheer luck that this worked.
I just didn't think to! :( |
|
@jeremywiebe Oh, no, I meant, why didn't it automatically happen after this line: |
Ah, that! When we declare |
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (d122224) and published it to npm. You Example: yarn add @khanacademy/perseus@PR1421If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR1421 |
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/math-input@20.0.0 ### Major Changes - [#1064](#1064) [`c6a5cbe13`](c6a5cbe) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - React 18 ### Patch Changes - [#1421](#1421) [`9a3bce37f`](9a3bce3) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Enhance types in tests using @testing-library/user-event - [#1064](#1064) [`c6a5cbe13`](c6a5cbe) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Fix: prevent `react` and `react-dom` from being bundled ## @khanacademy/perseus@26.0.0 ### Major Changes - [#1064](#1064) [`c6a5cbe13`](c6a5cbe) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - React 18 ### Minor Changes - [#1426](#1426) [`4b6fc712e`](4b6fc71) Thanks [@benchristel](https://github.com/benchristel)! - Tweak the animation of the protractor rotation handle. It's now slightly slower and the magnification is less extreme. ### Patch Changes - [#1421](#1421) [`9a3bce37f`](9a3bce3) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Enhance types in tests using @testing-library/user-event - [#1064](#1064) [`c6a5cbe13`](c6a5cbe) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Fix: prevent `react` and `react-dom` from being bundled - Updated dependencies \[[`9a3bce37f`](9a3bce3), [`c6a5cbe13`](c6a5cbe), [`c6a5cbe13`](c6a5cbe), [`c6a5cbe13`](c6a5cbe)]: - @khanacademy/math-input@20.0.0 - @khanacademy/simple-markdown@0.13.0 - @khanacademy/pure-markdown@0.3.7 ## @khanacademy/perseus-editor@9.0.0 ### Major Changes - [#1064](#1064) [`c6a5cbe13`](c6a5cbe) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - React 18 ### Patch Changes - [#1421](#1421) [`9a3bce37f`](9a3bce3) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Enhance types in tests using @testing-library/user-event - Updated dependencies \[[`4b6fc712e`](4b6fc71), [`9a3bce37f`](9a3bce3), [`c6a5cbe13`](c6a5cbe), [`c6a5cbe13`](c6a5cbe)]: - @khanacademy/perseus@26.0.0 - @khanacademy/math-input@20.0.0 ## @khanacademy/simple-markdown@0.13.0 ### Minor Changes - [#1064](#1064) [`c6a5cbe13`](c6a5cbe) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - React 18 - [#1064](#1064) [`c6a5cbe13`](c6a5cbe) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - React 18 ## @khanacademy/pure-markdown@0.3.7 ### Patch Changes - Updated dependencies \[[`c6a5cbe13`](c6a5cbe), [`c6a5cbe13`](c6a5cbe)]: - @khanacademy/simple-markdown@0.13.0 ## @khanacademy/perseus-dev-ui@2.0.0 ### Major Changes - [#1064](#1064) [`c6a5cbe13`](c6a5cbe) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - React 18 ### Patch Changes - Updated dependencies \[[`9a3bce37f`](9a3bce3), [`c6a5cbe13`](c6a5cbe), [`c6a5cbe13`](c6a5cbe), [`c6a5cbe13`](c6a5cbe)]: - @khanacademy/math-input@20.0.0 - @khanacademy/simple-markdown@0.13.0 - @khanacademy/pure-markdown@0.3.7 Author: khan-actions-bot Reviewers: jeremywiebe Required Reviewers: Approved By: jeremywiebe Checks: ✅ Upload Coverage (ubuntu-latest, 20.x), ⏭️ Publish npm snapshot, ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1433
Summary:
When I upgraded Perseus to
user-eventv14, tests that used fake timers needed to initialize an instance of theuser-eventlibrary (because we needed to tell it how to advance the fake timers in Jest).The configured variable was untyped and meant that we don't get any intellisense/code completion when using it, so this PR properly types the
let userEventdeclarations!Before
After
Issue: "none"
Test plan:
Open one of the affected test files and type
userEvent.in one of the tests. Note that your editor (if it supports this) should list the functions available on theuserEventinstance!