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

[권주현] Week19 #493

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 22 additions & 20 deletions api.ts
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

api 요청 경로 변경 코드입니다.

Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { CODEIT_BASE_URL } from "@/constants";
import { FormValues } from "./common/Auth/Form";
import {
FolderData,
LinkData,
Comment on lines 3 to 4
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

타입 또는 인터페이스를 선언할 때, 해당 타입들이 특정 데이터의 타입임을 표현하고 있는데요.
굳이 ~~Data 라는 식으로 타입 네이밍을 하는게 과연 좋을 지 한번 고민해볼 수 있으면 좋겠어요.

Expand All @@ -9,6 +8,7 @@ import {
Response,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

NextJS에서 이미 서버 리스폰스 객체를 위한 Response라는 예약어가 사용되고 있을텐데요,
page router를 사용할때는 충돌이 없었나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

예약어는 NextResponse로 설정되어 있어 충돌은 없지만 Response라는 단어가 너무 포괄적인 단어라 수정해야 할 듯 합니다.

UserData,
} from "./types/api";
import { FormValues } from "./types/form";
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

앞으로 form value들이 굉장히 다양하게 존재하게 될 텐데요.
지금 사용되는 formvalues 타입의 경우, 계정 정보 활용을 위한 폼 데이터를 관리하는 요소로 보여요.
그렇다면 form value보다는 더 좋은, 그리고 직관적인 이름이 있을텐데요.
어떻게 변경하면 좋을 지 한번 고민해볼까요?


interface TokenProp {
token: string;
Expand Down Expand Up @@ -134,70 +134,72 @@ export async function getLinksByFolderId({

// POST

export async function postEmailCheck(email: string): Promise<void | string> {
const response = await fetch(`${CODEIT_BASE_URL}/check-email`, {
export async function postEmailCheck(email: string): Promise<boolean | string> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

크게 프로젝트 아키텍쳐를 레이어로 나누어보면, 네트워크 요청을 전담하는 controller layer와 비즈니스 로직을 담당하는 service layer, 그리고 결과물을 사용자에게 보여주고 ui interaction을 처리하는 ui layer로 구분해볼 수 있어요.

여기서 postEmailCheck 과 같은 함수들이 우리가 말하는 비즈니스 로직을 담고 있는 서비스 레이어로 평가되는데요.
비즈니스 로직은 ui 레이어에서 최대한 필요한 데이터만 받아와 활용할 수 있도록 해주는 목적을 지니고 있습니다.

지금 작성된 코드를 살펴보면, ui 레이어에서 바로 필요한 데이터를 받아와 활용하기 어려워보여요.
특히, 이 함수의 반환값이 object일수도 있고, string일수도 있고, error 가 던져질 수도 있기 때문인데요.
이를 어떻게 개선 시킬 수 있을지 생각해보면 어떨까요?

const response = await fetch(`${CODEIT_BASE_URL}/users/check-email`, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({ email }),
});
const result = await response.json();

if (response.status === 409) {
const data = await response.json();
return data.error.message;
return result.message;
}

if (!response.ok) {
throw new Error("잘못된 요청입니다.");
}

return;
return result;
}

interface postData {
data: {
accessToken: string;
refreshToken: string;
};
data:
| {
accessToken: string;
refreshToken: string;
}
| { message: string };
}
Comment on lines 158 to 165
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97
마찬가지로 postData라는 인터페이스의 이름은 직관적이지 않아요. 어떤 응답에 대한 타입을 설정한건지를 명시를 해주면 어떨까요?

또한, 서버에서 반환되는 응답이 data type이거나 오류인경우 message: string으로 반환된다면, 이를 공통적으로 활용할 수 있는 ServerResponse와 같은 인터페이스와 제네릭을 조립하는 형태로 타입을 사용하는게 더 좋을 것 같은데 어떻게 생각하시나요?


export async function postSignup({
email,
password,
}: FormValues): Promise<postData> {
const response = await fetch(`${CODEIT_BASE_URL}/sign-up`, {
}: FormValues): Promise<postData | void> {
const response = await fetch(`${CODEIT_BASE_URL}/auth/sign-up`, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({ email, password }),
});
const data = await response.json();

if (!response.ok) {
return data.error.message;
throw new Error("잘못된 요청입니다.");
}

return data;
}
Comment on lines 167 to 182
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

success 여부에 따라 응답을 반환해주어 ui layer에서 성공적인지 아닌지를 명시적으로 활용할 수 있도록 해주는게 좋아요.

또한 response.ok 는 잘못된 요청뿐만 아니라 서버가 죽었거나, 네트워크 통신이 실패했거나, 등 다양한 오류 케이스가 존재하는 경우인데요. 이에 대해 더 적절한 에러핸들링이 필요합니다.


export async function postSignin({
email,
password,
}: FormValues): Promise<postData> {
const response = await fetch(`${CODEIT_BASE_URL}/sign-in`, {
const response = await fetch(`${CODEIT_BASE_URL}/auth/sign-in`, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({ email, password }),
});
const data = await response.json();

if (!response.ok) {
return data.error.message;
throw new Error("잘못된 요청입니다.");
}

return data;
const result = await response.json();

return result;
}

// 탠스택 쿼리와 미들웨어 api 라우트 구현
18 changes: 0 additions & 18 deletions common/Auth/Form/index.module.css

This file was deleted.

36 changes: 0 additions & 36 deletions common/Auth/Form/index.tsx

This file was deleted.

70 changes: 0 additions & 70 deletions common/Auth/Input/index.tsx
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

기본 html 태그 타입을 상속해주었습니다.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,23 @@
.container {
.formContainer {
width: 100%;
max-width: 400px;
display: flex;
flex-direction: column;
gap: 15px;
}

.submitButton {
font-size: 20px;
width: 100%;
height: 60px;
border: none;
border-radius: 8px;
color: var(--white);
background: var(--primary-gradient);
margin-bottom: 15px;
}

.inputContainer {
width: 100%;
position: relative;
}
Expand All @@ -10,6 +29,7 @@
border: 1px solid var(--gray-500);
border-radius: 8px;
margin-bottom: 10px;
font-size: 16px;
}

.inputWrapper:focus {
Expand All @@ -18,15 +38,15 @@

.inputLabel {
display: inline-block;
margin-bottom: 15px;
margin-bottom: 10px;
}

.eyeSlash {
.eyeIcon {
border: none;
background: none;
position: absolute;
right: 16px;
top: 53px;
top: 48px;
}

.errorBorder {
Expand Down
File renamed without changes.
78 changes: 78 additions & 0 deletions components/Auth/SigninForm/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { useForm } from "react-hook-form";
import useSignin from "@/hooks/auth/useSignin";
import { useStoreState } from "@/hooks/state";
import classNames from "classnames";
import { FormValues } from "@/types/form";
import { EMAIL_PATTERN, PW_PATTERN } from "@/constants";
import { FaEye, FaEyeSlash } from "react-icons/fa";
import styles from "../Form.module.css";

Comment on lines +5 to +9
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97

absolute path로 import하기로 경정했다면, 모든 요소들에 대해 absolute import를 해주세요

function SigninForm() {
const {
register,
handleSubmit,
formState: { errors },
} = useForm<FormValues>({ mode: "onBlur" });
const { currentType, toggleType } = useStoreState();
const { handleSignin } = useSignin();

return (
<form
className={styles.formContainer}
onSubmit={handleSubmit(handleSignin)}
>
<div className={styles.inputContainer}>
<label className={styles.inputLabel} htmlFor="email">
이메일
</label>
<input
id="email"
type="text"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kuum97
기본적인 email은 input type ="email"로 설정해서 더 semantic하게 해주세요
또한 input type="email"설정을 통해 근원적인 email type validation이 가능하답니다

placeholder="이메일을 입력해 주세요"
className={classNames(styles.inputWrapper, {
[styles.errorBorder]: errors.email,
})}
{...register("email", {
required: "이메일을 입력해 주세요",
pattern: {
value: EMAIL_PATTERN,
message: "올바른 형식의 이메일을 입력해 주세요",
},
})}
/>
{errors.email && (
<p className={styles.errorMessage}>{errors.email.message}</p>
)}
</div>
<div className={styles.inputContainer}>
<label className={styles.inputLabel} htmlFor="password">
비밀번호
</label>
<input
id="password"
type={currentType}
placeholder="영문, 숫자를 조합해 8자 이상 입력해 주세요"
className={classNames(styles.inputWrapper, {
[styles.errorBorder]: errors.password,
})}
{...register("password", {
required: "비밀번호를 입력해 주세요",
pattern: {
value: PW_PATTERN,
message: "영문, 숫자를 조합해 8자 이상 입력해 주세요",
},
})}
/>
{errors.password && (
<p className={styles.errorMessage}>{errors.password.message}</p>
)}
<button className={styles.eyeIcon} type="button" onClick={toggleType}>
{currentType === "password" ? <FaEye /> : <FaEyeSlash />}
</button>
</div>
<button className={styles.submitButton}>로그인</button>
</form>
);
}

export default SigninForm;
Loading
Loading