chore(noImplicitAny): fix more errors #6161
Conversation
| comments.push(comment) | ||
| const comment = commentLookup[reflection.id as keyof typeof commentLookup] | ||
| if (Array.isArray(comment)) { | ||
| // I think this was a bug before... |
There was a problem hiding this comment.
noImplicitAny caught a bug here
There was a problem hiding this comment.
Curious why the array check is necessary? If reflection.id doesn't exist in commentLookup, comment should be undefined? +1 could elaborate on the comment here
There was a problem hiding this comment.
all the values in commentLookup are arrays. perhaps that was an error? i didn't want to change that though
| @@ -1,5 +1,6 @@ | |||
| import graphql from 'babel-plugin-relay/macro' | |||
| import Parser from 'json2csv/lib/JSON2CSVParser' // only grab the sync parser | |||
| import type {Parser as JSON2CSVParser} from 'json2csv' | |||
There was a problem hiding this comment.
i don't usually import type, but i wanted to be sure we aren't imported the whole package because it's a big one
| @@ -307,14 +307,6 @@ export const enum SubscriptionChannel { | |||
| ORGANIZATION = 'organization' | |||
| } | |||
|
|
|||
| export const enum SuggestedActionTypeEnum { | |||
There was a problem hiding this comment.
we now have a single source of truth for these in the graphql enum
There was a problem hiding this comment.
yeah, single source of truth! the keyof typeof is really helpful when doing TypeScript legacy support.
btw: have we tried any migration tool like ts-migrate before? I'm curious about how it will actually work.
There was a problem hiding this comment.
the last time i checked it out it was still a little buggy & at best it would just explicitly cast things as any as a fallback.
this little exercise probably took the team 30-40 hours to complete, but we uncovered some cool bugs & got some really useful patterns (like keyof typeof) in place so future developers have some good patterns to go off of.
| ) | ||
| // turned into a noop to remove dependency on stripe | ||
| const subscriptions = [] | ||
| // const orgIds = Object.keys(orgs) |
There was a problem hiding this comment.
deleted the stripe helper, which this depends on, so had to remove any logic using it.
in the future, we shouldn't do stripe things in migrations, but this was 6 years ago
| @@ -18,9 +17,9 @@ const headerStyle = { | |||
| paddingTop: 24 | |||
| } | |||
|
|
|||
| const getHeaderText = (meetingType) => { | |||
| const getHeaderText = (meetingType: 'retrospective' | 'action' | 'poker') => { | |||
There was a problem hiding this comment.
shall we use the same way as TSuggestedActionTypeEnum, to use the MeetingTypeEnum type extracted from GraphQLEnumType?
There was a problem hiding this comment.
yep! the relay-compiler gives us those enums in the generated files so we don't have to reference the server from the client so I'll just pull from there.
import {
MeetingMembersWithoutTasks_meeting,
MeetingTypeEnum
} from 'parabol-client/__generated__/MeetingMembersWithoutTasks_meeting.graphql'
| comments.push(comment) | ||
| const comment = commentLookup[reflection.id as keyof typeof commentLookup] | ||
| if (Array.isArray(comment)) { | ||
| // I think this was a bug before... |
There was a problem hiding this comment.
Curious why the array check is necessary? If reflection.id doesn't exist in commentLookup, comment should be undefined? +1 could elaborate on the comment here
| @@ -26,7 +26,7 @@ const innerStyle = { | |||
| width: '100%' | |||
| } as const | |||
|
|
|||
| const EmailBlock = (props) => { | |||
| const EmailBlock = (props: any) => { | |||
There was a problem hiding this comment.
+1 could add types here and remove propTypes?
type Props = {
align?: 'center' | 'left'
children: ReactNode
innerMaxWidth: number
hasBackgroundColor?: boolean
}
const EmailBlock = (props: Props) => {
const {align = 'left', hasBackgroundColor, children, innerMaxWidth} = props
It looks like align and width properties can be removed from <table>
There was a problem hiding this comment.
this component is HTML4 😨
that's the only way to write components so they work in email
packages/client/modules/email/components/SummaryEmail/MeetingSummaryEmail/EmailBorderBottom.tsx
Outdated
Show resolved
Hide resolved
| orgId, | ||
| startAt: fromEpochSeconds(stripeInvoice.period_start), | ||
| startingBalance: stripeInvoice.startingBalance, | ||
| startingBalance: stripeInvoice.starting_balance, |
| {first, queryString, isJQL, projectKeyFilters}, | ||
| {authToken}: GQLContext | ||
| ) => { | ||
| resolve: async ({teamId, userId, accessToken, cloudIds}, args: any, {authToken}) => { |
There was a problem hiding this comment.
I was wondering why you didn't do args: IssueArgs and remove the casting below, but I see it makes the field type incompatible. Annoying that you can't define the args type in a GraphQLObjectType
Migration is many years old and the used stripe manager was deleted in PR #6161.
Migration is many years old and the used stripe manager was deleted in PR #6161.
fix #5831
notes: