[jello] Week4 #55
[jello] Week4 #55
Conversation
- jsdom for mocking DOM and browser API
- For simple test, change access modifier of getDate method.
- `dispatch` get new state by `invoke`. - By subscribers, call every re-render methods of component with new state.
src/components/main/Main.ts
Outdated
| import { createElement } from '../../utils/domUtils'; | ||
| import { MainHeader } from './MainHeader'; |
There was a problem hiding this comment.
주가 되는 것을 default로 export하고 부가적인 것들을 그냥 export하면 짐작을 통한 불필요한 연산없이 예를 들어서, 어떤게 component인건지 구분이 바로 되서 저는 개인적으로는 이게 좋은거 같아요.
import MainHeader, { MainHeaderProps } from "./MainHeader";
src/components/main/MainHeader.ts
Outdated
| @@ -1,15 +1,15 @@ | |||
| import { invoke } from '../../store'; | |||
| import { dispatch, subscribe } from '../../dispatch'; | |||
There was a problem hiding this comment.
Vite 및 TS에 path alias를 설정해두시면 더 명확하고 안정적이게 import 할 수 있을것 같습니다.
Ex: "@store/dispatch", "@utils/domUtils"
src/components/main/MainHeader.ts
Outdated
| private tabs; | ||
| private viewers; |
There was a problem hiding this comment.
createNavElement라는 함수로 nav용의 UI를 만드는것 같은데 tabs, viewers 이름만봐서 정확히 어떤 nav를 담당하는지 헷갈리는 것 같습니다.
package.json
Outdated
| "build": "tsc && vite build", | ||
| "preview": "vite preview" | ||
| }, | ||
| "devDependencies": { | ||
| "@vitest/ui": "^0.32.0", | ||
| "jsdom": "^22.1.0", |
There was a problem hiding this comment.
jsdom library를 직접적으로 사용하는 것이 아니기 때문에 불필요한것 같습니다.
src/components/main/MainHeader.ts
Outdated
| @@ -24,6 +24,7 @@ export default class MainHeader { | |||
|
|
|||
| this.element.append(tabNav, viewerNav); | |||
| this.setEvent(); | |||
| subscribe(this.updateView.bind(this)) | |||
There was a problem hiding this comment.
subscribe는 함수를 받는데 이 함수가 기대하는 type이 this.updateView()와 다른거 같은데 어떻게 동작이 되는건가요?
src/dispatch.ts
Outdated
| const state = invoke(action)!; | ||
| if (state) { |
There was a problem hiding this comment.
Non-null assertion 혹은 type guard 중복이 되서 둘 중하나만 해주시면 될것 같습니다.
src/tests/MainHeader.test.ts
Outdated
| mainHeader.updateView(state); | ||
| const gridViewIcon = mainHeader.element.querySelector('[data-viewer="gridView"]'); | ||
|
|
||
| expect(gridViewIcon?.classList.contains(style.active_viewer!)).toBe(true); |
There was a problem hiding this comment.
toContain이라는 matcher도 이용하실 수 있습니다.
expect(gridviewIcon?.classList).toContain(style.active_viewer!);- @, @components, @utils - export default for class
- Because its role overlaps with the type guard's.
- `toContain` cannot apply for DOMTokenList which is value of element's classList property. - With conditional statement and Array.from() method, test is passed.
- Subscribe every components
순환 참조 제거
main.ts와 store.ts 사이에 순환 참조로 테스트 실패.
중간에서 매개하는 모듈을 만들면 어떨까?
초기 state를 받아서 newsStand 컴포넌트를 생성하는 동작은 필요하기 때문에 getStore 함수는 필요하다.
그럼, 나머지 onChangeState 함수를 대체할 뭔가가 필요하다.
dispatch라는 것을 만들어서, invoke 후에 새로운 state를 반환받고 newsStand 컴포넌트의 updateView 메소드를 호출하면서 인자로 넣어준다.
결과 아래와 같이 다른 순환 참조로 인한 에러가 발생한다.
dispatch 안에서 main을 읽어오게 되면 NewsStand를 따라 모든 컴포넌트를 읽어와야한다.
마찬가지로 MainHeader를 실행하기 전에 dispatch를 읽어오다가 다시 MainHeader를 사용하는 코드를 읽어야하므로 참조의 순환이 발생한다.
MainHeader 컴포넌트의 재렌더링 메서드를 전달해서 저장하고 dispatch 함수의 마지막에 호출해서 에러를 제거했다.
리뷰 포인트
테스트보다는 에러를 제거하는데 시간을 더 썼네요.
리뷰어분이라면 어떤 식으로 이 문제를 해결하셨을 것 같은지 듣고 싶습니다.