-
Notifications
You must be signed in to change notification settings - Fork 598
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Migrate utils to TypeScript #1055
Conversation
🦋 Changeset detectedLatest commit: b7904f6 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/j45rqy8iz |
96112c4
to
7ac0956
Compare
@@ -1,16 +1,17 @@ | |||
/* eslint-disable no-console */ | |||
import {useRef, useCallback} from 'react' | |||
declare var __DEV__: boolean |
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.
Is there a better place for this?
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.
Not sure. I think this is fine for now 👍
@@ -96,7 +96,7 @@ export function renderStyles(node: React.ReactElement): any { | |||
return getComputedStyles(className) | |||
} | |||
|
|||
export function getComputedStyles(className: string): Record<string, string> { | |||
export function getComputedStyles(className: string): Record<string, string | Record<string, string>> { |
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.
This feels weird, but was required to make L80 of src/utils/test-matchers.tsx
play nice. My understanding of Record<string, string>
is that is types and object that maps strings to strings - but it seems that sometimes this will map strings to object groupings that are themselves Record<string, string>
, as in the case of the media query key invoked on L80.
Though, I might be misunderstanding this 😅
src/utils/testing.tsx
Outdated
Component: React.FunctionComponent<any> | ||
systemPropArray: any[] | ||
toRender?: () => React.ReactElement |
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 think my linter did this and some of the other changes 🤔
getting an error for I haven't made a declaration file for a non-typescript package yet, looking into a tutorial on how to do this... |
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.
Nice 🎉
This PR migrates the
utils
folder to TypeScript as part of the TypeScript refactor.