Open
Conversation
|
Contributor
📝 WalkthroughWalkthrough전화번호 입력 필드 기능을 추가하는 PR로, 새로운 React 훅, 예제 컴포넌트, 테스트 스위트를 포함합니다. GitHub Actions 워크플로우를 재구성하고 TypeScript 설정을 정규화했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Alpha Preview (Docs)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
docs/registry/registry-lib.ts (1)
11-19:@seed-design/react의~1.1.0버전 범위가 지나치게 좁습니다.
~1.1.0은1.1.x패치 버전만 허용하여,1.2.0같은 마이너 업데이트도 차단합니다. 이 훅 자체는@seed-design/react를 직접 import하지 않고 타입 참조만 사용하므로, 소비자 환경에서 더 유연하게 사용할 수 있도록^1.1.0을 고려해보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/registry/registry-lib.ts` around lines 11 - 19, The package range for "@seed-design/react" in the registry entry with id "phone-number-field" (snippets -> path "phone-number-field.ts") is too restrictive ("~1.1.0"); change the dependency version to a caret range (e.g. "^1.1.0") so consumers can receive minor updates without breaking, since the snippet only uses type references rather than runtime imports.docs/registry/lib/phone-number-field.ts (2)
50-58: 콜백 안정성을 위해useCallback적용을 고려하세요.
setValueRaw,setValue,handleValueChange가 매 렌더마다 새로운 함수 참조로 생성되어, 이를 prop으로 받는 메모이즈된 컴포넌트에서 불필요한 리렌더링을 유발할 수 있습니다. 스니펫으로 배포되는 훅이므로 소비자 환경을 고려해useCallback을 적용하는 것을 권장합니다.♻️ 제안하는 리팩터
- const setValueRaw = (value: string) => { + const setValueRaw = useCallback((value: string) => { if (propValue === undefined) setInternalValue(value); onValueChange?.(value); - }; + }, [propValue, onValueChange]); - const setValue = (value: string) => { + const setValue = useCallback((value: string) => { setValueRaw(toNationalDigits(value, countryCode)); - }; + }, [setValueRaw, countryCode]);
handleValueChange도 동일하게useCallback으로 감싸고,[value, formattedValue, setValueRaw, countryCode]를 의존성으로 추가합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/registry/lib/phone-number-field.ts` around lines 50 - 58, Wrap setValueRaw, setValue, and handleValueChange in React.useCallback to stabilize their references; ensure setValueRaw depends on propValue, setInternalValue, and onValueChange; setValue depends on countryCode and toNationalDigits and calls setValueRaw; and handleValueChange includes value, formattedValue, setValueRaw, and countryCode in its dependency array so memoized consumers don't get new function refs each render.
84-98:countryCode가 변경되어도 내부value가 자동으로 초기화되지 않습니다.현재 국가가 KR → US로 바뀌어도 저장된 raw 값(예:
"01012345678")은 그대로 남아 있어,formatIncompletePhoneNumber("01012345678", "US")와 같이 다른 국가 포맷으로 렌더링됩니다. 예제 컴포넌트에서는 국가 변경 시setValue("")를 호출하여 이를 처리하지만, 훅 자체에서 이 동작을 문서화하거나,useEffect로countryCode변경을 감지해 내부 상태를 초기화하는 방식을 고려해주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/registry/lib/phone-number-field.ts` around lines 84 - 98, The hook currently keeps the raw phone value when countryCode changes, causing misformatted rendering; update the hook that returns { value, setValue, textFieldProps, inputProps } to reset the internal raw value when countryCode changes (use an effect watching countryCode) — call setValue("") (or otherwise clear the raw value) when countryCode updates so formattedValue and textFieldProps.input reflect the new country's formatting; ensure this logic is tied to the existing symbols countryCode, value, setValue, formattedValue and does not interfere with handleValueChange or placeholder merging..github/workflows/docs-test.yml (1)
3-7:pull_request트리거 누락 — PR 단계에서 테스트가 실행되지 않습니다.현재
push만 트리거로 등록되어 있어 PR이 열리거나 업데이트될 때는 테스트가 실행되지 않습니다. PR 검증용으로pull_request트리거를 추가하는 것을 권장합니다.✅ 제안하는 수정
on: push: paths: - "docs/**/*" - - ".github/workflows/llms-transform-test.yml" + - ".github/workflows/docs-test.yml" + pull_request: + paths: + - "docs/**/*" + - ".github/workflows/docs-test.yml"As per coding guidelines,
.github/workflows/**파일 변경 전에는 팀 확인이 권장됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-test.yml around lines 3 - 7, The workflow currently only triggers on push; add a pull_request trigger to the workflow's "on" configuration so tests run on PRs as well—mirror the existing push paths block (include "docs/**/*" and ".github/workflows/llms-transform-test.yml") under a new pull_request entry. Update the top-level "on" mapping to include both push and pull_request triggers and ensure any required team approval is requested before committing changes to .github/workflows/** files.docs/examples/react/text-field-input/phone-number-formatting.tsx (2)
21-55: 코딩 가이드라인:forwardRef와displayName누락
TextFieldInputPhoneNumberFormatting컴포넌트에forwardRef와displayName이 적용되어 있지 않습니다. 예제 컴포넌트이므로ref노출이 실제로 필요하지는 않지만, 프로젝트 가이드라인의 준수를 위해 추가를 검토해주세요.다만 현재 프로젝트가 React 19.2.3을 사용하고 있으며, React 19에서는
forwardRef대신ref를 일반 prop으로 전달하는 방식이 표준입니다. 가이드라인이 React 19 이전 기준으로 작성된 것이라면, 팀 차원에서 가이드라인 업데이트를 논의해보시기 바랍니다.As per coding guidelines,
**/*.{tsx,jsx}파일의 React 컴포넌트에는forwardRef와displayName을 사용해야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/react/text-field-input/phone-number-formatting.tsx` around lines 21 - 55, Wrap the TextFieldInputPhoneNumberFormatting function with React.forwardRef so it accepts a ref (forward the ref to the root element or the appropriate child), convert the existing default export to export the forwardRef-wrapped component, and set TextFieldInputPhoneNumberFormatting.displayName = "TextFieldInputPhoneNumberFormatting"; ensure the forwarded ref type matches the rendered DOM element (or component) and update the function signature to accept (props, ref) and use the ref where appropriate.
30-33:onValueChange콜백의 타입 안전성 —as캐스팅
RadioGroup의onValueChange는string을 제공하는데,v as typeof countryCode로 강제 캐스팅합니다. 런타임에서v는 항상Object.keys(countries)중 하나이므로 실질적인 문제는 없지만, 타입 안전성을 높이려면 내로잉을 명시적으로 처리하는 방법을 고려하세요.♻️ 제안하는 수정
- onValueChange={(v) => { - setCountryCode(v as typeof countryCode); - phoneNumber.setValue(""); - }} + onValueChange={(v) => { + if (v in countries) { + setCountryCode(v as typeof countryCode); + phoneNumber.setValue(""); + } + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/react/text-field-input/phone-number-formatting.tsx` around lines 30 - 33, The onValueChange callback currently force-casts v with "setCountryCode(v as typeof countryCode)", which bypasses type safety; instead narrow or validate v against the known keys of countries before calling setCountryCode: treat v as string and check "if (v in countries)" (or use "const key = v as keyof typeof countries" after a runtime check) then call setCountryCode(key) and reset phoneNumber with phoneNumber.setValue(""); update the RadioGroup handler signature to accept (v: string) if needed to avoid the blanket "as" cast and ensure countryCode's type is the union of keys of countries for full type-safety (references: onValueChange, setCountryCode, countryCode, countries, RadioGroup, phoneNumber.setValue).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-test.yml:
- Line 7: The workflow's paths filter still references the deleted file
"llms-transform-test.yml", so edits to this workflow won't trigger CI; update
the paths entry in .github/workflows/docs-test.yml to reference the current
workflow file (e.g., " .github/workflows/docs-test.yml") or any relevant docs
paths you want to watch, ensuring the paths: array includes the actual file(s)
you expect to trigger this workflow so changes to docs-test.yml will run the CI;
modify the paths key in the docs-test.yml workflow to remove the deleted
filename and add the correct path(s).
---
Nitpick comments:
In @.github/workflows/docs-test.yml:
- Around line 3-7: The workflow currently only triggers on push; add a
pull_request trigger to the workflow's "on" configuration so tests run on PRs as
well—mirror the existing push paths block (include "docs/**/*" and
".github/workflows/llms-transform-test.yml") under a new pull_request entry.
Update the top-level "on" mapping to include both push and pull_request triggers
and ensure any required team approval is requested before committing changes to
.github/workflows/** files.
In `@docs/examples/react/text-field-input/phone-number-formatting.tsx`:
- Around line 21-55: Wrap the TextFieldInputPhoneNumberFormatting function with
React.forwardRef so it accepts a ref (forward the ref to the root element or the
appropriate child), convert the existing default export to export the
forwardRef-wrapped component, and set
TextFieldInputPhoneNumberFormatting.displayName =
"TextFieldInputPhoneNumberFormatting"; ensure the forwarded ref type matches the
rendered DOM element (or component) and update the function signature to accept
(props, ref) and use the ref where appropriate.
- Around line 30-33: The onValueChange callback currently force-casts v with
"setCountryCode(v as typeof countryCode)", which bypasses type safety; instead
narrow or validate v against the known keys of countries before calling
setCountryCode: treat v as string and check "if (v in countries)" (or use "const
key = v as keyof typeof countries" after a runtime check) then call
setCountryCode(key) and reset phoneNumber with phoneNumber.setValue(""); update
the RadioGroup handler signature to accept (v: string) if needed to avoid the
blanket "as" cast and ensure countryCode's type is the union of keys of
countries for full type-safety (references: onValueChange, setCountryCode,
countryCode, countries, RadioGroup, phoneNumber.setValue).
In `@docs/registry/lib/phone-number-field.ts`:
- Around line 50-58: Wrap setValueRaw, setValue, and handleValueChange in
React.useCallback to stabilize their references; ensure setValueRaw depends on
propValue, setInternalValue, and onValueChange; setValue depends on countryCode
and toNationalDigits and calls setValueRaw; and handleValueChange includes
value, formattedValue, setValueRaw, and countryCode in its dependency array so
memoized consumers don't get new function refs each render.
- Around line 84-98: The hook currently keeps the raw phone value when
countryCode changes, causing misformatted rendering; update the hook that
returns { value, setValue, textFieldProps, inputProps } to reset the internal
raw value when countryCode changes (use an effect watching countryCode) — call
setValue("") (or otherwise clear the raw value) when countryCode updates so
formattedValue and textFieldProps.input reflect the new country's formatting;
ensure this logic is tied to the existing symbols countryCode, value, setValue,
formattedValue and does not interfere with handleValueChange or placeholder
merging.
In `@docs/registry/registry-lib.ts`:
- Around line 11-19: The package range for "@seed-design/react" in the registry
entry with id "phone-number-field" (snippets -> path "phone-number-field.ts") is
too restrictive ("~1.1.0"); change the dependency version to a caret range (e.g.
"^1.1.0") so consumers can receive minor updates without breaking, since the
snippet only uses type references rather than runtime imports.
| push: | ||
| paths: | ||
| - "docs/**/*" | ||
| - ".github/workflows/llms-transform-test.yml" |
Contributor
There was a problem hiding this comment.
삭제된 파일명을 참조하고 있어 워크플로우 자체 변경 시 트리거되지 않습니다.
llms-transform-test.yml은 이 PR에서 삭제된 파일입니다. paths 필터가 현재 워크플로우 파일인 docs-test.yml을 참조해야 합니다. 그렇지 않으면 이 워크플로우 파일 자체를 수정해도 CI가 실행되지 않습니다.
🐛 제안하는 수정
- - ".github/workflows/llms-transform-test.yml"
+ - ".github/workflows/docs-test.yml"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - ".github/workflows/llms-transform-test.yml" | |
| - ".github/workflows/docs-test.yml" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docs-test.yml at line 7, The workflow's paths filter still
references the deleted file "llms-transform-test.yml", so edits to this workflow
won't trigger CI; update the paths entry in .github/workflows/docs-test.yml to
reference the current workflow file (e.g., " .github/workflows/docs-test.yml")
or any relevant docs paths you want to watch, ensuring the paths: array includes
the actual file(s) you expect to trigger this workflow so changes to
docs-test.yml will run the CI; modify the paths key in the docs-test.yml
workflow to remove the deleted filename and add the correct path(s).
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
usePhoneNumberFieldhook snippet 추가 (lib/phone-number-field)libphonenumber-js기반 전화번호 포맷팅/파싱 (KR, JP, CA, US, GB)Test plan
bun docs:test통과 확인bun generate:all정상 동작 확인🤖 Generated with Claude Code
Summary by CodeRabbit
릴리스 노트
New Features
Documentation
Tests
Chores