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

[NDD-136] setting 전역 hook 작성 (1h/2h) #38

Merged
merged 3 commits into from
Nov 14, 2023
Merged

Conversation

Yoon-Hae-Min
Copy link
Collaborator

@Yoon-Hae-Min Yoon-Hae-Min commented Nov 13, 2023

NDD-136 Powered by Pull Request Badge

Why

다른건 건드릴수가 없어서 어쩌다보니 atom 생성만 하였습니다. 그래서 한시간 정도 감소했네요.
빠르게 머지하고 성인님이 해당 코드 사용하시죠!!

How

페이지 마다 얻은 상태를 전역으로 관리합니다. atom의 특징을 조금 찾아보니까 하나의 큰 스토어 안에 state가 여러개 있는게 아니라 모듈로 나눌 수 있다는 장점이 크더라고요. useState를 분리하는 기준과 똑같이 나누어 보았습니다.

성인님이 페이지별로 세팅 값을 토대로 간단하게 더미데이터를 만들고 작업을 진행하시면 될것 같습니다.

포인트1

타입은 아직 폴더 구조나 어디에 추가로 의존하고 있을지 몰라서 같은 파일에 놔두었는데 아마 SlectedData의 타입은 문제 데이터를 받아오는 api fetch 결과값과 똑같을것이라고 예상되어서 해당 부분 작업할때 합치는 쪽으로 가는게 좋을것 같습니다.

포인트2

제가 이해한 내용을 공유드리면 recoil은 3가지 방법으로 상태를 받아올 수 있었는데요.

  1. useRecoilState(// 생성한 atom의 결과값) 우리가 아는 useState와 똑같은 꼴
  2. useRecoilValue(// 생성한 atom의 결과값) getter만 반환
  3. useSetRecoilState(// 생성한 atom의 결과값) setter만 반환

굉장히 별거 없더라고요. 지금까지의 느낌은 그냥 전역 useState그 자체인것 같습니다

@Yoon-Hae-Min Yoon-Hae-Min added the FE 프론트엔드 코드 변경사항 label Nov 13, 2023
@Yoon-Hae-Min Yoon-Hae-Min self-assigned this Nov 13, 2023
type PageStatus = 'pending' | 'success' | 'error';
type RecordMethod = 'local' | 'idrive' | 'none' | undefined;

type SelectedData = {
Copy link
Collaborator

@adultlee adultlee Nov 14, 2023

Choose a reason for hiding this comment

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

[p-1]
아마도 이 형태에서는 여러개의 selectedData가 들어가지 못할것으로 생각됩니다.
배열형태로 저장됨이 필요하며,

type QuestionData = {
  id: number;
  content: string;
  answer: string;
};

type SelectedData = QuestionData[];

이런 느낌으로 바꿔보면 어떨까요?

export const questionSetting = atom<{
  status: PageStatus;
  selectedData: SelectedData;
}>({
  key: 'questionSetting',
  default: {
    status: 'pending',
    selectedData: [], // 빈 배열로 초기화
  },
});

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

Choose a reason for hiding this comment

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

현재 이 타입에서는 아래와 같이 저장할 수 있어서 여러개의 데이터가 들어갈 수 있을 것이라고 생각합니다!
사용자가 문제 선택 박스를 클릭함에 따라서 상태에서 추가되고 제거되어야 하니 배열보다는 현재 객체 형태가 더 좋다고 생각합니다.

const test:SelectedData = {
  key1: {
    id: 1,
    content: "content 1",
    answer: "anser 1"
  },
  key2: {
    id: 2,
    content: "content 2",
    answer: "anser 2"
  }
}

Copy link
Collaborator

@adultlee adultlee Nov 14, 2023

Choose a reason for hiding this comment

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

제가 recoil을 잘 몰라서 일까요 😂 잘 이해가 안되는 부분이 있어요! 예시를 한번 볼 수 있을까요?
key1 이나 key2는 저희가 사용할 변수명이 아닐거 같기도하며 (어떤 key 네이밍이 좋을까요)
제 생각에는 camera 페이지에서 return 받는 최종값은 배열형태로
배열 내부의 객체 형태로 받는것이 여러가지 의미로 편할 수 있을거 같습니다!

예를들어

const selctedQuestions = [{
 title : "1번 질문",
 answer : "1번 질문에 대한 답변" 
},{
 title : "2번 질문",
 answer : "2번 질문에 대한 답변" 
}
]
...

이런 형태로 말이죠!

Copy link
Collaborator

Choose a reason for hiding this comment

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

아하 어느정도 이해했습니다. 왜 key 값으로 관리가 되어야 할지에 대해서 동의했습니다!!

아마 getter 등을 사용해서 반환할때만 배열형태로만 반환하면 될거 같긴하군요

Copy link
Collaborator

Choose a reason for hiding this comment

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

네넵! 실제 인터뷰 페이지에서 사용될 때 Object.values() 등을 사용해서 배열로 변환해서 사용하면 될 것 같네요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

앗 요부분은 제가 잘못 타이핑 되었네요. 수정본은 이렇습니다

Suggested change
type SelectedData = {
type SelectedData = {
[key: string]: {
id: number;
content: string;
answer: string;
}[];
};

제가 의도한 바로는 slectedData를 각 각 분야 별(FE,BE,CS 등등)로 선택된 배열을 묶어서 전역 상태로 사용하려고 합니다
지금 기획에서는 성인님 쪽에서 분야를 제외 하고 사용을 하셔서 성인님이 제시해 주신 부분이 맞지만 전역적인 관점에서 봤을때 나중에 문제에 대한 분류를 달아준다던가. 진행상황을 저장한다던가 등 해당 질문의 분류가 필요해 질때를 고려해서 작성하였습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

아!! key값에 카테고리가 저장되는거였군요!! 제가 잘못 이해했네요!
그럼 각 선택된 질문은 배열 형태로 관리되는걸까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,43 @@
import { atom } from 'recoil';
Copy link
Collaborator

@adultlee adultlee Nov 14, 2023

Choose a reason for hiding this comment

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

[p-2]atoms라는 새로운 폴더가 생성되었으니, tsconfig.ts 와 webpack.config.js의 alias도 추가해 주시길 바랍니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

@milk717 milk717 left a comment

Choose a reason for hiding this comment

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

recoil 정말 간단하네요!
고생하셨습니다!

Copy link

cloudflare-workers-and-pages bot commented Nov 14, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 441cca7
Status: ✅  Deploy successful!
Preview URL: https://167c56e3.gomterview.pages.dev
Branch Preview URL: https://feature-ndd-136.gomterview.pages.dev

View logs

@Yoon-Hae-Min Yoon-Hae-Min merged commit 0fe8c9c into dev Nov 14, 2023
1 check passed
@Yoon-Hae-Min Yoon-Hae-Min deleted the feature/NDD-136 branch November 23, 2023 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트엔드 코드 변경사항
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants