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

[#3] deeplyCopy 구현 #4

Merged
merged 8 commits into from
Jul 6, 2024
Merged

[#3] deeplyCopy 구현 #4

merged 8 commits into from
Jul 6, 2024

Conversation

kleekich21
Copy link
Collaborator

@kleekich21 kleekich21 commented Jun 20, 2024

상세

  • deeply 함수 구현
  • 테스트 코드 refactoring

이번 PR에서 중점적으로 봐주었으면 하는 사항들

  • deeplyCopy 함수 구현 내용
    • object 타입 체크시 null이 아닐 경우 추가 체크
    • 순환 참조는 WeakSet을 사용하여 처리
    • Object와 Array 타입의 차이에 따른 return value 분기 처리
  • 보완하거나 개선할 사항들이 있다면 알려주세요.

참고

deeplyCopy에서 null check을 하는 이유는 자바스크립트 고유 버그로 인해 typeof null이 'object'이기 때문이다.
관련 내용: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/typeof#typeof_null

테스트 진행은 npm run test 로 진행하실 수 있습니다.
현재 모든 테스트는 pass하는 상태입니다.

Copy link

@f-lab-cadiz f-lab-cadiz left a comment

Choose a reason for hiding this comment

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

➜  deeply-copy git:(feature/2) npm i
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/eslint
npm ERR!   dev eslint@"^9.5.0" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer eslint@"^8.56.0" from [email protected]
npm ERR! node_modules/typescript-eslint
npm ERR!   dev typescript-eslint@"^7.13.1" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!
npm ERR!
npm ERR! For a full report see:
npm ERR! /Users/hyeonsu/.npm/_logs/2024-06-20T05_03_21_285Z-eresolve-report.txt

npm install 이 잘 안됩니다!

@kleekich21
Copy link
Collaborator Author

혹시 pnpm 으로 모듈 install 해보실수 있으신가요?

Copy link

@f-lab-cadiz f-lab-cadiz left a comment

Choose a reason for hiding this comment

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

image image

ESLint가 깨져있는 것 같아요 👀

@kleekich21
Copy link
Collaborator Author

kleekich21 commented Jun 25, 2024

#2 에서 위 ESLint 문제 해결했습니다. 기록을 위해 남깁니다.

Copy link

@f-lab-cadiz f-lab-cadiz left a comment

Choose a reason for hiding this comment

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

👍

Copy link

sonarqubecloud bot commented Jul 5, 2024

Copy link

@f-lab-cadiz f-lab-cadiz left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~

@kleekich21 kleekich21 merged commit f5f15b7 into main Jul 6, 2024
3 checks passed
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.

2 participants