Upstream useSafeTimeout & add tests #1041
Conversation
🦋 Changeset detectedLatest commit: 0a392a0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/primer/primer-components/q1oh7cujf |
…o upstream-behaviors
| @@ -0,0 +1,33 @@ | |||
| import {useCallback, useEffect, useRef} from 'react' | |||
|
|
|||
| type SetTimeout = (handler: TimerHandler, timeout?: number, ...args: any[]) => number | |||
There was a problem hiding this comment.
Same suggestion about using typeof window.setTimeout as above.
There was a problem hiding this comment.
I tried that, but there are some required properties on the native type that doesn't make sense to pass in manually to these wrapper functions:
Property '__promisify__' is missing in type '(handler: TimerHandler, timeout?: number | undefined, ...args: any[]) => number' but required in type 'typeof setTimeout'.ts(2322)
There was a problem hiding this comment.
Oh weird! Thanks for checking.
| import useSafeTimeout from '../hooks/useSafeTimeout' | ||
|
|
||
| test('should call callback after time', async () => { | ||
| const { result, waitFor } = renderHook(() => useSafeTimeout()) |
There was a problem hiding this comment.
Is there a reason for not using fake timers on this test?
Co-authored-by: Trevor Gau <t-hugs@github.com>
|
Note: I had to do an extensive refactoring of our tests to get them to work nicely with typescript after adding the |
Co-authored-by: Cole Bemis <colebemis@github.com>
Co-authored-by: Cole Bemis <colebemis@github.com>
Co-authored-by: Cole Bemis <colebemis@github.com>
Co-authored-by: Cole Bemis <colebemis@github.com>
Co-authored-by: Cole Bemis <colebemis@github.com>
This PR upstreams the
useSafeTimeoutshook and adds some simple tests.I've also added documentation, including adding a new
Hookssubsection to the documentation site 🎉 We might want to rethink how we structure the sidebar later but I think this is fine for now.Related issue: #1013
Merge checklist
index.d.ts) if necessaryTake a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.