-
Notifications
You must be signed in to change notification settings - Fork 31
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단계 - 지하철 노선도 미션] 지그(송지은) 미션 제출합니다. #13
Conversation
- eslint, prettier 설정 - storybook 설치 - emotion 설치 Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: jieun song <zigsong@users.noreply.github.com>
Co-authored-by: jieun song <zigsong@users.noreply.github.com>
Co-authored-by: jieun song <zigsong@users.noreply.github.com>
Co-authored-by: jieun song <zigsong@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
- 지하철역 불러오기 - 지하철역 생성 - 지하철역 삭제 Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
- 서버 선택지 위치 변경 Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: jieun song <zigsong@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
-NavBar amrgin 조정 Co-authored-by: jieun song <zigsong@users.noreply.github.com>
- NavBar margin 조정 Co-authored-by: jieun song <zigsong@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
- api 요청 중복 코드 개선 - Loading 시 spinner 표시 Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
Co-authored-by: HyuuunjuKim <HyuuunjuKim@users.noreply.github.com>
"scripts": { | ||
"start": "react-app-rewired start", | ||
"build": "react-app-rewired build", | ||
"test": "react-app-rewired test", | ||
"eject": "react-app-rewired eject", | ||
"storybook": "start-storybook -p 6006 -s public", | ||
"build-storybook": "build-storybook -s public" | ||
}, |
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.
emotion
관련하여 babel 설정 때문에 react-app-rewired
를 사용했습니다.
const [color, setColor] = useState<string>(''); | ||
const [name, setName] = useState<string>(''); | ||
const [upStationId, setUpStationId] = useState<string>(''); | ||
const [downStationId, setDownStationId] = useState<string>(''); | ||
const [distance, setDistance] = useState<string>(''); | ||
const [extraFare, setExtraFare] = useState<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.
이렇게 하나의 컴포넌트에서 많은 값들을 input으로 받는 경우는 하나하나 useState
를 해주는 게 좋을까요?
아니면 객체 형태로 관리하거나, 또 다른 방법이 있을까요?
큰 불편함은 없지만 뭔가 너무 많아서, reset시켜줄 때에도 일일이 비워주는 것이 아주 맘에 들진 않아서 여쭤봅니다..!
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.
Form을 다루는 일은 굉장히 번거롭고 생각보다 어렵습니다.
react-hook-form이나 formik같은 라이브러리가 어떤식으로 처리하고있는지 확인해보시면 좋을것같아요 👍
객체형태로 관리하는것도 좋은방법입니다.
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.
객체형태로 관리해보려고도 시도해봤으나 여기저기서 단독으로 사용되는 곳들이 많아 우선 그대로 진행하도록 하겠습니다!
src/hooks/useFetch.ts
Outdated
} | ||
}; | ||
|
||
return { fetchData, loading }; |
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.
여기서 사실 리턴을
return [ fetchData, loading ];
과 같이 배열로 해주고, 사용하는 쪽에서
const [fetchStations, stationsLoading] = useFetch(...)
해주고 싶었으나, TypeScript에서 배열의 인자 각각에 type을 붙여주는 방법을 찾지 못해 lint에 지고 말았습니다.
그래서 실제로는
const { fetchData: fetchStations, loading: stationsLoading } = useFetch(...);
와 같이 따로 네이밍해주는 방식으로 사용하고 있습니다. (하나의 컴포넌트에서 서로 다른 요청들, 즉 서로 다른 useFetch
를 여러 개씩 쓰고 있기 때문에 네이밍의 분리가 필요한 경우들이 생겼습니다.)
오래 찾아봐도 답을 알지 못해 여쭤봅니다 ㅠㅠ
이렇게 키값이 붙어있는는 객체로 리턴하는 방법밖엔 없을까요?
배열을 사용하며 type을 붙이는 방법을 알고 계시나요....
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.
return [ fetchData, loading ];
처럼 하면 그냥 타입추론 되지 않나요..?
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.
// useFetch.ts
return [ fetchData, loading ];
위와 같이 리턴하고
사용하는 컴포넌트에서
const [fetchData, loading] = useFetch('GET');
로 사용하면 되는데,
하나의 컴포넌트에서 여러 개의 useFetch
를 사용하는 경우가 많아
// StationsPage.tsx
const [getStationsAsync, getStationsloading] = useFetch('GET');
const [addStationAsync, addStationLoading] = useFetch('POST');
와 같이 fetchData
, loading
이 아닌 다른 이름으로 받으면
// StationsPage.tsx
const res = await getStationsAsync(END_POINT.STATIONS);
// error: getStationsAsync is not callable...
이런 식의 에러가 납니다. (hover하면 원래 에러가 뜨는데, 오늘 종일 제 노트북과 vscode자체가 조금 먹통이라 에러 메시지를 확인할 수 없네요 ㅠㅠ 예전에 작성 시 해당 에러가 났었습니다.)
TypeScript 관련해서 찾아보니 현재 useFetch
hook에서 리턴해주는 배열의 인자가 fn 타입(fetchData
) 또는 bool 타입(loading
)이라 타입추론을 하기가 어려워서 발생하는 문제같은데, 이럴 경우 타이핑을 어떻게 해줘야 할까요? 😂 (아마 방법이 없는 것 같기도...??)
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.
return [ fetchData, loading ];
처럼해도 TypeScript가 이 함수의 반환값은 배열길이가 2이고 첫번째 인자는 함수이고 두번째인자는 boolean이라고 잘 추론해야정상입니다.
TS Server를 리스타트하거나 VSCode를 재시작해보세욥..
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.
여전히 안돼서 이것저것 시도해보다가 return 문에
return [fetchData, loading] as const;
를 붙여주면 작동하여 그렇게 수정했습니다!
const useNotify = () => { | ||
const [message, setMessage] = useState<string>(''); | ||
const [isValid, setValid] = useState<boolean>(false); | ||
const [isVisible, setVisible] = useState<boolean>(false); | ||
|
||
const showNotiMessage = ({ | ||
message = '', | ||
valid = false, | ||
visible, | ||
}: { | ||
message?: string; | ||
valid?: boolean; | ||
visible: boolean; | ||
}) => { | ||
setMessage(message); | ||
setValid(valid); | ||
setVisible(visible); | ||
}; | ||
|
||
const NotiMessage = () => ( | ||
<NotificationComponent message={message} isValid={isValid} isVisible={isVisible} /> | ||
); | ||
|
||
return { NotiMessage, showNotiMessage }; | ||
}; |
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.
여기는 React Component를 리턴하고 있어서 그런지 ...
.tsx
확장자가 아니면 아무것도 인식되지 않아서 .tsx
를 붙였습니다.
컴포넌트 자체를 리턴하는 custom hook은 다른 크루에게 배웠는데, 무조건 좋은 방식인지는 둘 다 헷갈리는 상태입니다.
component를 리턴하는 hook이라서 .ts
가 아닌 .tsx
확장자가 필요한 것이 맞을까요?
그리고 custom hook에서 컴포넌트를 리턴하는 패턴에 대해서는 어떻게 생각하시는지 궁금합니다!
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.
component를 리턴하는 hook이라서 .ts가 아닌 .tsx 확장자가 필요한 것이 맞을까요?
- 네. JSX문법을 사용하려면 무조건 .jsx 혹은 .tsx 확장자여야 합니다.
custom hook에서 컴포넌트를 리턴하는 패턴에 대해서는 어떻게 생각하시는지 궁금합니다!
- 많이 보진못했고 그렇게 사용해보진 않았지만 나쁜패턴이라고생각하진 않습니다.
string, | ||
{ dispatch: AppDispatch; state: RootState } | ||
>('auth/getUser', async (accessToken, { rejectWithValue, getState }) => { | ||
const BASE_URL = getState().serverSlice.server; |
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.
요렇게 authSlice
에서 store
의 상태에 바로 접근하고 있는데...
생각해보면 동일한 store를 가지고 하나의 리듀서(authSlice
)에서 다른 리듀서(serverSlice
)에 접근하는 순환 참조(?)같은 것을 하고 있다는 생각이 들었습니다. 약간은 뭔가 찜찜한 기분...
그래도 requestGetUser
함수를 수행하기 위해서는 redux store에 있는 BASE_URL
에 대한 정보가 필요하기에 가져다 썼는데, 혹시 저희가 생각지 못한 사이드 이펙트가 있을지 여쭤봅니다!
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.
아래쪽 첫번째 await이후에 실행된다면 비동기로 실행되어서 시점때문에 사이드이펙트가 발생할 여지가있을것같아요.
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.
혹시 비동기로 실행되는지 여부는 어떻게 알 수 있을까요?
지금은 getState
가 동작하는 시점에 있어서 사이드이펙트의 여지가 없어보이는데, 혹시 지금 시점에서 수정해야 할 부분이 있을까요? 🤔
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.
첫번째 await을 만나기전까지는 동기로실행되고, 첫번째 await 이후에는 모두 비동기라고 생각하시면 됩니다.
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.
아하 그렇겠군요!
현재는 한번 로그인하거나 이미 로그인된 상태에서 새로고침한 이후에는 서버(BASE_URL
)을 변경하지 못하도록 되어있기 때문에 따로 사이드이펙트를 고려해주지는 않아도 될 것 같습니다. 설명 감사드립니다! 👍👍
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.
현재 redux로 상태관리하고 있는 값들은 User 정보와 server입니다. server는 새로고침을 해도 유지되어야 하기 때문에 어쩔 수 없이 web storage에 함께 넣었는데, 처음 로그인 시 선택할 때에만 storage에 server 주소를 심어주고 그 이후에는 전체 앱의 상태를 관리하는 redux에서 가지고 있게끔 하였습니다. 사실은 새로고침을 고려하지 않고 당연히(?) redux로 상태관리를 해주고 있다가 이후 web storage로 옮겼는데, redux로 코드를 짠 것이 아쉬워서 😅 그냥 두었습니다. 그런데 또 storage에 매번 접근하는 것은 비효율적일 수 있기에 처음에만 storage에 저장하고 그 이후에는 (로그아웃 전까지) redux store에서 server를 가져다 쓰는 것이 나쁘지 않다고 판단했는데, 무엇이 더 효과적인 방식인지 의견 여쭙고 싶습니다!
- 새로고침 정보 유지를 위해 서버정보를 web storage에 저장한 것은 적절한 방법인것 같습니다 👍
고생하셨어요! 👏 👏 👏
const [color, setColor] = useState<string>(''); | ||
const [name, setName] = useState<string>(''); | ||
const [upStationId, setUpStationId] = useState<string>(''); | ||
const [downStationId, setDownStationId] = useState<string>(''); | ||
const [distance, setDistance] = useState<string>(''); | ||
const [extraFare, setExtraFare] = useState<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.
Form을 다루는 일은 굉장히 번거롭고 생각보다 어렵습니다.
react-hook-form이나 formik같은 라이브러리가 어떤식으로 처리하고있는지 확인해보시면 좋을것같아요 👍
객체형태로 관리하는것도 좋은방법입니다.
src/hooks/useFetch.ts
Outdated
} | ||
}; | ||
|
||
return { fetchData, loading }; |
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.
return [ fetchData, loading ];
처럼 하면 그냥 타입추론 되지 않나요..?
src/hooks/useFetch.ts
Outdated
|
||
type HTTP_METHOD = 'GET' | 'POST' | 'PUT' | 'DELETE'; | ||
|
||
const useFetch = (method: HTTP_METHOD) => { |
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.
useFetch는 일반적으로 get method를 사용하고 어떤데이터를 가져오는지 endpoint나 데이터의 키값을 정의하는것이 일반적입니다.
https://use-http.com/
이러한 라이브러리가 인터페이스를 어떻게 구현했는지 확인해보세요. 👍
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.
좋은 참고자료 감사합니다! 사실 올려주신 라이브러리 뿐 아니라 다른 여러 자료에서도 useFetch
는 GET
으로만 사용하고 있어서 저희 나름대로 커스터마이징을 하는데 애를 많이 먹었습니다 😂
이번 미션에서는 여러 도메인에서 GET
, POST
, PUT
, DELETE
등 다양한 http method가 사용되고 있어서 요청 방식을 hook의 인자로 넣어주고 있는데, 말씀주신 대로라면 GET
요청을 보내는 데 있어서만 공통적으로 hook을 사용하고, 나머지 요청들은 hook 없이 작성하는 것이 좋을까요? 🤔
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.
일반적으로 GET요청을 가장 많이사용하기때문에 GET을 default method로 두고,(fetch의 기본 method도 GET입니다)
나머지 요청이 필요할땐 인자로 넣어주면 될것같아요! 👍
const useNotify = () => { | ||
const [message, setMessage] = useState<string>(''); | ||
const [isValid, setValid] = useState<boolean>(false); | ||
const [isVisible, setVisible] = useState<boolean>(false); | ||
|
||
const showNotiMessage = ({ | ||
message = '', | ||
valid = false, | ||
visible, | ||
}: { | ||
message?: string; | ||
valid?: boolean; | ||
visible: boolean; | ||
}) => { | ||
setMessage(message); | ||
setValid(valid); | ||
setVisible(visible); | ||
}; | ||
|
||
const NotiMessage = () => ( | ||
<NotificationComponent message={message} isValid={isValid} isVisible={isVisible} /> | ||
); | ||
|
||
return { NotiMessage, showNotiMessage }; | ||
}; |
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.
component를 리턴하는 hook이라서 .ts가 아닌 .tsx 확장자가 필요한 것이 맞을까요?
- 네. JSX문법을 사용하려면 무조건 .jsx 혹은 .tsx 확장자여야 합니다.
custom hook에서 컴포넌트를 리턴하는 패턴에 대해서는 어떻게 생각하시는지 궁금합니다!
- 많이 보진못했고 그렇게 사용해보진 않았지만 나쁜패턴이라고생각하진 않습니다.
string, | ||
{ dispatch: AppDispatch; state: RootState } | ||
>('auth/getUser', async (accessToken, { rejectWithValue, getState }) => { | ||
const BASE_URL = getState().serverSlice.server; |
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.
아래쪽 첫번째 await이후에 실행된다면 비동기로 실행되어서 시점때문에 사이드이펙트가 발생할 여지가있을것같아요.
@ysm0622
1단계 리뷰가 온지 모르고 2단계를 며칠 전에 마무리해놨는데 조만간 백엔드 서버가 닫힐 예정이라 😂😂 |
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.
피드백 잘 반영해주셔서 감사합니다 :)
고생하셨어요! 👏 👏 👏 👏
학습로그 |
안녕하세요! 지하철 노선도 1단계 제출합니다.
이번 미션은 백엔드 크루들과 함께 진행하여, 로그인/회원가입 페이지에서 5명의 크루들 중 한 명을 선택해야 다음 단계로 진행하실 수 있습니다!
리뷰 잘 부탁드리겠습니다 ㅎㅎ 😉
🚇 demo site
✅ Todos - 1단계
회원 기능
역 관리 기능
노선 관리 기능
구간 관리 기능
🤔 고민한 내용
useFetch
라는 custom hook을 만들어 사용했는데, custom hook의 리턴값과 관련한 궁금증이 있어 자세한 부분 코멘트로 남기기로 했습니다.User
정보와server
입니다.server
는 새로고침을 해도 유지되어야 하기 때문에 어쩔 수 없이 web storage에 함께 넣었는데, 처음 로그인 시 선택할 때에만 storage에 server 주소를 심어주고 그 이후에는 전체 앱의 상태를 관리하는 redux에서 가지고 있게끔 하였습니다. 사실은 새로고침을 고려하지 않고 당연히(?) redux로 상태관리를 해주고 있다가 이후 web storage로 옮겼는데, redux로 코드를 짠 것이 아쉬워서 😅 그냥 두었습니다. 그런데 또 storage에 매번 접근하는 것은 비효율적일 수 있기에 처음에만 storage에 저장하고 그 이후에는 (로그아웃 전까지) redux store에서 server를 가져다 쓰는 것이 나쁘지 않다고 판단했는데, 무엇이 더 효과적인 방식인지 의견 여쭙고 싶습니다!saga
를 공부하여 써보고 싶었으나, 시간에 쫓겨 toolkit에 내장된 thunk를 사용했습니다.