feat: 리마인더 구현

[hozzijeong]

🔥 연관 이슈

• close 리마인더 기능을 구현한다. #125

🚀 작업 내용

• [�x] 식물 리마인더 데이터 불러오기
• 날짜에 따른 카드 분리하기
• 관리 기록 관련 이벤트를 등록한다.
  • 체크박스 클릭 시에 오늘 날짜 넘어가기
  • 미루기 클릭 시에 하루 추가하기
  • 날짜 입력 선택 시에 달력창으로 현재 날짜 입력할 수 있도록 하기 (직접 물을 준 날짜를 선택하는 기능으로, 최대값이 '오늘'이 되도록 설정했습니다.)
  • 관리 기록 이벤트가 발생하고 난 다음에 데이터 Fetch가 한번 더 일어나야 한다.
• [x�] 지각 | 오늘 일정 | 남은 일정에 따른 배경색을 분리힌다.

2023-07-26.9.30.48.mov

---

대략적인 Reminder 데이터의 섹션(?)을 구분해 봤습니다!

작업 내용이 조금 많이 있습니다. msw 설정과 날짜 계산 로직을 넣어본다고 넣었는데 잘 넣었는지 모르겠네요 ㅎ

checkbox구현을 했는데 쓸모가 없어져서... 혹시 모르니 같이 올려놔 봅니다!

• API명세대로 데이터를 받긴 하지만, 디자인에 맞춰서 값을 새로 분리해야 했습니다. 분리된 타입은 ReminderExtendType입니다.
• API명세를 해당 타입으로 변환하는 메서드는 useReminder에 있는 convertReminderData입니다.
• 3개의 훅을 하나로 합치는 훅인 useReminderHooks를 선언했는데, 데이터에 변화가 있을 때마다 refetch를 해야했기 때문에 3개의 훅이 같은 파일에서 있을 필요가 있다고 생각해서 새로운 훅을 만들었습니다.
• 코드를 보면 아시겠지만, Reminder가 최대 4뎁스까지 가기 때문에, ContextAPI를 통해서 callback 함수들을 전역으로 설정했습니다.

이후 궁금하신 사항 있으면 바로바로 말씀해 주세요!

💬 리뷰 중점사항

우선은 기능 구현이 잘 되는지 확인 부탁드립니다.
리마인더 로직을 짜면서 테스트의 필요성에 대해 살짝 느끼게 되었는데, 테스팅 라이브러리 도입에 대한 여러분의 생각을 듣고싶습니다!!

ps. 쵸파 테코톡 화이팅, 참새 아프지 마세요 ㅠ

[WaiNaat]

핵심기능 멋있게 만들어주셔서 감사해요 클린!!
로컬로 돌렸을 때는 개발서버랑 스토리북 둘 다 잘 돌아갑니다 👍

[bassyu]

작업량이 정말 엄청나네요 😮
코멘트는 큰 내용은 없고 pushOff 네이밍을 delay로 하면 어떨지 제안해봅니다!
수고하셨어요 클린..

[bassyu]

R

Suggested change

	const reminderAPI = {
	const ReminderAPI = {

아래 컨벤션 참조해서 파스칼 케이스로 네이밍하는 것도 좋아보입니다!
https://github.com/ParkSB/javascript-style-guide#naming--PascalCase-singleton

[bassyu]

C

타입이 있어서 아래처럼 작성해도 안전할 것 같은데 어떤가요?

Suggested change

	switch (status) {
	case 'late':
	return theme.color.accent;
	case 'today':
	return theme.color.primary;
	case 'future':
	return theme.color.grayDark;
	default:
	return theme.color.grayDark;
	return {
	late: theme.color.accent,
	today: theme.color.primary,
	future: theme.color.grayDark,
	}[status];

[bassyu]

C

여기도 이렇게 어떤가요?

Suggested change

	const convertSubFix = (status: TodayStatus) => {
	switch (status) {
	case 'today':
	return '오늘이에요!';
	case 'late':
	return '일 지났어요🥺';
	case 'future':
	return '일 남았습니다!';
	default:
	return '';
	}
	};
	const convertSubFix = (status: TodayStatus) => ({
	today: '오늘이에요!',
	late: '일 지났어요🥺',
	future: '일 남았습니다!',
	}[status]);

[bassyu]

R

네이밍을 delay라고 하는 건 어떤가요?

[hozzijeong]

아예 네이밍이 바뀔것 같습니다 .. ㅎ

[github-actions]

Unit Test Results

67 tests   67 ✔️  3s ⏱️
12 suites    0 💤
12 files      0 ❌

Results for commit a03da58.

[bassyu]

리팩터링 양도 상당하네요!!
코드 양도 많고, 대부분 잘 구현해주신 것 같습니다!
반영할 것만 반영해주세요!

[bassyu]

R

• 함수의 타입이어서 Props보다는 Params가 맞는 것 같습니다!
• 여기서만 사용하는 타입이라면 import 대신 이 파일에서 타입을 정의하는 건 어떤가요?

[hozzijeong]

확실히 Params가 더 맞는것 같네요...!! 수정하겠습니다 ㅎ

[bassyu]

R

Suggested change

	setChecked((prev) => !prev);
	setChecked(!checked);

실시간으로 변경된 스냅샷을 사용하지 않는다면 콜백을 사용하지 않는 것이 의도에 더 맞는 것 같아요!

[hozzijeong]

전 함수의 순수성(?)을 생각해서 외부의 값을 참조하지 않고 해당 메서드에서 처리되는게 낫다고 생각을 해서 콜백 형식으로 작성했습니다. 직접 값을 할당하는 방식이 콜백을 사용하는 것과 다르게 특별한 기능적 차이나 의도의 차이가 있을까요?? (의도의 차이가 어떤건지 잘 몰라서 여쭙습니다...ㅎ)

[bassyu]

제가 아는 부분에서 설명 드릴게요!
한번의 리렌더링에서 setState를 여러번 쓰고, 실시간으로 바뀌는 상태를 사용하고 싶은 의도일 때 콜백을 사용한다고 알고 있습니다.
간단하게 아래 두 코드를 보면 이해가 될 것 같습니다!

setState(count + 1); // 2
setState(count + 1); // 2
// 최종 2로 리렌더링

setState((prev) => prev + 1); // 2 
setState((prev) => prev + 1); // 3 
// 최종 3로 리렌더링

!checked 이런식으로 작성해도 원시값은 참조가 완전 바뀌기 때문에 리렌더링 트리거에 차이는 없을거에요!

[hozzijeong]

설명 감사합니다!

개인적인 생각으로는 Handler가 조건에 따른 분기처리를 해야하는 경우라면 상황에 맞는 setState를 사용하는게 맞다고 생각이 들지만, 결국 변경된 상태에 따라 동일한 작업을 수행하는 것이라면 코드를 수정할 필요가 있을까? 하는 생각이 들었습니다! 쵸파는 어떻게 생각하시나요??

[bassyu]

사실 지금 상태에서 결과는 똑같긴 합니다!
다만 '실시간 데이터를 반영하는 것은 아니다'는 메세지를 주는 의미가 있다고 생각했어요.

[bassyu]

C

Suggested change

	const checkHandler = () => {
	const check = () => {

핸들러가 맞긴 하지만 함수의 행동만 네이밍 하는 건 어떤가요?

[hozzijeong]

저는 Handler를 붙이는 게 좀 더 명시적인것 같아요... 메서드의 이름은 최대한 구체적으로 작성하는게 낫다고 생각하는데 쵸파의 생각은 어떤가요?

[bassyu]

C

Suggested change

	<Input type={'checkbox'} id={id} />
	<Input type="checkbox" id={id} />

[bassyu]

R

사용되지 않는 친구인 것 같습니다!

[bassyu]

R

type import 가 빠졌네요!

[bassyu]

C

여기도 Handler Suffix를 빼보는 건 어떨지 제안해봅니다!
보통 핸들러 함수는 컴포넌트 안에, 로직 함수는 훅으로 빼는 패턴이 많이 쓰인다고 생각하는데요,
그래서 핸들러라는 표현을 안해줘도 이해할 수 있고, 행동에 네이밍 초점을 맞추면 좋지 않을까? 생각했어요.

[hozzijeong]

"표현을 안해줘도 이해할 수 있고"는 상당히 주관적인 것 같아요🥲 행동에 네이밍 초점을 맞춘다면 그것은 유틸 함수를 작성할때 좀 더 알맞지 않나? 라고 생각했습니다. 핸들러의 역할을 갖는 메서드라면 핸들러라고 명확하게 명시하는게 좀 더 깔끔하다고 생각이 듭니다

[bassyu]

클린 말도 맞습니다!
표현을 안해줘도 이해할 수 있다는 뜻은 '함수 명만 보고도 핸들러구나'까지는 아니고 코드를 조금 살펴보면 알 수 있다는 뜻이었어요!
마치 저희가 타입을 사용할 때 ~Type Suffix를 붙이지 않는 이유랑 비슷하다고 생각했어요.

[bassyu]

A

이렇게 어떤가요?

Suggested change

	if (reminderData === undefined) return null;
	if (!reminderData) return null;

[WaiNaat]

변경해주신 부분들 확인했어요!
중간에 리마인더 기획이 바뀌어서 할 일이 많았을 것 같은데 고생하셨습니다ㅠㅠ

[WaiNaat]

C

React.InputHTMLAttributes<HTMLInputElement>에 들어있는 value나 onChange가 아니라 직접 선언하신 이유가 궁금해요

[hozzijeong]

value는 타입 체크때문에 그렇습니다. value를 받아서 사용하는 convertDateKorYear에서 받는 타입과 HTMLInputElement에서 받는 value의 타입이 달라서 특정했습니다.

onChange는 약간의 레거시 느낌이 있긴 합니다. 당시에 작성했을 때는 changeHandler와 changeHandler가 발생하고 난 다음에 실행될 callback을 분리하는게 관심사 분리 측면에서 낫다는 생각에 분리했었는데, 지금 생각해보니 눈가리고 아옹(?) 느낌이네요

[WaiNaat]

value는 이해했습니다!
onChange도 어떤 느낌인지는 알 것 같아요. 만약 요 DateInput 내에서 입력값에 대한 검증이나 전처리가 필요하다면 callback을 받아서 처리 후에 실행하는 방향이 깔끔해 보여요. 근데 지금은 value, setValue 역할을 form reducer가 해 줘서 불필요해 보이기도 합니다.

[WaiNaat]

변경해 주신 부분들 확인했어요! 부상투혼 멋있습니다 클린