-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
chore: fix any type #255
chore: fix any type #255
Conversation
으워~~ 엄청 고생 많이 했네요. 수고하셨어요. 리뷰는 수요일에 볼게요. |
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.
[01/04] 수고하셨습니다. 리뷰 완료했어요.
함수 파라미터 중에 불필요하게 optional인 부분들은 전반적으로 체크 부탁 드립니다.
index.d.ts
Outdated
@@ -1,22 +1,40 @@ | |||
// Type definitions for TOAST UI Calendar v1.9.0 | |||
// TypeScript Version: 3.2.1 | |||
|
|||
type templateFunc = (...args: Array<any>) => string; | |||
type DateType = string | TZDate | Date; | |||
type EventHandlerFunc = (event: any) => void; |
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.
이벤트 핸들러의 파라미터도 타입을 지정해야 하는거 아닌지 확인 부탁 드립니다. 다른 쪽은 어떻게 했어요?
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.
네, 수정하겠습니다.
index.d.ts
Outdated
color: string; | ||
day: number; | ||
label: string; | ||
left: number; |
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.
color, left, width는 인터페이스로 내보내지 않아도 될것 같습니다.
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.
넵, 확인했습니다.
index.d.ts
Outdated
isToday: boolean; | ||
left: number; | ||
renderDate: string; | ||
width: number; |
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.
color, left, width는 인터페이스로 내보내지 않아도 될 것 같습니다.
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.
넵 확인했습니다.
index.d.ts
Outdated
hiddenSchedules: number; | ||
isOtherMonth: boolean; | ||
isToday: boolean; | ||
left: number; |
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.
backgroundColor, color, left, width는 인터페이스로 내보내지 않아도 될 것 같습니다.
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.
넵, 확인했습니다~~!
index.d.ts
Outdated
timegridDisplayPrimayTime?: templateFunc | ||
timegridDisplayTime?: templateFunc | ||
interface ITimeGridHourLabel { | ||
color: 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.
color, fontWeight는 인터페이스로 내보내지 않아도 될 것 같습니다.
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.
네, 확인했습니다.
index.d.ts
Outdated
comingDuration?: (schedule?: ISchedule) => string; | ||
monthMoreTitleDate?: (date?: string, dayname?: string) => string; | ||
monthMoreClose?: () => string; | ||
monthGridHeader?: (model?: IGridDateModel) => 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.
파라미터가 optional이 아니니까 물음표 빼야 되지 않을까요?
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.
네, 수정하겠습니다.
index.d.ts
Outdated
width: number; | ||
} | ||
|
||
interface ITemplateConfig { |
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.
옵셔널 파라라미터 한번 확인해 주세요
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.
helper.js
를 참고하여 수정하겠습니다.
index.d.ts
Outdated
startDayOfWeek?: number; | ||
daynames?: Array<string>; | ||
daynames?: 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.
이건 무슨 차이인가요? 컨벤션인가요?
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.
네~ tslint에서 Array<string>
=> string[]
로 심플하게 표현하라고 하더라구요~!
index.d.ts
Outdated
} | ||
}; | ||
scheduleFilter?: (...args:Array<any>) => any; | ||
scheduleFilter?: (schedule: ISchedule | null) => 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.
파라미터가 null이 올 수 없을 것 같아요
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.
null
타입 삭제하겠습니다.
index.d.ts
Outdated
@@ -237,17 +300,17 @@ interface Schedule { | |||
dragBgColor?: string; | |||
borderColor?: string; | |||
customStyle?: string; | |||
raw?: any; | |||
raw?: IRaw | null; |
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.
raw는 any로 가도 됩니다. 그야 말로 any용도입니다. IRaw도 빼도 될것 같아요.
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.
넵, 알겠습니다.
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.
@dongsik-yoo 피드백 내용 적용하였습니다.
추가적으로 type 선언이이 글로벌하게 사용되더라구요.
모듈(네임스페이스)안으로 이동하여 독립적이로 선언된 타입을 사용하도록 수정하였습니다.
index.d.ts
Outdated
timegridDisplayPrimayTime?: templateFunc | ||
timegridDisplayTime?: templateFunc | ||
interface ITimeGridHourLabel { | ||
color: 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.
네, 확인했습니다.
index.d.ts
Outdated
hiddenSchedules: number; | ||
isOtherMonth: boolean; | ||
isToday: boolean; | ||
left: number; |
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.
넵, 확인했습니다~~!
index.d.ts
Outdated
isToday: boolean; | ||
left: number; | ||
renderDate: string; | ||
width: number; |
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.
넵 확인했습니다.
index.d.ts
Outdated
color: string; | ||
day: number; | ||
label: string; | ||
left: number; |
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.
넵, 확인했습니다.
index.d.ts
Outdated
startDayOfWeek?: number; | ||
daynames?: Array<string>; | ||
daynames?: 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.
네~ tslint에서 Array<string>
=> string[]
로 심플하게 표현하라고 하더라구요~!
index.d.ts
Outdated
} | ||
}; | ||
scheduleFilter?: (...args:Array<any>) => any; | ||
scheduleFilter?: (schedule: ISchedule | null) => 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.
null
타입 삭제하겠습니다.
index.d.ts
Outdated
@@ -237,17 +300,17 @@ interface Schedule { | |||
dragBgColor?: string; | |||
borderColor?: string; | |||
customStyle?: string; | |||
raw?: any; | |||
raw?: IRaw | null; |
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.
넵, 알겠습니다.
index.d.ts
Outdated
@@ -1,22 +1,40 @@ | |||
// Type definitions for TOAST UI Calendar v1.9.0 | |||
// TypeScript Version: 3.2.1 | |||
|
|||
type templateFunc = (...args: Array<any>) => string; | |||
type DateType = string | TZDate | Date; | |||
type EventHandlerFunc = (event: any) => void; |
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.
네, 수정하겠습니다.
index.d.ts
Outdated
width: number; | ||
} | ||
|
||
interface ITemplateConfig { |
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.
helper.js
를 참고하여 수정하겠습니다.
index.d.ts
Outdated
comingDuration?: (schedule?: ISchedule) => string; | ||
monthMoreTitleDate?: (date?: string, dayname?: string) => string; | ||
monthMoreClose?: () => string; | ||
monthGridHeader?: (model?: IGridDateModel) => 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.
네, 수정하겠습니다.
@jungeun-cho 수고 많으셨습니다. |
Please check if the PR fulfills these requirements
fix #xxx[,#xxx]
, where "xxx" is the issue number)Description
Thank you for your contribution to TOAST UI product. 🎉 😘 ✨