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

[8팀 김도운] [Chapter 1-2] 프레임워크 없이 SPA 만들기 #4

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

devJayve
Copy link

@devJayve devJayve commented Dec 22, 2024

과제 체크포인트

기본과제

가상돔을 기반으로 렌더링하기

  • createVNode 함수를 이용하여 vNode를 만든다.
  • normalizeVNode 함수를 이용하여 vNode를 정규화한다.
  • createElement 함수를 이용하여 vNode를 실제 DOM으로 만든다.
  • 결과적으로, JSX를 실제 DOM으로 변환할 수 있도록 만들었다.

이벤트 위임

  • 노드를 생성할 때 이벤트를 직접 등록하는게 아니라 이벤트 위임 방식으로 등록해야 한다
  • 동적으로 추가된 요소에도 이벤트가 정상적으로 작동해야 한다
  • 이벤트 핸들러가 제거되면 더 이상 호출되지 않아야 한다

심화 과제

1) Diff 알고리즘 구현

  • 초기 렌더링이 올바르게 수행되어야 한다
  • diff 알고리즘을 통해 변경된 부분만 업데이트해야 한다
  • 새로운 요소를 추가하고 불필요한 요소를 제거해야 한다
  • 요소의 속성만 변경되었을 때 요소를 재사용해야 한다
  • 요소의 타입이 변경되었을 때 새로운 요소를 생성해야 한다

2) 포스트 추가/좋아요 기능 구현

  • 비사용자는 포스트 작성 폼이 보이지 않는다
  • 비사용자는 포스트에 좋아요를 클릭할 경우, 경고 메세지가 발생한다.
  • 사용자는 포스트 작성 폼이 보인다.
  • 사용자는 포스트를 추가할 수 있다.
  • 사용자는 포스트에 좋아요를 클릭할 경우, 좋아요가 토글된다.

과제 셀프회고

React의 핵심 개념들에 대한 심층적인 이해가 부족했던 터라, Virtual DOM의 본질적인 메커니즘에 대해서도 피상적으로만 이해하고 있었습니다.
누군가 Virtual DOM에 대해 물어본다면 아마 이렇게 답변했을 것 같습니다..

"가상돔은 diffing 알고리즘으로 DOM 업데이트를 최적화해서 빠르게 돌아간다!"

그래서 준일 코치님의 발제 시간에 들었던 "가상 돔이 항상 성능 향상을 보장하지 않으며
작은 규모에선 오히려 오버헤드가 될 수 있다"는 말씀이 나름의 충격이었습니다. 그렇다보니 가상 돔을 이론적인 글로 살펴보는
것 뿐만 아니라 직접 구현해보며 더 구체적으로 케이스를 공부할 수 있어 더더욱 좋은 기회였습니다. 주로 학습한 내용은
다음과 같습니다.

  • 가상돔을 생성하는 파이프라인에 대한 이해
  • 가상돔의 필요성과 효율적인 관리 (Garbage Collection 기반 동작 등)
  • 합성 이벤트 시스템과 이벤트 위임 메커니즘 설계

가장 고민하며 구현한 부분은 이벤트 처리 시스템입니다. DOM 이벤트를 루트 컨테이너에 위임하되, 이벤트 핸들러들을 Map에 저장하고
유지하는 방식으로 설계했습니다. 여기에 리액트의 합성 이벤트 시스템 방식을 차용하여 핵심 코드를 리팩토링하였습니다. 이를 위해 리액트 소스코드를 Deep Dive(Shallow Dive에 가까운..) 해보았습니다.

React 톺아보기 (이벤트 시스템)

React 라이브러리 코드를 직접 보며 해석한 내용이다보니 잘못된 내용에 대한 지적, 피드백을 적극 환영합니다 🙌

이벤트 처리의 핵심 파이프라인을 따라가보겠습니다. 리액트는 루트 컨테이너를 생성하면서 이벤트 리스너를 등록합니다.

export function createRoot(container, options) {
  
  // rootContainer를 설정
  const rootContainerElement = container.nodeType === COMMENT_NODE  
    ? container.parentNode  
    : container;


  // rootContainer에 지원하는 이벤트에 대한 리스너 등록
  listenToAllSupportedEvents(rootContainerElement);

listenToAllSupportedEvents는 Native Event Set을 순회하면서 각각의 이벤트 마다
개별적으로 리스너를 등록합니다. 이때 이벤트 위임 여부에 따라 적절한 플래그를 설정하고 이벤트를 등록합니다.

export function listenToAllSupportedEvents(rootContainerElement) {
  ...
  allNativeEvents.forEach(domEventName => {
    if (!nonDelegatedEvents.has(domEventName)) {  
      listenToNativeEvent(domEventName, false, rootContainerElement);  
    }  
    listenToNativeEvent(domEventName, true, rootContainerElement);  
  })
}

listenToNativeEvent 함수 내에서 flag에 대한 별도의 처리를 한 뒤 addTrappedEventListener를 호출합니다. createEventListenerWrapperWithPriority
함수를 통해 어떤 우선순위에 따라 listener를 가져오고 해당 리스너를 flag 값에 따라 이벤트 버블 리스너, 이벤트 캡쳐 리스너로 분류하여 등록합니다.

function addTrappedEventListener(
  targetContainer,
  domEventName,
  eventSystemFlags,
  isCapturePhaseListener,
  isDeferredListenerForLegacyFBSupport
) {
  let listener = createEventListenerWrapperWithPriority(
    targetContainer,
    domEventName,
    eventSystemFlags,
  );
  if (isCapturePhaseListener) { // 캡쳐 페이즈 리스너
    unsubscribeListener = addEventCaptureListener(
      targetContainer,
      domEventName,
      listener,
    );
  } else { // 버블 페이즈 리스너
    unsubscribeListener = addEventBubbleListener(
      targetContainer,
      domEventName,
      listener,
    );
  }
}

이제 리스너가 최종적으로 등록되는 시점은 알았으니 우선순위에 따라 리스너를 생성하는 로직을 살펴보겠습니다. 아래 getEventPriority의 경우 이벤트 이름에 따라
리액트에서 지정한 우선순위를 구별하여 값을 가져옵니다. 여기서 우선순위에 따라 각각 다른 함수를 변수에 할당해주고 있는데 핵심은 모두 같은 dispatchEvent를 기반으로
두고 있다는 것입니다. 결국 클라이언트에서 실제 이벤트가 발생되었을 때 dispatchEvent가 트리거되게 됩니다.

export function createEventListenerWrapperWithPriority(
  targetContainer,
  domEventName,
  eventSystemFlags
) {
  const eventPriority = getEventPriority(domEventName);
  let listenerWrapper;
  switch (eventPriority) {
    case DiscreteEventPriority:
      listenerWrapper = dispatchDiscreteEvent;
      break;
    case ContinuousEventPriority:
      listenerWrapper = dispatchContinuousEvent;
      break;
    case DefaultEventPriority:
    default:
      listenerWrapper = dispatchEvent;
      break;
  }
}

dispatchEvent는 몇 가지 과정을 거쳐 dispatchEventsForPlugins함수에 도달하게 됩니다.
해당 함수에서는 extractEvents -> processDispatchQueue 순으로 호출됩니다.
먼저 extractEvent의 흐름을 살펴보겠습니다.

  1. 해당 함수는 이벤트명을 기준으로 합성이벤트를 생성
  2. target 컨테이너를 기준으로 탐색하며 리스너를 수집 (accumulateSinglePhaseListeners)
  3. 합성이벤트와 리스너들을 dispatchQueue에 삽입

위와 같은 과정을 통해 합성이벤트 생성과 target 컨테이너에서 리스너를 수집하여 등록하였다면 processDispatchQueue
를 통해 적절한 우선순위에 따라 큐 안의 이벤트들을 처리하게 됩니다.
실제 리액트에서는 훨씬 더 복잡한 엣지 케이스들을 고려하고 있지만 이번 분석을 통해 리액트 이벤트 시스템의 아키텍쳐를 이해할 수 있는 좋은 기회였습니다.

합성 이벤트 실제 구현

위와 같은 리액트의 접근 방식을 참고하여 합성 이벤트 시스템을 구현해보았습니다. 먼저 지원하는 이벤트명을 기준으로 각각 리스너를 등록합니다.

export function setupEventListeners(root) {
  supportedEventNames.forEach((eventName) => {
    listenToNativeEvent(root, eventName);
  });
}

이때 리스너가 트리거될 경우 dispatchEvent가 호출됩니다. 위에서 리액트의 흐름과 같이 extractEvent함수를 통해
합성 이벤트를 생성하고, target 컨테이너를 기준으로 버블링 순회하며 핸드러를 수집합니다.

const syntheticEvent = extractEvent(
  domEventName,
  nativeEvent,
  nativeEvent.target,
);
const dispatchQueue = accumulateListeners(
  nativeEvent.target,
  targetContainer,
  domEventName,
);

리뷰 받고 싶은 내용

  • Slack에서 최기환 학습 메이트님이 리뷰해주신 내용처럼 WeakMap을 사용할 경우 GC에 의해 컨테이너가 소멸되었을
    때 자동적으로 Map에서 제거되는 것으로 이해했습니다. 이때 지금 코드 상에서 구현한 부분처럼 컨테이너 변수로
    할당해주어도 해당 container로 할당해주어도 컨테이너 소멸 시 메모리 상 제거되는 것으로 알고 있는데 이게 맞는 정보인지 궁금합니다.
const oldVNode = container._vnode;
...
container._vnode = normalizedVNode;

@devJayve devJayve changed the title [8팀 김도운] [Chapter 1-2] 프레임워크 없이 SPA 만들기 (2) [8팀 김도운] [Chapter 1-2] 프레임워크 없이 SPA 만들기 Dec 22, 2024
Copy link

@wonjung-jang wonjung-jang left a comment

Choose a reason for hiding this comment

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

전체적으로 보면서 도운님께서 세심하게 고민하고 구현하신 것 같아서 감탄만 하고 갑니다...👍
특히 합성 이벤트를 만들고 처리하는 부분은 토요일 발제날에 따로 알려주실 수 있나요??
이벤트 처리 시에 모든 로직에 try-catch를 사용해서 에러 핸들링까지 하시고 도운님의 코드를 보면서 나는 과제 통과만 생각하고 고민하지 않았다는 반성을 하게 되네요.

Comment on lines +7 to +18
const { toggleLike } = globalStore.actions;
const { currentUser } = globalStore.getState();
const activationLike =
currentUser && likeUsers.includes(currentUser.username);

const handleToggleLike = () => {
if (!loggedIn) {
alert("로그인 후 이용해주세요");
return;
}
toggleLike(id);
};

Choose a reason for hiding this comment

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

globalStore에서 currentUser를 받아오는 거라면 loggedIn 또한 받아 올 수 있을 것 같은데 loggedIn은 컴포넌트 인자로 받으신 이유가 있을까요?

비슷한 이유로 toggleLikeglobalStore에서 관리하면 로그인 검사 로직을 안으로 넣을 수 있을 것 같은데 로그인 검사는 관심사가 달라서 일부러 분리하신 건지 궁금합니다!

Copy link
Author

Choose a reason for hiding this comment

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

  1. loggedIn을 Home에서 먼저 사용하다보니 다시 선언하기보단 재사용하기 위해 Post 컴포넌트에 props로 넘겨주는게 더 효율적이라고 생각했어요. 근데 또 다시 생각해보면 React에서 Provider 이용해서 합성 컴포넌트 만들듯이 PostPage에서 선언하고 분리하는게 더 깔끔할 수 있다는 생각이 드네요..!

  2. alert를 화면에 보여주는게 UI에 관여하는 작업이라고 생각이 들어서 비즈니스 로직과 분리하기 위해 의도적으로 이렇게 작성했습니다!

e.preventDefault();
const contentInput = document.getElementById("post-content");
const content = contentInput.value.trim();
if (!content) return;

Choose a reason for hiding this comment

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

content가 없을 경우 반환한다.
당연한 건데 저는 작성을 안 해놓은 것 같네요.
도운님의 세심함을 배워야 할 것 같습니다!👍

placeholder="무슨 생각을 하고 계신가요?"
className="w-full p-2 border rounded"
/>
<button
id="post-submit"
type="submit"

Choose a reason for hiding this comment

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

form안에 button태그의 type 기본값이 submit이라 생략할 수도 있을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

아 그렇네요 이걸 까먹고 있었는데 감사합니다 ㅎㅎ

Comment on lines +7 to +43
function updateAttributes(target, newProps, oldProps) {
const allProps = new Set([
...Object.keys(newProps),
...Object.keys(oldProps),
]);

export function updateElement(parentElement, newNode, oldNode, index = 0) {}
allProps.forEach((prop) => {
if (prop === "key" || prop === "children") return;

const newValue = newProps[prop];
const oldValue = oldProps[prop];

// 값이 같으면 스킵
if (newValue === oldValue) return;

// 이벤트 핸들러 처리
if (prop.startsWith("on")) {
const eventType = prop.slice(2).toLowerCase();

// 이전 핸들러 제거
if (oldValue) {
removeEvent(target, eventType, oldValue);
}

// 새 핸들러 등록
if (newValue) {
addEvent(target, eventType, newValue);
}

return;
}

// 일반 속성 처리
const attrName = prop === "className" ? "class" : prop;
target.setAttribute(attrName, newValue);
});
}

Choose a reason for hiding this comment

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

와.. 이렇게 하니까 반복문을 두 번 호출할 필요없이 한 번에 가능하고 코드도 훨씬 깔끔해졌네요..!👍
className을 갖고 있을 때 별도의 조건문으로 추가한게 아닌 삼항 연산자를 통해서 attrName을 바꾸고 setAttributes를 통해서 한 번에 속성을 추가하는 것도 너무 좋은 방법 같아요!👍
많이 배우고 갑니다 도운님!👍

Comment on lines +15 to +32
// root 엘리먼트에 합성 이벤트로 처리되는 리스너 등록
setupEventListeners(container);

// 이전 가상 DOM 가져오기
const oldVNode = container._vnode;

// 새로운 가상 DOM 정규화
const normalizedVNode = normalizeVNode(vNode);

if (!oldVNode) {
// 최초 렌더링
container.appendChild(createElement(normalizedVNode));
} else {
updateElement(container, normalizedVNode, oldVNode, 0);
}

// 현재 가상 DOM 저장
container._vnode = normalizedVNode;

Choose a reason for hiding this comment

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

제 생각에는 container의 속성에 직접 oldVNode를 저장하게 되면 개발자 도구로 container에 접근할 수 있기 때문에 속성보다는 별도의 변수에 저장하는 게 더 좋을 것 같다는 생각이 듭니다.

Copy link
Author

Choose a reason for hiding this comment

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

container에 내부 속성에 oldVNode를 저장해도 어차피 container가 소멸되는 시점에 같이 소멸되니깐 상관없지 않을까?라고 생각했는데 원정님 말씀 듣고보니 디버깅이나 접근하는 관점에서 WeakMap을 쓰는게 더 명확한 방법이겠네요..!! 좋은 피드백 감사합니다 bb

Copy link

@Geunbaek Geunbaek left a comment

Choose a reason for hiding this comment

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

전체적으로 다 잘 작성하셨지만 특히나 합성이벤트 인터페이스를 모두 작성하신 부분에서 감명받았습니다 bb!

Comment on lines +1 to +138
stopPropagation: function () {
const event = this.nativeEvent;
if (!event) return;
if (event.stopPropagation) {
event.stopPropagation();
}
this.isPropagationStopped = () => true;
},
});

return SyntheticBaseEvent;
}

// 기본 이벤트 인터페이스 정의
const EventInterface = {
eventPhase: 0,
bubbles: 0,
cancelable: 0,
defaultPrevented: 0,
};

// UI 이벤트 (click, focus 등의 기본이 되는 이벤트)
const UIEventInterface = {
...EventInterface,
view: 0,
detail: 0,
};

// 터치 이벤트
const TouchEventInterface = {
...UIEventInterface,
touches: 0,
targetTouches: 0,
changedTouches: 0,
altKey: 0,
metaKey: 0,
ctrlKey: 0,
shiftKey: 0,
};

// 마우스 이벤트 (click, hover 등)
const MouseEventInterface = {
...UIEventInterface,
screenX: 0,
screenY: 0,
clientX: 0,
clientY: 0,
pageX: 0,
pageY: 0,
ctrlKey: 0,
shiftKey: 0,
altKey: 0,
metaKey: 0,
button: 0,
buttons: 0,
};

// 드래그 이벤트
const DragEventInterface = {
...MouseEventInterface,
dataTransfer: 0,
};

// 터치 이벤트
const FocusEventInterface = {
...UIEventInterface,
relatedTarget: 0,
};

// 휠 이벤트 (마우스 휠, 트랙 패드)
const WheelEventInterface = {
...MouseEventInterface,
deltaX(event) {
return "deltaX" in event
? event.deltaX
: "wheelDeltaX" in event
? -event.wheelDeltaX
: 0;
},
deltaY(event) {
return "deltaY" in event
? event.deltaY
: "wheelDeltaY" in event
? -event.wheelDeltaY
: "wheelDelta" in event
? -event.wheelDelta
: 0;
},
deltaZ: 0,
deltaMode: 0,
};

// 각 이벤트 타입별 합성 이벤트 생성자 export
export const SyntheticEvent = createSyntheticEvent(EventInterface);
export const SyntheticUIEvent = createSyntheticEvent(UIEventInterface);
export const SyntheticTouchEvent = createSyntheticEvent(TouchEventInterface);
export const SyntheticMouseEvent = createSyntheticEvent(MouseEventInterface);
export const SyntheticDragEvent = createSyntheticEvent(DragEventInterface);
export const SyntheticFocusEvent = createSyntheticEvent(FocusEventInterface);
export const SyntheticWheelEvent = createSyntheticEvent(WheelEventInterface);

Choose a reason for hiding this comment

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

와우 이걸 다 정의하셨군요.. bb!

Comment on lines +54 to +58
if (!newNode && oldNode) {
// Case 1: 노드 삭제
parentElement.removeChild(parentElement.childNodes[index]);
return;
}

Choose a reason for hiding this comment

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

혹시 노드가 삭제되거나 교체되었을때 해당 노드에 걸려있는 이벤트 핸들러나 해당 노드의 자식 노드의 이벤트 핸들러는 정리가 되나요?

저는 이런경우 혹시 모른다는 생각에 WeakMap 객체를 사용하긴했는데, 궁금해서 질문드립니다 !

Copy link
Author

Choose a reason for hiding this comment

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

네 맞습니다 저도 비슷한 생각에 event 리스너를 등록하는 eventStore를 WeakMap으로 선언하기도했고 oldVNode 자체도 현재 로직 상 container가 소멸됨에 따라 같이 GC에 의해 제거되는 걸로 알고있어서 문제는 없었던 것 같아요 근데 좀 더 자세히 디버깅이 필요할 듯 합니다..


// 이벤트 취소/전파 중단 상태 초기화
const defaultPrevented = nativeEvent.defaultPrevented ?? false;
this.isDefaultPrevented = () => defaultPrevented;

Choose a reason for hiding this comment

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

혹시 이 메서드를 통해서 어떤 역할을 수행하는지 여쭤봐도될까요?

Copy link
Author

Choose a reason for hiding this comment

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

nativeEvent.defaultPrevented값은 nativeEvent 자체가 가지고 있는 속성 값이에요. event.preventDefault()를 호출했는지에 대한 값을 가지고 있습니다! Event.defaultPrevented 그래서 해당 값이 없으면 기본값으로 false를 사용해요. 그리고 this.isDefaultPrevented의 경우 현재 defaultPrevented 값을 확인할 수 있는 메서드입니다.

정리하면,

  1. client → e.preventDefault() 호출
  2. native.defaultPrevented → true
  3. client → e.isDefaultPrevented로 기본 동작 취소 확인 가능

Choose a reason for hiding this comment

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

그럼 하위에서 기본 이벤트 동작을 취소하면 버블링되면서 해당 이벤트들의 기본동작들을 모두 취소하는건가요??

* @param Interface Interface 이벤트 타입 별 속성 정의
* @returns {function(*, *, *, *): SyntheticBaseEvent} 합성 이벤트 생성자 함수
*/
function createSyntheticEvent(Interface) {

Choose a reason for hiding this comment

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

와 이거 코드 내일 설명해 주시면 안될까요?!

클래스가 아닌 함수로 구현하신 이유도 궁금합니다.

패턴 명도 궁금하구요!

Copy link
Author

Choose a reason for hiding this comment

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

발제시간에 얘기해보도록 하겠습니다 ㅎㅎ

Copy link

@osohyun0224 osohyun0224 left a comment

Choose a reason for hiding this comment

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

안녕하세요 도운님 :D 담당 학습메이트 오소현입니다~!

이번주차 우리팀 과제 제일 빠르게 완수하신 도운님 ⭐️⭐️ 빠르게 요구사항 파악하고 과제를 착착 진행하시는 모습이 정말 인상적이었습니다 ㅎㅎ 코어타임때도 팀원분들이 이슈 생길 때 잘 도와주시고 항상 너무 감사합니다!! 그리고� 도운님 다른 분들 코드 리뷰 해주셔서 넘 감사했습니다 최고!! 🏅🏅

PR에 작성해주신 React의 Virtual DOM과 이벤트 시스템에 대해서 학습하고 구현해주신 내용에 대해 잘 정리해주셔서 얼마나 열심히 하셨는지,,, 알 수 있었습니다 bb 최고예요!! 특히 더 좋았던 점은 가상 DOM이 항상 성능 향상을 보장하지 않는다는 것을 깨달으신 점과 구체적인 사례를 들어주셔서 리뷰하는 입장에서 엄청 좋았습니다 bb 항상 깔끔하게 도운님 생각 정리해주시고 너무 좋습니다 언제나 좋은 글 작성해주셔서 너무 감사해요 :)

특히 과제에서는 다양한 이벤트를 정의해두시고 , 각 이벤트 타입별 합성 이벤트 생성자를 정의해두신게 정말 좋았습니다 bb 이렇게 사용자에게 발생하는 다양한 이벤트를 구현해두시고,, 최고입니다 !!
이렇게 생각하기까지 여러번 고려해주시는 bb 정말 최고네요

이번주 과제하시느라 너무 고생많으셨어요 ㅎㅎ 부족하지만 도운님의 코드에 리뷰를 달아보았습니다 제 리뷰가 이번주 과제를 한 번 더 되돌아 볼 수 있는 좋은 기회가 되었으면 좋겠어요 :)🍀🍀
다음주 2주차 과제도 화이팅입니다 👍🏻👍🏻

const { addPost } = globalStore.actions;

const handlePostSubmit = (e) => {
e.preventDefault();

Choose a reason for hiding this comment

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

e.preventDefault() 을 추가해주셔서 제출 동작을 막아주시고 계시는 군요!! 잘 추가해주신 것 같습니다 bb

Comment on lines +7 to +53
export function createElement(vNode) {
// 문자열이나 숫자 처리
if (typeof vNode === "string" || typeof vNode === "number") {
return document.createTextNode(String(vNode));
}

function updateAttributes($el, props) {}
// null, undefined, boolean 처리
if (vNode == null || typeof vNode === "boolean") {
return document.createTextNode("");
}

// 배열(fragment) 처리
if (Array.isArray(vNode)) {
const fragment = document.createDocumentFragment();
vNode.forEach((child) => {
const childElement = createElement(child);
fragment.appendChild(childElement);
});
return fragment;
}

const element = document.createElement(vNode.type);

// props 처리
if (vNode.props) {
Object.entries(vNode.props).forEach(([key, value]) => {
if (key === "className") {
element.setAttribute("class", value);
} else if (key.startsWith("on") && typeof value === "function") {
const eventType = key.slice(2).toLowerCase();
if (supportedEventNames.has(eventType)) {
addEvent(element, eventType, value);
}
} else if (key !== "key" && key !== "children") {
element.setAttribute(key, value);
}
});
}

// 자식 노드 처리
vNode.children.forEach((child) => {
const childElement = createElement(child);
element.appendChild(childElement);
});

return element;
}

Choose a reason for hiding this comment

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

키야 createElement 함수 내부를 각 역할에 맞게 잘 작성해주셨네요 bb 최고입니다 각 vNode의 유형에 따라 Dom 요소 생성 로직이 잘 구현되어있는 것 같습니다 !!

이미 완벽하지만 저는 확장성을 고려해본다면 createElement 내부

  1. 속성을 설정
  2. 이벤트 처리 부분
  3. 자식 요소 추가

위의 3부분을 모듈화 해서 작은 함수로 분할해보면 좋을 것 같아요!

Suggested change
export function createElement(vNode) {
// 문자열이나 숫자 처리
if (typeof vNode === "string" || typeof vNode === "number") {
return document.createTextNode(String(vNode));
}
function updateAttributes($el, props) {}
// null, undefined, boolean 처리
if (vNode == null || typeof vNode === "boolean") {
return document.createTextNode("");
}
// 배열(fragment) 처리
if (Array.isArray(vNode)) {
const fragment = document.createDocumentFragment();
vNode.forEach((child) => {
const childElement = createElement(child);
fragment.appendChild(childElement);
});
return fragment;
}
const element = document.createElement(vNode.type);
// props 처리
if (vNode.props) {
Object.entries(vNode.props).forEach(([key, value]) => {
if (key === "className") {
element.setAttribute("class", value);
} else if (key.startsWith("on") && typeof value === "function") {
const eventType = key.slice(2).toLowerCase();
if (supportedEventNames.has(eventType)) {
addEvent(element, eventType, value);
}
} else if (key !== "key" && key !== "children") {
element.setAttribute(key, value);
}
});
}
// 자식 노드 처리
vNode.children.forEach((child) => {
const childElement = createElement(child);
element.appendChild(childElement);
});
return element;
}
export function createElement(vNode) {
if (typeof vNode === "string" || typeof vNode === "number") {
return document.createTextNode(String(vNode));
}
if (vNode == null || typeof vNode === "boolean") {
return document.createTextNode("");
}
if (Array.isArray(vNode)) {
return createFragment(vNode);
}
return createHTMLElement(vNode);
}
function createFragment(vNodes) {
const fragment = document.createDocumentFragment();
vNodes.forEach(child => {
fragment.appendChild(createElement(child));
});
return fragment;
}
function createHTMLElement(vNode) {
const element = document.createElement(vNode.type);
setProps(element, vNode.props);
appendChildren(element, vNode.children);
return element;
}
function setProps(element, props) {
Object.entries(props || {}).forEach(([key, value]) => {
if (key === "className") {
element.setAttribute("class", value);
} else if (key.startsWith("on") && typeof value === "function") {
handleEvent(element, key, value);
} else if (key !== "key" && key !== "children") {
element.setAttribute(key, value);
}
});
}
function handleEvent(element, key, value) {
const eventType = key.slice(2).toLowerCase();
if (supportedEventNames.has(eventType)) {
addEvent(element, eventType, value);
}
}
function appendChildren(element, children) {
children.forEach(child => {
element.appendChild(createElement(child));
});
}

저는 우선 이렇게 재구현을 해봤는데 테스트는 통과하네요! 저도 좋은 코드인지는 모르겠지만 제 제안 코드를 보고 아 이 함수를 요렇게도 기능 단위로 분리할 수 있구나!를 참고해주셨으면 좋겠습니다 ㅎㅎ

Copy link
Author

Choose a reason for hiding this comment

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

suggestion 작성하시고 테스트까지 직접 돌려보신게 정말 대단하시네요.. bb 사실 createElement가 과제에서 지정해준 함수 양식이라고 생각해 모듈화를 해볼 생각을 못했는데 좋은 케이스인 것 같습니다 !!

Comment on lines +15 to +19
for (let prop in Interface) {
if (Object.hasOwn(Interface, prop)) {
this[prop] = Interface[prop];
}
}

Choose a reason for hiding this comment

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

오호 여기서 혹시 모든 속성을 반복해서 복사하는 방식으로 구현되고 있을까용?
필요한 속성만 선택적으로 복사하는 방식이 있다면 어떻게 구현하는 게 좋을지 궁금하네용 !!

Copy link
Author

Choose a reason for hiding this comment

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

제가 이해하기론 Object.hasOwn()을 통해 해당 객체가 직접 소유한 속성만 복사하는 방식이에요!

for (let prop in Interface) {
    this[prop] = Interface[prop];
}

예를 들어 위와 같이 할 경우 toString, valueOf와 같은 프로토타입 체인에 있는 모든 속성들까지 복사되기 때문에 Object.hasOwn()을 통해 Interface에서 필요한 속성만 복사하고 있습니다

Comment on lines +132 to +138
export const SyntheticEvent = createSyntheticEvent(EventInterface);
export const SyntheticUIEvent = createSyntheticEvent(UIEventInterface);
export const SyntheticTouchEvent = createSyntheticEvent(TouchEventInterface);
export const SyntheticMouseEvent = createSyntheticEvent(MouseEventInterface);
export const SyntheticDragEvent = createSyntheticEvent(DragEventInterface);
export const SyntheticFocusEvent = createSyntheticEvent(FocusEventInterface);
export const SyntheticWheelEvent = createSyntheticEvent(WheelEventInterface);

Choose a reason for hiding this comment

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

와,,, 이런 다양한 이벤트들 정의,,하시고 그냥 최곱니다,,, bb 이걸 사용하는 쪽 코드가 궁금해지네요

Copy link
Author

@devJayve devJayve Dec 28, 2024

Choose a reason for hiding this comment

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

사실 간단하게 작동하는지 테스트는 해보았지만 직접 적용해보진 못했습니다 ㅠㅠ 현재는 click과 관련한 MouseEvent만 주로 활용되고 있어요 시간이 만약 더 있었다면 wheel 이벤트 등을 활용해서 무한 pagination + throttling을 적용해보고싶다는 생각만 해보았습니다 😂

Comment on lines +132 to +138
export const SyntheticEvent = createSyntheticEvent(EventInterface);
export const SyntheticUIEvent = createSyntheticEvent(UIEventInterface);
export const SyntheticTouchEvent = createSyntheticEvent(TouchEventInterface);
export const SyntheticMouseEvent = createSyntheticEvent(MouseEventInterface);
export const SyntheticDragEvent = createSyntheticEvent(DragEventInterface);
export const SyntheticFocusEvent = createSyntheticEvent(FocusEventInterface);
export const SyntheticWheelEvent = createSyntheticEvent(WheelEventInterface);

Choose a reason for hiding this comment

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

도운님 이벤트를 각 특징의 표로 정리해봤어요~! 리뷰하시는 분들 참고해주세요!

이벤트 타입 주요 속성 사용 사례
SyntheticEvent bubbles, cancelable 모든 이벤트의 기본
SyntheticUIEvent view, detail 사용자 인터페이스 이벤트
SyntheticMouseEvent clientX, clientY, button 클릭, 호버 등
SyntheticTouchEvent touches, targetTouches 모바일 터치 동작
SyntheticDragEvent dataTransfer 드래그 앤 드롭
SyntheticWheelEvent deltaX, deltaY 스크롤, 줌

}
currentTarget = currentTarget.parentNode;
}
return dispatchQueue;

Choose a reason for hiding this comment

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

return dispatchQueue.reverse(); 이렇게 배열을 뒤집어서 이벤트 버블링 순서대로 처리하도록 해보는건 어떨까요오오????

Copy link
Author

Choose a reason for hiding this comment

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

엇 제가 �이해하기론 버블링 순서대로 실행되고 있는 것 같아요!

<div id="root>
  <div onclick="console.log('div')">
    <section onclick="console.log('section')">
      <button onclick="console.log('button')">클릭</button>
    </section>
  </div>
</div>
  • 위 코드에서 button에서 onClick 발생 시 currentTarget → button, rootContainer → root
  • button에서 시작해서 root 직전
    까지 핸들러 수집
  • queue → [buttonHandler, sectionHandler, divHandler]
  • 이제 dispatchQueue를 첫 번째 인덱스부터 실행하므로 buttonHandler → sectionHandler → divHandler 순으로 실행되는 것 같은데 제가 잘못 이해한걸까요 ?!

Comment on lines +31 to +47
export function extractEvent(domEventName, nativeEvent, nativeEventTarget) {
if (!supportedEventNames.has(domEventName)) {
console.error("Unsupported Event :", domEventName);
return;
}
let SyntheticEventConstructor = SyntheticEvent;
let eventType = domEventName;
switch (domEventName) {
case "click":
case "mousedown":
case "mousemove":
case "mouseup":
case "mouseout":
case "mouseover":
case "contextmenu":
SyntheticEventConstructor = SyntheticMouseEvent;
break;

Choose a reason for hiding this comment

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

위의 코드에서 한 함수 내부에서 매핑하는 로직까지 같이 있으니까 이벤트 매칭하는 거는 다음과 같이 분리해보면 어떨까요???

Suggested change
export function extractEvent(domEventName, nativeEvent, nativeEventTarget) {
if (!supportedEventNames.has(domEventName)) {
console.error("Unsupported Event :", domEventName);
return;
}
let SyntheticEventConstructor = SyntheticEvent;
let eventType = domEventName;
switch (domEventName) {
case "click":
case "mousedown":
case "mousemove":
case "mouseup":
case "mouseout":
case "mouseover":
case "contextmenu":
SyntheticEventConstructor = SyntheticMouseEvent;
break;
const eventTypeMapping = {
click: SyntheticMouseEvent,
mousedown: SyntheticMouseEvent,
mousemove: SyntheticMouseEvent,
mouseup: SyntheticMouseEvent,
mouseout: SyntheticMouseEvent,
mouseover: SyntheticMouseEvent,
contextmenu: SyntheticMouseEvent,
drag: SyntheticDragEvent,
dragstart: SyntheticDragEvent,
dragend: SyntheticDragEvent,
dragenter: SyntheticDragEvent,
dragleave: SyntheticDragEvent,
dragover: SyntheticDragEvent,
drop: SyntheticDragEvent,
touchstart: SyntheticTouchEvent,
touchmove: SyntheticTouchEvent,
touchend: SyntheticTouchEvent,
touchcancel: SyntheticTouchEvent,
scroll: SyntheticUIEvent,
wheel: SyntheticWheelEvent,
focusin: SyntheticFocusEvent,
focusout: SyntheticFocusEvent,
select: SyntheticUIEvent,
keydown: SyntheticUIEvent,
keyup: SyntheticUIEvent,
keypress: SyntheticUIEvent,
change: SyntheticUIEvent,
input: SyntheticUIEvent,
submit: SyntheticUIEvent,
blur: SyntheticFocusEvent,
focus: SyntheticFocusEvent,
};

Copy link
Author

@devJayve devJayve Dec 28, 2024

Choose a reason for hiding this comment

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

eventTypeMapping을 만들경우 switch문 구문의 길이가 줄어서 핵심 로직이 더욱 간결해져서 좋을 것 같아요! 다만 걱정되는 부분은

  1. SyntheticDragEvent, SyntheticUIEvent 등은 각각 createSyntheticEvent를 호출하여 Interface에 맞는 생성자를 만드는 생성자 함수에요. 위와 같이 할 경우 불필요하게 많은 생성자를 만들어야하는 문제가 발생할 것 같아요 (특히나 react 원본 코드에서는 event가 훨~씬 많더라고요...)
  2. 앞서 언급한 내용대로 이벤트가 원래는 훨씬 더 많다보니 switch문을 사용했을 때 어떤 이벤트들이 그룹이 되어 특정 합성 이벤트 생성자를 가르키고 있는지 한 눈에 확인하기 쉬웠어요! (예를 들어 click과 contextmenu, mouseup 등은 다 같이 MouseEvent로 처리되는구나..하고 확인하는 느낌)

그래서 위와 같은 이유로 switch문으로 처리하게 되었습니다

Comment on lines +79 to +86
const event = new SyntheticEventConstructor(
name,
eventType,
nativeEvent,
nativeEventTarget,
);

return event;

Choose a reason for hiding this comment

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

그럼 요렇게도 바꿀 수 있을 것 같습니다 :D

Suggested change
const event = new SyntheticEventConstructor(
name,
eventType,
nativeEvent,
nativeEventTarget,
);
return event;
export function extractEvent(domEventName, nativeEvent, nativeEventTarget) {
if (!supportedEventNames.has(domEventName)) {
console.error("Unsupported Event :", domEventName);
return;
}
const SyntheticEventConstructor = eventTypeMapping[domEventName] || SyntheticEvent;
const event = new SyntheticEventConstructor(domEventName, domEventName, nativeEvent, nativeEventTarget);
return event;
}

Copy link
Author

Choose a reason for hiding this comment

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

아하 ㅋㅋㅋ 역시 한국말은 끝까지... 제가 염려할 부분을 미리 예측해서 코드를 수정해놓으신게 대단하시네요 bb 다만 이렇게 될 경우 새로운 생성자가 필요할 때 eventTypeMapping에서 생성자를 교체해주는 로직이 필요할 것 같아요!! 예를 들어 nativeEvent에서 각각 click이 3번 발생한 경우 첫 번째 click은 맵핑된 생성자함수를 사용하고 이후부턴 eventTypeMapping에서 생성자 함수를 교체해줘야하는 이슈가 있을 것 같습니다 ..! (잘은 모르지만 얕은 복사..와 같은 개념으로 매번 새로운 생성자 함수를 자동으로 호출되게하는.. 뭔가 그런 아이디어도 떠오르는데 구체적인 적용 방식은 고민해봐야겠습니다)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants