[#1] 프로젝트 셋업 / TDD를 위한 deeplyCopy 테스트 코드 작성 / Dual Package 지원 #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…odule system is set to module eslint package is set as dpendencies
f-lab-cadiz
approved these changes
Jun 20, 2024
There was a problem hiding this comment.
README.md엔 모듈 설치 방법이랑 간단한 API 소개 정도만 있으면 좋을 것 같네요!
- GitHub Actions로 테스트 항상 실행되고 모두 통과해야 main으로 머지될 수 있도록 설정해보면 어떨까요?
- CJS, ESM 지원 모두 할 수 있도록 세팅해봐도 좋을 것 같아요! (관련 아티클)
|
Prettier도 설정해두면 좋겠네요! |
…ct with prettier and add .gitignore path
kleekich21
changed the title
|
듀얼 패키지 지원을 위해 컴파일 단계에서 .cjs 파일을 생성하려고 했지만 Typescript가 아직 그 기능을 지원하고 있지 않음. 따라서, 확장자명 변경 로직을 추가 함. 관련 이슈 |
f-lab-cadiz
reviewed
Jul 2, 2024
renameFiles.js
Outdated
Comment on lines
1
to
24
| import fs from 'fs'; | ||
| import path from 'path'; | ||
|
|
||
| function renameFiles(directory, oldExt, newExt) { | ||
| fs.readdir(directory, (err, files) => { | ||
| if (err) throw err; | ||
|
|
||
| files.forEach((file) => { | ||
| if (path.extname(file) === oldExt) { | ||
| const oldPath = path.join(directory, file); | ||
| const newPath = path.join( | ||
| directory, | ||
| path.basename(file, oldExt) + newExt, | ||
| ); | ||
| fs.rename(oldPath, newPath, (err) => { | ||
| if (err) throw err; | ||
| console.log(`Renamed: ${file} -> ${path.basename(newPath)}`); | ||
| }); | ||
| } | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| renameFiles('./build/cjs', '.js', '.cjs'); |
f-lab-cadiz
reviewed
Jul 2, 2024
package.json
Outdated
| "build": "npm run build:cjs && npm run build:esm && npm run build:rename && npm run rename", | ||
| "build:cjs": "tsc --p ./cjs/tsconfig.json", | ||
| "build:esm": "tsc --p ./esm/tsconfig.json", | ||
| "build:rename": "npx tsc renameFiles.ts", |
There was a problem hiding this comment.
tsx 혹은 ts-node 써서 하는 방식으로 수정 부탁드려요!
NOTE: npx ts-node renameFiles.ts not working issue: TypeStrong/ts-node#2100
f-lab-cadiz
approved these changes
Jul 5, 2024
|
|
@f-lab-cadiz 꼼꼼히 봐주셔서 감사합니다! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
상세
이번 PR에서 중점적으로 봐주었으면 하는 사항들
deeplyCopy 인자와 return value의 타입=> 가이드에 P로 generic type을 명시해주셨군요deeplyCopy 테스트 케이스
참고
TDD로 진행하므로 테스트 코드 먼저 작성하였습니다. deeplyCopy는 구현 전이라, 현재 모든 테스트는 fail하는 상태입니다.
테스트 진행은
npm run test로 진행하실 수 있습니다.질문
다음 PR