Skip to content
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

나무 1주차 #32

Merged
merged 8 commits into from
Mar 31, 2022
Merged

나무 1주차 #32

merged 8 commits into from
Mar 31, 2022

Conversation

wooooooood
Copy link
Member

@wooooooood wooooooood commented Mar 19, 2022

🔎 개요

  • 나무] 구조 정리 #31

  • 함수형으로 리팩토링을 진행하고 있습니다

  • typescript 변환이 쉬운 일부 코드는 ts로 변경했습니다

    • 다른 코드는 좀 더 이해가 필요할것 같아 보류중입니다 ㅎㅎ;

📝 진행상황

  • 커밋을 목적별로 잘 분리하기 ing
  • 페이지 분리
  • 내 컨벤션 적용하기 내 prettier
  • 폴더링 ing
  • js -> ts 변경 ing
  • 상수 분리 및 함수 유틸화

🖼 스크린샷

💬 의견

바닐라가 낯설어서 리팩토링을 어떤식으로 할지 고민하느라 시간이 길어질 것 같아 일단 pr을 먼저 남깁니당!! 작업은 하는대로 업데이트하겠습니다
하다보니 폴더링 방식이나 저만의 구조 스타일이 없었다는게 느껴지네요 😇
각자의 힌트나 조언을 주시거나 리팩토링한 코드에서 의문점/개선방안을 날려주시면 감사드리겠습니다~~

async function getParsedData(url: string, config: object) {
const response = await fetch(url, config);
return await response.json();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 parsed 부분와 config 를 이렇게 나누면 나중에 재활용하거나 활용하기 좋아 보이네용 굳굳 👍🏻

return getParsedData(url, getConfig(METHOD.GET, data));
}

export async function deleteAsync(url: string, data: object) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

await 키워드가 없는 함수에 async 가 붙어있네용?. ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

핚 이런 날카로운 리뷰..^^ my (옛)pair답군요 감사합니당!!

Copy link
Member

@InSeong-So InSeong-So left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 나무🤗
몸은 좀 괜찮아졌나요? 설명도 없이 코드 수정한다구 고생했어요!!

Comment on lines +18 to +19
headers: new Headers({ "content-type": "application/json" }),
body: data && JSON.stringify(data),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&& 연산자로 초기화한 부분👍👍

Comment on lines +1 to +7
export function setLocal(key: string, value: string) {
localStorage.setItem(key, value);
}

export function getLocal(key: string) {
return localStorage.getItem(key);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

간단하게 래핑한 것 같네요🤭 key, value가 null/undefined 경우 에러/예외 처리를 추가하는 것도 좋아보여요👀

@InSeong-So
Copy link
Member

나무는 조금 늦게 시작해서 조금 더 열어둘게요!😁

@wooooooood
Copy link
Member Author

나무는 조금 늦게 시작해서 조금 더 열어둘게요!😁

감사합니다ㅠㅠ 이번주중에 마무리해볼게요!!

@InSeong-So InSeong-So merged commit 5695c81 into pagers-org:main Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants