Migrate remaining shapes, Complete shapes migration #44

yumincho · 2024-03-12T10:41:38Z

Description

공통적으로 사용되는 enum들을 관리하는 enum.ts를 shapes 폴더 안에 생성했습니다.
shapes 내 모든 파일에 대해 .tsx 확장자를 .ts 확장자로 변경하였습니다.
shape 파일명과 interface명을 camelCase로 작성된 파일명을 PascalCase로 변경하였습니다.
아직 migrate되지 않은 shape들을 .ts 파일로 migrate하였습니다.
shape에서 enum이나 다른 shape을 import할 때 type만 import하도록 수정했습니다.

To-do

prop-types에서는 런타임에 validation을 할 수 있는 custom function을 만들어서 패스할 수 있지만, typescript에서는 interface 레벨에서의 validation이 불가능합니다. 기존 /shapes/state/timetable/LectureFocusShape.js에서 사용하던 emptyArrayShape를 대체할 validation을 component 레벨에 추가해야 합니다.

github-actions · 2024-03-12T10:42:26Z

PR Preview Action v1.4.7
🚀 Deployed preview to https://sparcs-kaist.github.io/otlplus-web/pr-preview/pr-44/
on branch `gh-pages` at 2024-03-12 10:42 UTC

github-actions · 2024-03-12T10:42:27Z

[PR Preview GH Proxy (on bap)]
👍 Proxied Github Pages Preview URL to http://pr-44.otl-pr-preview.sparcsandbox.com/index.html
Proudly Powered by [email protected]

.tsx -> .ts camelCase -> PascalCase

yumincho · 2024-03-13T07:56:22Z

Link shape이 react-router-dom의 Link와 conflict가 나서 shape명을 추가로 변경하였습니다. (6bf160b)

jooyeongmee · 2024-03-13T08:20:48Z

src/shapes/model/planner/TakenPlannerItem.ts

+
+export default interface TakenPlannerItem {
+  id: number;
+  item_type: 'TAKEN';


혹시 이 부분은 enum으로 처리하지 않은 이유가 있나요? 여러 파일들 확인해보니 현재 TAKEN, FUTURE, ARBITRARY 중 하나로 사용되고 있고 값 체크 하는 부분도 꽤 있어서요!

오 제가 놓친 것 같습니다! 수정해서 푸시할게요 👍

해결했습니다! (6084410) 나머지 코드에서 쓰이는 string들은 리팩토링 진행하면서 차차 enum으로 바꿔가면 좋을 것 같아요.

sboh1214

몇가지 질문들이나 제안들이 있긴 하지만, 전반적으로 좋은 것 같습니다. 수고하셨습니다 :)

sboh1214 · 2024-03-13T21:39:15Z

src/shapes/enum.ts

@@ -0,0 +1,45 @@
+export enum AdditionalTrackType {


이 친구는 const를 안하신 이유가 있나요?

제가 실수로 빠뜨린 것 같습니다... 😂

sboh1214 · 2024-03-13T21:40:07Z

src/shapes/model/graduation/AdditionalTrack.ts

@@ -0,0 +1,12 @@
+import type Department from '../subject/Department';


상대 경로 import와 절대 경로 import를 정하는 기준이 궁금합니다.

앞으로는 tsconfig.json에 정의된 대로 alias를 쓸까 생각 중입니다. 슬랙 통해서 다른 분들과 논의해 보겠습니다.

"paths": { "@/*": ["src/*"] }

sboh1214 · 2024-03-13T21:48:40Z

src/shapes/model/subject/Lecture.ts

@@ -30,11 +31,10 @@ interface lecture {
  class_title: string;
  class_title_en: string;
  review_total_weight: number;
-  professors: nestedProfessor;
+  professors: NestedProfessor;


단순한 궁금증인데, professors는 뭔가 배열이 있을거 같은데 type은 NestedProfessor 네요?

LectureShape.js 확인해본 결과 array가 맞습니다. 짚어주셔서 감사합니다. 수정하겠습니다.

sboh1214 · 2024-03-13T21:55:49Z

src/shapes/state/planner/ItemFocus.ts

@@ -0,0 +1,76 @@
+import type { ItemFocusFrom } from '@/shapes/enum';
+


여기 new line 이 있는 이유가 있나요?

파일명을 변경하기 이전 파일(ItemFocusShape.js)에 newline이 있어서 그런 것 같습니다.

sboh1214 · 2024-03-13T22:04:02Z

src/shapes/state/planner/ItemFocus.ts

+  old_code: string;
+}
+
+interface NoneItem {


interface NoneItem extends ItemProtoType { ... }

이런식으로 공통 부분을 프로토타입 인터페이스로 묶어주면 좋을 것 같아요

sboh1214 · 2024-03-13T22:09:34Z

src/shapes/state/write-reviews/ReviewsFocus.ts

+
+interface FromNone {
+  from: ReviewsFocusFrom.NONE;
+  lecture?: null;


이 property는 null | undefined 인건가요?

어디에 사용되는지 확인해보겠습니다.

yumincho requested a review from sboh1214 March 12, 2024 10:41

yumincho self-assigned this Mar 12, 2024

yumincho changed the title ~~Migration@shapes~~ Migrate remaining shapes, Complete shapes migration Mar 12, 2024

yumincho requested review from Davidace136, useruseruse and jooyeongmee March 12, 2024 10:44

yumincho added 12 commits March 13, 2024 14:14

chore: add enum.ts

fb82211

chore: convert file name and extension

308468d

.tsx -> .ts camelCase -> PascalCase

migrate: /shapes/model/timetable

abf9079

migrate: /shapes/state/dictionary

ee27f88

feat: convert ItemFocusFrom object to enum

01b261a

migrate: /shapes/state/planner

a0584c1

feat: convert LectureFocusFrom object to enum

66e2b0e

migrate: /shapes/state/timetable

ed2472a

feat: convert ReviewsFocusFrom object to enum

4ab4074

migrate: /shapes/state/write-reviews

9ed5912

migrate: /shapes/LinkShape.js

b7d7d37

chore: import only types in shapes

24a1c34

sboh1214 force-pushed the migration@shapes branch from 79bc4a8 to 24a1c34 Compare March 13, 2024 05:14

yumincho added the migrate migrate from JS CC to TS FC label Mar 13, 2024

yumincho marked this pull request as draft March 13, 2024 07:52

fix: rename link to avoid conflict with react-router-dom

6bf160b

yumincho marked this pull request as ready for review March 13, 2024 07:55

jooyeongmee reviewed Mar 13, 2024

View reviewed changes

feat: create PlannerItemType enum

6084410

sboh1214 approved these changes Mar 13, 2024

View reviewed changes

sboh1214 merged commit d6c9cf7 into migration Mar 13, 2024
3 checks passed

sboh1214 deleted the migration@shapes branch March 13, 2024 22:10

yumincho mentioned this pull request Mar 14, 2024

Complement shapes migration #48

Merged

3 tasks

yumincho added this to the TypeScript Migration milestone Mar 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate remaining shapes, Complete shapes migration #44

Migrate remaining shapes, Complete shapes migration #44

yumincho commented Mar 12, 2024 •

edited

Loading

github-actions bot commented Mar 12, 2024

github-actions bot commented Mar 12, 2024

yumincho commented Mar 13, 2024

jooyeongmee Mar 13, 2024

yumincho Mar 13, 2024

yumincho Mar 13, 2024

sboh1214 left a comment

sboh1214 Mar 13, 2024

yumincho Mar 14, 2024

sboh1214 Mar 13, 2024

yumincho Mar 14, 2024

sboh1214 Mar 13, 2024

yumincho Mar 14, 2024

sboh1214 Mar 13, 2024

yumincho Mar 14, 2024

sboh1214 Mar 13, 2024

sboh1214 Mar 13, 2024

yumincho Mar 14, 2024

		@@ -0,0 +1,12 @@
		import type Department from '../subject/Department';

		@@ -0,0 +1,76 @@
		import type { ItemFocusFrom } from '@/shapes/enum';

Migrate remaining shapes, Complete shapes migration #44

Migrate remaining shapes, Complete shapes migration #44

Conversation

yumincho commented Mar 12, 2024 • edited Loading

Description

To-do

github-actions bot commented Mar 12, 2024

github-actions bot commented Mar 12, 2024

yumincho commented Mar 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sboh1214 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yumincho commented Mar 12, 2024 •

edited

Loading