Skip to content

feat: add isPhoneNumber(region) validator decorator #215

Merged
dami0806 merged 4 commits intodevelopfrom
CARGO-396
Apr 16, 2026
Merged

feat: add isPhoneNumber(region) validator decorator #215
dami0806 merged 4 commits intodevelopfrom
CARGO-396

Conversation

@dami0806
Copy link
Copy Markdown
Contributor

Phone number rules vary by country and change over time, making it difficult to accurately support worldwide numbers through regex alone. This made it necessary to introduce a library based on Google's phone number database.

We evaluated two options: google-libphonenumber and libphonenumber-js.

Both libraries use the same Google phone number database and produce identical validation results.
The difference lies in the implementation.
google-libphonenumber is a mechanical JS transpilation of Google's original Java code, resulting in a large bundle size. libphonenumber-js, on the other hand, is a clean JS-native reimplementation of the same logic, making it significantly lighter.

Since both offer the same accuracy without the unnecessary overhead, we chose libphonenumber-js.

@dami0806 dami0806 self-assigned this Apr 15, 2026
@auto-assign auto-assign Bot requested review from laggu, sezeme and yuchem2 April 15, 2026 11:17
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

이번 풀 리퀘스트는 libphonenumber-js 라이브러리를 활용하여 전화번호 유효성을 검사하는 @IsPhoneNumber 데코레이터를 도입합니다. 이 데코레이터는 특정 지역 코드 기반의 로컬 번호와 국제 표준 번호 형식을 모두 지원하며, 이에 맞춰 다국어 문서와 예제 코드, 단위 테스트가 추가되었습니다. 리뷰 피드백으로는 region 설정 시 에러 메시지에 해당 지역 정보를 포함하여 검증 실패 원인을 더 명확하게 알 수 있도록 개선하라는 제안이 있었습니다.

Comment thread packages/express-cargo/src/validator.ts Outdated
propertyKey,
'isPhoneNumber',
(value: unknown) => typeof value === 'string' && isValidPhoneNumber(value, region),
message || `${String(propertyKey)} must be a valid phone number`,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

기본 에러 메시지에 region 정보를 포함하면 사용자가 어떤 국가 형식으로 검증이 실패했는지 더 명확하게 알 수 있습니다. region 파라미터가 제공된 경우 이를 메시지에 포함하는 것을 권장합니다.

Suggested change
message || `${String(propertyKey)} must be a valid phone number`,
message || `${String(propertyKey)} must be a valid phone number${region ? ` for ${region}` : ''}`,
References
  1. 가독성: 코드는 항상 읽기 쉽고 이해하기 쉬워야 합니다. 에러 메시지의 명확성을 높여 가독성과 유지보수성을 개선합니다. (link)

@yuchem2
Copy link
Copy Markdown
Member

yuchem2 commented Apr 15, 2026

외부 라이브러리를 도입하는 건 어쩔 수 없어보이네요. 고생하셨어요!

@dami0806 dami0806 merged commit c810cf4 into develop Apr 16, 2026
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