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

타입스크립트 컨벤션에 맞춰 리팩토링을 진행한다 #870

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from

Conversation

liswktjs
Copy link
Collaborator

Close #790

  • axios Instance를 활용하여 중복되어서 사용되고 있는 HOME_URL과 accessToken을 분리한다
  • REST DOCS에 따라 중복되는 타입을 커스텀 타입을 만들어 리팩토링을 진행한다
  • 각각의 api 하위 폴더에 type을 위치시킨다

@liswktjs liswktjs added 🌈 frontend 프론트엔드 이슈 ♻️ refactor 리팩터링 이슈 📝 typescript labels Oct 24, 2022
@liswktjs liswktjs self-assigned this Oct 24, 2022
Copy link
Collaborator

@hwangstar156 hwangstar156 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~ comment 날린것만 확인해주세요~

Comment on lines 6 to 15
export const generateAxiosInstanceWithAccessToken = () => {
const accessToken = localStorage.getItem(ACCESSTOKEN_KEY);
const data = await axios.get<{ tag: string[] }>(`${HOME_URL}/api/tags`, {
return axios.create({
baseURL: HOME_URL,
headers: {
'Access-Control-Allow-Origin': '*',
Authorization: `Bearer ${accessToken}`,
},
});
return data.data;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

axios를 사용한다면 axios.create를 이용해서 할 수 있지 않을까요?

const axiosInstance = axios.create({
  headers: {
    Authorization : `Bearer ${localStorage.getItem("access_token")}`
    }
  }
})

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

어떤 말인지 자세히 설명해 줄 수 있나요? accessToken을 분리하지 않고 인라인으로 하라는 걸까요? 이미 create를 사용하고 있어서🤔

Comment on lines +1 to +18
import { MutableRefObject } from 'react';

import { postImageUrlConverter } from '@/api/article/image';
import { Editor } from '@toast-ui/react-editor';

export const takeToastImgEditor = (content: MutableRefObject<Editor | null>) => {
if (content.current) {
content.current.getInstance().removeHook('addImageBlobHook');
content.current.getInstance().addHook('addImageBlobHook', (blob, callback) => {
(async () => {
const formData = new FormData();
formData.append('imageFile', blob);
const url = await postImageUrlConverter(formData);
callback(url, 'alt-text');
})();
});
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

이것은 왜 있는거죠? 제가 따로 훅으로 만들어 둔게 있던거 같은데요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

스밍이 만든 훅이 맞는데, 현재 브랜치에는 반영이 안되어 있어 병합하는 과정에서 create로 되어 있는 것 같습니다.

Comment on lines 35 to 45
category,
sort,
cursorId,
cursorViews,
cursorLikes,
}: {
category: string;
sort: '추천순' | '조회순' | '최신순';
cursorId: string;
cursorViews: string;
cursorLikes: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

외부 interface로 빼주세요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

분리하였습니다!

Comment on lines 62 to 70
category,
sort,
cursorId,
cursorViews,
}: {
category: string;
sort: '추천순' | '조회순' | '최신순';
cursorId: string;
cursorViews: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

얘도 빼주세요~

Comment on lines 137 to 140
id: string;
title: string;
content: string;
tag: string[];
Copy link
Collaborator

Choose a reason for hiding this comment

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

interface

Comment on lines 100 to 102
category: string;
cursorId: string;
cursorLikes: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

interface!

@@ -0,0 +1,12 @@
import { generateAxiosInstanceWithAccessToken } from '@/utils/generateAxiosInstance';

export const postAddLikeArticle = (articleId: string) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

articleId만 따로 타입을 빼서 만들수 있지 않을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

어떤 말인지 자세히 설명해줄 수 있나요? interface로 분리하라는 건가...🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

이때의 나는 무슨 얘기를 했던것일까..🤨

Comment on lines 5 to 11
id,
content,
isAnonymous,
}: {
id: string;
content: string;
isAnonymous: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

interface로 빼주세요

@liswktjs
Copy link
Collaborator Author

liswktjs commented Nov 4, 2022

테스트 작동시 아래와 같은 에러 발생 하면서 실패하네요 mock 서버가 잘못된 것 같은데 이유를 아시나요?
console.error
Error: Error: connect ECONNREFUSED ::1:80
at Object.dispatchError (/Users/jasun/2022-gong-seek/frontend/node_modules/jsdom/lib/jsdom/living/xhr/xhr-utils.js:63:19)
at Request. (/Users/jasun/2022-gong-seek/frontend/node_modules/jsdom/lib/jsdom/living/xhr/XMLHttpRequest-impl.js:655:18)
at Request.emit (node:events:532:35)
at NodeClientRequest. (/Users/jasun/2022-gong-seek/frontend/node_modules/jsdom/lib/jsdom/living/helpers/http-request.js:121:14)
at NodeClientRequest.emit (node:events:520:28)
at NodeClientRequest.Object..NodeClientRequest.emit (/Users/jasun/2022-gong-seek/frontend/node_modules/@mswjs/interceptors/src/interceptors/ClientRequest/NodeClientRequest.ts:315:22)
at /Users/jasun/2022-gong-seek/frontend/node_modules/@mswjs/interceptors/src/interceptors/ClientRequest/NodeClientRequest.ts:206:14 {
type: 'XMLHttpRequest'

@liswktjs
Copy link
Collaborator Author

liswktjs commented Dec 6, 2022

api instance 적용 이후, test코드를 작동시 mocking의 서버가 localhost로 잡혀서 테스트가 진행되지 않았었습니다.
그래서 mocking서버의 HOME_URL을 localhost로 변경하였습니다.
스토리북 작동을 확인해보았을 때에 PopularArticle를 제외하고는 모두 정상 동작하였습니다.
그런데 PopularArticle은 왜 오류가 나는지 모르겠더라고요?? 🤔 스토리북 확인 부탁드립니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌈 frontend 프론트엔드 이슈 ♻️ refactor 리팩터링 이슈 📝 typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

타입스크립트 컨벤션에 맞춰 리팩토링을 진행한다
2 participants