refactor(apply): migrate common modules to typescript #644

lokba · 2022-09-25T14:02:49Z

Resolves #594

해결하려는 문제가 무엇인가요?

공통으로 사용되는 컴포넌트(@common)를 타입스크립트로 마이그레이션한다.

어떻게 해결했나요?

1. props의 순서를 일관성있게 배치하였습니다.

기존에 component, storybook의 props의 순서가 일관되지 못했습니다.
props 순서를 코드에서 등장하는 순서대로 일관성있게 수정하였습니다.

2. defaultProps -> Object default values

before

const Button = ({ className, variant, cancel, type, children, ...props }) => { ... };

Button.defaultProps = {
  variant: BUTTON_VARIANT.CONTAINED,
  cancel: false,
};

after

const Button = ({
  className,
  variant = "contained",
  cancel = false,
  type,
  children,
  ...props
}: ButtonProps) => { ... };

defaultProps -> Object default values 형식으로 변환했습니다.
dan_abramov에 따르면, defaultProps가 deprecated될 예정이라고 나와있습니다.
- dan_abramov 관련 자료

3. props 타입의 옵셔널 기준

common 폴더 하위에 있는 컴포넌트의 props에 타입을 명시할 때, 어떤 기준으로 optional을 부여할지 고민을 많이 했습니다. 결론적으로 아래 사진과 같은 순서도로 기준을 잡았습니다.

4. Storybook(MessageTextInput, MessageTextarea, TextInput) 에러 수정

기존에 Storybook에서 MessageTextInput, MessageTextarea, TextInput에 값을 입력시 아래와 같은 에러가 발생했습니다.

ts로 마이그레이션 작업하면서, onChange를 props로 넘겨서 에러 해결하였습니다.

어떤 부분에 집중하여 리뷰해야 할까요?

타입이 적절하게 제한되었는지 리뷰해주세요.
현재의 컨벤션에서 개선되거나 수정할 만한 부분이 존재하는지 리뷰해주세요.

RCA 룰

r: 꼭 반영해 주세요. 적극적으로 고려해 주세요. (Request changes)
c: 웬만하면 반영해 주세요. (Comment)
a: 반영해도 좋고 넘어가도 좋습니다. 그냥 사소한 의견입니다. (Approve)

soyi47

안녕하세요, 록바!

구현하면서 세웠던 기준들을 깔끔하고 자세히 공유해주셔서 읽기 좋았어요.
스토리북 에러도 잘 챙겨주셨네요. 고생하셨습니다. 👍

타입 관련 리뷰 소소하게 남겼으니 확인해주세요~

props 순서에 대한 의견

기존에 component, storybook의 props의 순서가 일관되지 못했습니다.
props 순서를 코드에서 등장하는 순서대로 일관성 있게 수정하였습니다.

props 순서에 규칙이 있는 건 정말 좋습니다! 그렇지만 '코드 등장 순서'가 그 규칙으로 적절한지는 고민이 되네요. 저는 컴포넌트 내에서의 중요도나 유사성을 기준으로 모아두려고 하는 편인데요. 정확히 기준을 제시하긴 어려워서 조금 더 생각해보고 의견 남기겠습니다.

soyi47 · 2022-09-27T16:07:35Z

frontend/src/components/@common/MessageTextInput/MessageTextInput.tsx

+  name: string;
+  maxLength?: number;
+  errorMessage?: string;
+  onChange: (e: React.ChangeEvent<HTMLInputElement>) => void;


c:
요렇게도 가능합니다. 저는 아래와 같이 작성하는 게 편하던데 어떠신가요?

Suggested change

onChange: (e: React.ChangeEvent<HTMLInputElement>) => void;

onChange: React.ChangeEventHandler<HTMLInputElement>;

음....저는 함수의 매개변수와 반환 타입을 명시적으로 표시하는 것이 좋다고 생각해서 기존 방식을 유지하긴 했습니다.
다른 분들의 의견도 들어보고 맞춰보도록 하겠습니다.

저는 특별한 이유는 없고 개인적으로 리액트에서 제공하는 타입들을 쓰려고하는 편이라 React.ChangeEventHandler를 선호합니다.

반영했습니다. fa46bd7

soyi47 · 2022-09-27T16:14:16Z

frontend/src/components/@common/TextInput/TextInput.tsx

+export type TextInputProps = React.InputHTMLAttributes<HTMLInputElement> & {
+  maxLength?: number;
+  type?: "text" | "email" | "password" | "tel" | "number" | "url";
+  value?: string;
+  className?: string;
+  readOnly?: boolean;
+  onChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
+};


r:
className 외에는 이미 React.InputHTMLAttributes에 다 정의되어 있는 속성들 같은데, intersection 연산으로 다시 정의한 이유가 있나요?
Button 컴포넌트의 Props 타입도 그렇고, 위와 같은 상황들이 몇 군데 있는 것 같아요. 이유가 있는지 궁금합니다.

좋은 질문이네요. 저도 고민한 부분인데요.

const TextInput = ({ maxLength, type = "text", value = "", className = "", readOnly = false, onChange, ...props }: TextInputProps) => { ... }

위와 같이 TextInput 컴포넌트의 props 형식과 동일하게 가져가고 싶었습니다. 상황을 예시로 들어보겠습니다. A라는 개발자가 maxLength의 타입이 궁금해졌습니다. 이 경우 자연스럽게 TextInputProps로 눈을 돌리게 되고, maxLength의 타입을 확인합니다. 해당 상황에서 React.InputHTMLAttributes을 타고 들어가서 확인하는게 번거롭다고 생각했습니다. 어떻게 생각하시나요?

타입이 궁금한 prop이 있다면 Props 타입 정의를 다시 확인하기보다 해당 prop에 커서를 올려 확인하는 게 더 빠르다고 생각합니다. 그 이상으로 구체적인 타입 정의를 확인하고 싶으면 타고 들어가는 게 맞을 것 같고요.

무엇보다 타입을 새로 정의 할 때 & 연산으로 속성 간에 타입 충돌이 있으면, 타입을 적용할 때가 되어야 에러가 나타나 타입 충돌 문제를 알 수 있어요.
아래 코드를 playground에서 실행시켜보시면 무슨 말인지 이해가 쉬울 겁니다.

type A = { value?: string; maxLength?: number; }; type B = { value?: number; maxLength?: number; }; type C = A & B; const test: C = { value: "test value!", maxLength: 5, };

그래서 저는 & 연산 과정에서 두 타입 내에 같은 이름의 속성이 있을 때는 휴면 에러가 발생하기 쉽고 특별히 주의해야 한다고 생각합니다.
록바가 위와 같은 문제가 없도록 신경써서 잘 작성해주셨을 것 같지만 누구나 실수를 만들기 쉬운 부분인 것 같습니다. 아마 요 공통 컴포넌트들을 사용하는 컴포넌트까지 마이그레이션을 시작하면 에러가 나타나거나, 예상했던 것과 다른 속성 타입이 나타날 수도 있을 것 같아요.
이왕 제공되는 타입을 쓸 거라면 그대로 잘 활용해서 쓰는 게 일관성 있고 안전한 길이라고 봅니다.

소피아가 말해주신 2번(타입 충돌 가능성)때문에 React.InputHTMLAttributes에 정의된 속성은 배제하도록 하겠습니다.

소피아에 의견에 동의합니다. 재정의하는 것 자체가 go to definition 만큼 번거로울 것 같아요.

React.~~~HTMLAttributes<T>에 정의된 속성의 타입과 동일한 구조의 타입일 때만 삭제하였습니다. 반영했습니다. b7dc136 0d9873c

상세 설명 : 완전히 타입이 동일한 경우에만 삭제하였습니다

greenblues1190

File changed가 어마어마하네요..! 코멘트 달았습니다.

greenblues1190 · 2022-10-03T10:07:11Z

frontend/src/components/@common/Button/Button.stories.tsx


-const Template = (args) => <Button {...args} />;
+const Template: ComponentStory<typeof Button> = (args: ButtonProps) => <Button {...args} />;


c: ComponentStory 타입에서 args의 타입을 추론해주던데 인자 타입을 직접 지정할 필요가 있을까요?

Suggested change

const Template: ComponentStory<typeof Button> = (args: ButtonProps) => <Button {...args} />;

const Template: ComponentStory<typeof Button> = (args) => <Button {...args} />;

반영했습니다. dec6003

greenblues1190 · 2022-10-03T10:45:47Z

frontend/src/components/@common/Button/Button.tsx

+
+const Button = ({
+  className,
+  variant = "contained",


r: BUTTON_VARIANT에서 참조하지 않은 이유가 있나요?

반영했습니다. f0bba00

greenblues1190 · 2022-10-03T10:47:26Z

frontend/src/components/@common/Button/Button.tsx

+const Button = ({
+  className,
+  variant = "contained",
+  cancel = false,
+  type,
+  children,
+  ...props
+}: ButtonProps) => {
+  return (
+    <button
+      className={classNames(className, styles[variant], styles.button, {
+        [styles.cancel]: cancel,
+      })}
+      type={type}
+      {...props}
+    >
+      {children}
+    </button>
+  );
+};


c: return 키워드와 type, children 바인딩을 생략하면 코드가 간단해지네요.

Suggested change

const Button = ({

className,

variant = "contained",

cancel = false,

type,

children,

...props

}: ButtonProps) => {

return (

<button

className={classNames(className, styles[variant], styles.button, {

[styles.cancel]: cancel,

})}

type={type}

{...props}

>

{children}

</button>

);

};

const Button = ({

className,

variant = "contained",

cancel = false,

...props

}: ButtonProps) => (

<button

className={classNames(className, styles[variant], styles.button, {

[styles.cancel]: cancel,

})}

{...props}

/>

);

return, children은 존재하는 것이 명시적이라고 판단하여, type 바인딩만 생략하였습니다. 반영했습니다. 420cfd1

greenblues1190 · 2022-10-04T06:08:12Z

frontend/src/components/@common/MessageTextInput/MessageTextInput.tsx

+  name: string;
+  maxLength?: number;
+  errorMessage?: string;
+  onChange: (e: React.ChangeEvent<HTMLInputElement>) => void;


저는 특별한 이유는 없고 개인적으로 리액트에서 제공하는 타입들을 쓰려고하는 편이라 React.ChangeEventHandler를 선호합니다.

greenblues1190 · 2022-10-04T06:14:02Z

frontend/src/components/@common/TextInput/TextInput.tsx

+export type TextInputProps = React.InputHTMLAttributes<HTMLInputElement> & {
+  maxLength?: number;
+  type?: "text" | "email" | "password" | "tel" | "number" | "url";
+  value?: string;
+  className?: string;
+  readOnly?: boolean;
+  onChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
+};


소피아에 의견에 동의합니다. 재정의하는 것 자체가 go to definition 만큼 번거로울 것 같아요.

greenblues1190

👍

lokba · 2022-10-05T11:40:12Z

공통 컴포넌트를 import하는 모든 파일(js -> tsx)을 변환한 후, 테스트 해본 결과 이상 없습니다.
변환하는데 도움을 주신 @hwangstar156 @greenblues1190 에게 감사를 전합니다.

woowapark

소소하게 코멘트 남겨두었습니다 🙂 확인 및 의견 부탁드려요~!

woowapark · 2022-10-05T14:05:57Z

frontend/src/components/@common/IconButton/IconButton.tsx

+const IconButton = ({ className, src, alt, ...props }: IconButtonProps) => {
+  return (
+    <button className={styles.button} {...props}>
+      <img src={src} alt={alt} className={className} />


r: alt가 optional 이지만 기본값을 따로 주지 않은 건 의도하신 부분일까요~? (아래 <Label>의 'htmlFor'도 동일합니다!

공원 반갑습니다. 의도한 부분이 맞습니다.
현재 프로젝트에서 IconButton의 사용처는 Panel 컴포넌트입니다. 코드부터 한번 보실까요?

<IconButton src={isPanelOpen ? "/assets/icon/arrow-up.svg" : "/assets/icon/arrow-down.svg"} aria-label={isPanelOpen ? "패널 닫기" : "패널 열기"} />

IconButton에 alt prop을 넘기지 않고 있습니다. 이 때문에 alt을 optional로 타입을 정했습니다. 왜 기본값을 안줬냐? 우선, 기존에 js파일에서도 defaultProps를 주지 않은 상태였고, 대체 텍스트이다 보니 기본값으로 ''(빈 문자열)을 주는 것이 맞는가 의문이 들어 기본값을 주지 않았습니다.

근데 최근에 접근성 미션도 진행했고, 이미지에 대체 텍스트가 없는 것은 접근성을 떨어뜨리는 코드이기에 alt를 optional하지 않게 수정하겠습니다. 그에 따라 사용처에서도 alt에 대한 텍스트를 주입하겠습니다.

수정했습니다. caab7a1 31d78eb

woowapark · 2022-10-05T14:26:03Z

frontend/src/components/@common/Label/Label.tsx

+const Label = ({ htmlFor, className = "", required = false, children }: LabelProps) => {
+  return (
+    <label
+      htmlFor={htmlFor}


c: 의미상으로 보자면

htmlFor가 항상 있거나

생략되는 경우 children에 라벨링 될 대상이 들어와야 할 것 같은데요.

기존에 <Label>을 사용하는 코드들에서 사용방식이 조금씩 다른 상태네요 😅
한번 맞춰봐도 좋을 것 같은데 록바 의견은 어떠신가요?
TS 전환 범위를 다소 벗어나는 수정이 늘어날 것 같아 만약 수정이 필요하더라도 이 PR에서는 하지 않는 게 좋을 것 같아요!
이슈 사항 확인 및 질문용으로 코멘트 남겨둡니다 🙏

기존에는 htmlFor 속성이 없었는데, Label 컴포넌트의 사용처들을 tsx로 변환하는 과정에서 BirthField 컴포넌트에서 htmlFor 속성을 주입하고 있더라고요. 그리하여, htmlFor prop이 추가되었습니다.

// BirthField.js <Label htmlFor="birthday" required={required} className={styles.label}> 생년월일 </Label>

기존에 을 사용하는 코드들에서 사용방식이 조금씩 다른 상태네요 😅

저도 공감합니다. 사실 Label 뿐만이 아니라...다른 곳도 모호한 부분이 있긴 하죠... 추후에 시간이 여유로울때 작업해도 좋을 것 같습니다.😎

woowapark · 2022-10-05T14:39:38Z

frontend/src/components/@common/MessageTextInput/MessageTextInput.tsx

+export type MessageTextInputProps = React.InputHTMLAttributes<HTMLInputElement> & {
+  type?: "text" | "email" | "password" | "tel" | "number" | "url";
+  label?: string;
+  description?: JSX.Element | string;


r:

React.ReactNode와 JSX.Element 중에 선택하신 기준이 궁금합니다!

컴포넌트 사용처들을 보았는데 description을 string 으로 주입해서 사용하는 경우는 현재 없는 것 같더라고요. 요건 아마 기존 코드에서 defaultProps 가 빈 문자열이어서 이렇게 해주셨을 것 같은데요. 대부분 description을 거의 사용하지 않고, 예외적으로 ApplicationRegister에서만 아예 예외적으로 노드를 통으로 주입해서 사용하는 것 같아 string 타입을 제외시켜도 문제는 없을 것 같은데 어떠신가요? (물론 이렇게 되면 description이라는 이름이 다소 어색해 보일 수는 있겠네요 😅 요 부분까지 포함해서 의견이 궁금합니다!)

React.ReactNode와 JSX.Element 중에 선택하신 기준이 궁금합니다!

//ReactNode type ReactNode = ReactElement | string | number | ReactFragment | ReactPortal | boolean | null | undefined; //JSX.Element namespace JSX { interface Element extends React.ReactElement<any, any> { }

description으로 들어오는 값이 ReactElement(ApplicationRegister), String(defaultProps)으로 판단되어서 ReactNode보다는 JSX.Element가 타입이 더 좁기 때문에 JSX.Element를 사용하였습니다.

여기서 추가적으로 JSX.Element vs ReactElement를 고민하긴 했는데, 변수명 서치 사이트에서 JSX.Element가 압도적으로 많이 나와서 최종적으로 JSX.Element를 선택하였습니다.

컴포넌트 사용처들을 보았는데 description을 string 으로 주입해서 사용하는 경우는 현재 없는 것 같더라고요. 요건 아마 기존 코드에서 defaultProps 가 빈 문자열이어서 이렇게 해주셨을 것 같은데요. 대부분 description을 거의 사용하지 않고, 예외적으로 ApplicationRegister에서만 아예 예외적으로 노드를 통으로 주입해서 사용하는 것 같아 string 타입을 제외시켜도 문제는 없을 것 같은데 어떠신가요? (물론 이렇게 되면 description이라는 이름이 다소 어색해 보일 수는 있겠네요 😅 요 부분까지 포함해서 의견이 궁금합니다!)

와우...정말 꼼꼼하게 다 보셨네요. 제가 놓친 부분입니다. string 타입을 제외시키도록 하겠습니다. description 네이밍은 기존대로 가는게 좋지 않을까 싶습니다. 아래 코드처럼, Description 컴포넌트의 children으로 description prop을 주고 있기 때문에 네이밍이 변경되면 오히려 헷갈릴 것 같습니다.

const MessageTextInput = ({ ... description, ...props }: MessageTextInputProps) => { return ( <div className={classNames(styles.box, className)}> ... {description && <Description className={styles.description}>{description}</Description>} ... </div> </div> ); };

반영했습니다. 1c051a3

woowapark · 2022-10-05T14:46:55Z

frontend/src/components/@common/Textarea/Textarea.tsx

+export type TextareaProps = React.TextareaHTMLAttributes<HTMLTextAreaElement> & {
+  value?: string;
+  onChange: React.ChangeEventHandler<HTMLTextAreaElement>;
+};


c: readonly는 value와 똑같이 따로 정의해주면 어떨까요?

결국에는 네이티브 textarea에서 자체적으로 지원하는 property와 같은 값이겠지만, 이 컴포넌트에서 주입받는 props는 네이티브의 속성이 아니라 '컴포넌트의' 옵션 값을 제어하기 위한 것이라고 생각해서요!

수정될 가능성이 크진 않겠지만, 만약 아래의 기본값 선언에서 readOnly의 이름을 바꾼다거나 하면 React.TextareaHTMLAttributes<HTMLTextAreaElement>에 속하지 않게 되어 타입이 깨질 수 있을 것 같습니다.

음...말씀해주신 상황이 잘 이해가 안되네요.😂

데일리에서 다시 얘기해보시죠~! 🙂
요건 그냥 의견 및 질문으로 남긴 코멘트라 PR은 approve할게요!

hwangstar156 · 2022-10-06T08:32:47Z

frontend/src/components/@common/Container/Container.tsx

  return (
    <div
      className={classNames(
        styles.box,
        { [styles.narrow]: size === CONTAINER_SIZE.NARROW },
        className
      )}
-      {...props}
+      onClick={onClick ?? (() => {})}


c: onClick={onClick} 이 훨씬 보기 좋지않을까요? undefined여도 동작은 문제없이 될것 같네요.

반영했습니다. 511eea4

hwangstar156 · 2022-10-06T08:35:16Z

frontend/src/components/@common/Label/Label.tsx

+        [styles.required]: required,
+      })}
+    >
+      {children}


c: 이 children이 정확히 어떤것이 들어가는지 확인한후 타입을 좁히는것이 좋을것 같습니다. JSX.Element 혹은 string으로도 줄일 수 있지 않을까 싶네요.

의견 감사합니다. 현재 Label의 children으로는 JSX.Element와 string 타입이 옵니다. 해당 Label의 children 타입을 내로잉 작업을 진행하면, 공통 컴포넌트 전반적으로 children 타입을 좁히는 작업까지 해야할 것 같은 생각이 들어서요. 우선은 ReactNode로 유지하는 것이 좋을 것 같습니다.

록바 의견에 동의합니다~

hwangstar156 · 2022-10-06T08:39:57Z

frontend/src/components/@common/Panel/Panel.tsx

+  children: React.ReactNode;
+};
+
+const Panel = ({ isOpen = false, className, title, children }: PanelProps) => {


c: 여기 사용되는 chilldren도 더 줄일수 있는지 확인해보면 좋을것 같네요.

위에 남긴 답변과 동일한 생각입니다. #644 (comment)

hwangstar156 · 2022-10-06T08:41:44Z

frontend/src/components/@common/TextInput/TextInput.tsx

+import styles from "./TextInput.module.css";
+
+export type TextInputProps = React.InputHTMLAttributes<HTMLInputElement> & {
+  type?: "text" | "email" | "password" | "tel" | "number" | "url";


r: MessageTextInput과 정확히 같은 타입이 사용되고 있네요. 재사용성을 고려하여서 type alias로 빼도 좋을것 같습니다.

의견 감사합니다. 타입이 중복적으로 선언된 상태이지만, 분리까지 할 필요가 있나 생각이 들긴 합니다. 아직까지 두 군데에서만 동일한 타입이 호출이 되고, 추후에 다른 곳에서도 사용이 된다면 분리하는 것을 고려해보겠습니다. :)

soyi47

록바 반영해주신 거 확인했습니다! 고생하셨어요. 👍

woowapark

수정 사항 확인하였습니다 🙏
고생하셨어요~! 👏👏👏👏👏

(cherry picked from commit cc4065c)

(cherry picked from commit 048e83a)

(cherry picked from commit fa46bd7)

(cherry picked from commit dec6003)

(cherry picked from commit f0bba00)

(cherry picked from commit b7dc136)

- Button - IconButton (cherry picked from commit 420cfd1)

…s possible

- import * as styles -> import styles

- TextInput - MessageTextInput

- JSX.Element | string -> JSX.Element

woowahan-pjs

고생하셨습니다. 👍

lokba requested review from jho2301, greenblues1190, SunYoungKwon, soyi47, KangYunHo1221, woowapark and hwangstar156 September 25, 2022 14:02

lokba self-assigned this Sep 25, 2022

lokba changed the title ~~refactor: migrate common modules to typescript~~ refactor: migrate common file to typescript Sep 25, 2022

soyi47 suggested changes Sep 27, 2022

View reviewed changes

greenblues1190 reviewed Oct 4, 2022

View reviewed changes

lokba force-pushed the feature/typescript-common branch from 420cfd1 to a739082 Compare October 4, 2022 12:01

greenblues1190 approved these changes Oct 5, 2022

View reviewed changes

lokba requested a review from soyi47 October 5, 2022 11:40

woowapark mentioned this pull request Oct 5, 2022

feat(apply): unify the common style of all pages #645

Merged

woowapark requested changes Oct 5, 2022

View reviewed changes

lokba mentioned this pull request Oct 5, 2022

feat(apply): implement the function to preview the application form before final submission #649

Merged

hwangstar156 reviewed Oct 6, 2022

View reviewed changes

lokba requested a review from woowapark October 6, 2022 11:15

soyi47 approved these changes Oct 6, 2022

View reviewed changes

woowapark approved these changes Oct 7, 2022

View reviewed changes

lokba force-pushed the feature/typescript-common branch from dd8348e to b1d13a7 Compare October 8, 2022 09:18

lokba added 7 commits October 9, 2022 14:09

refactor: migrate common files to ts

ec94e3e

(cherry picked from commit cc4065c)

refactor: remove unnecessary code and apply consistently

7ed1f95

(cherry picked from commit 048e83a)

refactor: change event handler funnction type

eb29cd7

(cherry picked from commit fa46bd7)

refactor: delete type when type inference is possible

9479117

(cherry picked from commit dec6003)

refactor: apply constant for human error prevention

29c0860

(cherry picked from commit f0bba00)

refactor: remove duplicated type

4e4f183

(cherry picked from commit b7dc136)

refactor: remove type attribute and use ...props instead

3041df8

- Button - IconButton (cherry picked from commit 420cfd1)

lokba added 17 commits October 9, 2022 14:09

refactor: TabItem - change type and props position

5e3c08b

refactor: StatusIndicator - change type and props position

7d2940e

refactor: StatusIndicator.stories - delete type when type inference i…

bdb618d

…s possible

refactor: remove duplicated type

067111c

refactor: StatusIndicator - remove spread operator(props)

ceb63b7

refactor: Container - remove spread operator(props)

47cfd24

feat: Label - add htmlFor prop

f126937

refactor: apply css import declaration consistently

693bf98

- import * as styles -> import styles

feat: enhance stability by adding a type

014f5da

- TextInput - MessageTextInput

feat: IconButton - change alt prop type

c8e9b6e

feat: Panel - IconButton 컴포넌트에 alt 속성 추가

294ecf1

refactor: MessageTextInput - change description prop type

9a18009

- JSX.Element | string -> JSX.Element

refactor: Container - add onClick defaultProps

d38503e

refactor: StatusIndicator - add optional type on className prop

68c9a8b

refactor: MessageTextInput.stories - change description value

04858aa

fix: conflict 해결

f0956f2

fix: conflict 해결

3a95809

lokba force-pushed the feature/typescript-common branch from 88cbc1f to 3a95809 Compare October 9, 2022 05:10

refactor: Container - add optional type on titleAlign prop

c967a73

woowahan-pjs approved these changes Oct 10, 2022

View reviewed changes

woowahan-pjs changed the title ~~refactor: migrate common file to typescript~~ refactor(apply): migrate common modules to typescript Oct 10, 2022

woowahan-pjs merged commit 2689936 into develop Oct 10, 2022

woowahan-pjs deleted the feature/typescript-common branch October 10, 2022 09:34

	onChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
	onChange: React.ChangeEventHandler<HTMLInputElement>;


		const Template = (args) => <Button {...args} />;
		const Template: ComponentStory<typeof Button> = (args: ButtonProps) => <Button {...args} />;

refactor(apply): migrate common modules to typescript #644

refactor(apply): migrate common modules to typescript #644

Conversation

lokba commented Sep 25, 2022 • edited Loading

해결하려는 문제가 무엇인가요?

어떻게 해결했나요?

1. props의 순서를 일관성있게 배치하였습니다.

2. defaultProps -> Object default values

before

after

3. props 타입의 옵셔널 기준

4. Storybook(MessageTextInput, MessageTextarea, TextInput) 에러 수정

어떤 부분에 집중하여 리뷰해야 할까요?

RCA 룰

soyi47 left a comment

Choose a reason for hiding this comment

props 순서에 대한 의견

Choose a reason for hiding this comment

lokba Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lokba Oct 4, 2022 • edited Loading

Choose a reason for hiding this comment

greenblues1190 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lokba Oct 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greenblues1190 left a comment

Choose a reason for hiding this comment

lokba commented Oct 5, 2022 • edited Loading

woowapark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lokba Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lokba Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soyi47 left a comment

Choose a reason for hiding this comment

woowapark left a comment

Choose a reason for hiding this comment

woowahan-pjs left a comment

Choose a reason for hiding this comment

lokba commented Sep 25, 2022 •

edited

Loading

lokba Sep 27, 2022 •

edited

Loading

lokba Oct 4, 2022 •

edited

Loading

lokba Oct 4, 2022 •

edited

Loading

lokba commented Oct 5, 2022 •

edited

Loading

lokba Oct 6, 2022 •

edited

Loading

lokba Oct 6, 2022 •

edited

Loading