Add use client directive at top of client-component files #2468

nayounsang · 2024-10-29T09:53:25Z

components that use react hook or memo...

Self Checklist

I wrote a PR title in English and added an appropriate label to the PR.
I wrote the commit message in English and to follow the Conventional Commits specification.
I added the changeset about the changes that needed to be released. (or didn't have to)
I wrote or updated documentation related to the changes. (or didn't have to)
I wrote or updated tests related to the changes. (or didn't have to)
I tested the changes in various browsers. (or didn't have to)
- Windows: Chrome, Edge, (Optional) Firefox
- macOS: Chrome, Edge, Safari, (Optional) Firefox

Related Issue

close #2456

Summary

Add use client directives

Details

I add use client directives to all component filds
I didn't see any code that used the window API.
~~I am not sure about files that Radix exports without additional logic.~~
- As suggested in the comments, I tested it in the code sandbox and found no problems.
- Currently, directives have been added including this one. When I checked, I saw that hooks, etc. were used in the internal implementation.

// page.tsx
import * as Dialog from "@radix-ui/react-dialog";
import * as Tooltip from "@radix-ui/react-tooltip";
import * as Separator from "@radix-ui/react-separator";
import * as Switch from "@radix-ui/react-switch";
import * as VisuallyHidden from "@radix-ui/react-visually-hidden";

export default function Home() {
  return (
    <main className="flex min-h-screen flex-col items-center justify-between p-24">
      {/* code with radix-ui ...*/}
    </main>
  );
}

Add the changeset
To ignore Module level directives cause errors when bundled warnings, I returned onWarn early when the warning appeared.It seemed appropriate to write it inside the generateConfig.

Breaking change? (Yes/No)

No

References

remix-run/remix#8891
rollup/rollup#4699

changeset-bot · 2024-10-29T09:53:30Z

🦋 Changeset detected

Latest commit: c5f601f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@channel.io/bezier-react	Minor
bezier-figma-plugin	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

channeltalk · 2024-10-29T09:53:31Z

https://desk.channel.io/#/channels/1/team_chats/groups/1237/6720b09ab4c80d2bd6dc

github-actions · 2024-10-30T04:17:08Z

Chromatic Report

🚀 Congratulations! Your build was successful!

codecov · 2024-10-30T04:17:12Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.28%. Comparing base (3954134) to head (c5f601f).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2468   +/-   ##
=======================================
  Coverage   83.28%   83.28%           
=======================================
  Files         141      141           
  Lines        2943     2943           
  Branches      912      912           
=======================================
  Hits         2451     2451           
- Misses        462      487   +25     
+ Partials       30        5   -25

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sungik-choi

Self Checklist를 체크해주시고, 담당자와 라벨을 추가해주시면 감사하겠습니다. (잘 안되시면 알려주세요)
radix-ui등 서드파티 라이브러리를 사용하는 컴포넌트에 use client 지시어 추가 여부가 확실치 않다면, code sandbox 등에서 Next.js App Router 셋팅 후 import하여 테스트해보면 좋을 거 같습니다!

nayounsang · 2024-10-30T12:29:10Z

Self Checklist를 체크해주시고, 담당자와 라벨을 추가해주시면 감사하겠습니다. (잘 안되시면 알려주세요)

radix-ui등 서드파티 라이브러리를 사용하는 컴포넌트에 use client 지시어 추가 여부가 확실치 않다면, code sandbox 등에서 Next.js App Router 셋팅 후 import하여 테스트해보면 좋을 거 같습니다!

넵. 피드백 감사합니다.
2번의 경우 진행해보고 말씀드리겠습니다.
1번은 제가 assigness와 label 설정이 안됩니다!

nayounsang · 2024-10-30T13:53:10Z

@sungik-choi 확인 부탁드립니다~ PR 메시지에 체크박스에 대해 궁금한 점도 적었습니다.(완료시 삭제 예정)

yangwooseong · 2024-10-31T01:34:36Z

changeset 은 붙이는 게 좋을 것 같습니다! 마이너 버전 업으로 하는게 어떨까요? 추가하고 체크리스트 전부 확인하시고 괜찮으면 체크까지 해주세요.

혹시 어떤 기준으로 use client를 사용하셨나요? 문서를 찾아보니 ref forwarding 이나 컨텍스트를 사용하는 곳에서도 use client 를 추가하는게 맞는 것 같아서요. nextjs로 테스트 해보니 AlphaDialogPrimitive 를 사용하거나 Text 같이 이벤트 핸들러가 없는 컴포넌트라도 ref forwarding 을 하면 에러가 뜹니다. 그래서 모든 컴포넌트에 붙여야 하지 않나 생각이 듭니다.

https://react.dev/reference/rsc/use-client#using-third-party-libraries

sungik-choi · 2024-10-31T02:38:07Z

@sungik-choi 확인 부탁드립니다~ PR 메시지에 체크박스에 대해 궁금한 점도 적었습니다.(완료시 삭제 예정)

I think didn't have to 라면 체크해주시면 됩니다!
Is there a way to use the package with the changes reflected directly in next? : 따로 스냅샷 배포 등을 진행하진 않고 있어서 이 PR의 변경사항이 포함되고나서 다음 배포 버전에서 사용 가능합니다. 미리 테스트해보고 싶으시다면 로컬에서 패키지 경로를 바꿔 테스트할 수 있습니다.

nayounsang · 2024-10-31T05:56:20Z

changeset 은 붙이는 게 좋을 것 같습니다! 마이너 버전 업으로 하는게 어떨까요? 추가하고 체크리스트 전부 확인하시고 괜찮으면 체크까지 해주세요.

혹시 어떤 기준으로 use client를 사용하셨나요? 문서를 찾아보니 ref forwarding 이나 컨텍스트를 사용하는 곳에서도 use client 를 추가하는게 맞는 것 같아서요. nextjs로 테스트 해보니 AlphaDialogPrimitive 를 사용하거나 Text 같이 이벤트 핸들러가 없는 컴포넌트라도 ref forwarding 을 하면 에러가 뜹니다. 그래서 모든 컴포넌트에 붙여야 하지 않나 생각이 듭니다.

https://react.dev/reference/rsc/use-client#using-third-party-libraries

아~ forwardRef 는 제가 잘못 알고 있었네요.
그럼 모든 컴포넌트에 붙히도록 하겠습니다. 컴포넌트가 많다보니 컨텍스트나 훅을 사용해도 제가 빼먹은게 생겼네요...!!

yangwooseong · 2024-11-01T01:05:10Z

packages/bezier-react/rollup.config.mjs

@@ -113,6 +113,13 @@ const generateConfig = ({ output = [], plugins = [] }) =>
      minifycss(),
      ...plugins,
    ],
+    onwarn(warning, warn) {
+      // Suppress "Module level directives cause errors when bundled" warnings
+      if (warning.code === 'MODULE_LEVEL_DIRECTIVE') {


이대로라면 모든 MODULE_LEVEL_DIRECTIVE 워닝을 끄게 되서 "use client"까지 확인하는게 좋을것 같습니다.

Suggested change

if (warning.code === 'MODULE_LEVEL_DIRECTIVE') {

if (warning.code === 'MODULE_LEVEL_DIRECTIVE' && warning.message.includes("use client")) {

제가 미처 생각하지 못한 것인데 좋네요! 제가 실수로 changeset을 patch로 했는데 그거와 함께 고쳐서 푸시할게요.

components that use react hook or memo...

"Module level directives cause errors when bundled" warnings

I wrote the commit message incorrectly. This reverts commit 53114f2

sungik-choi · 2024-11-04T02:36:49Z

LGTM
기여 감사드립니다!

@nayounsang

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @channel.io/bezier-react@2.4.0 ### Minor Changes - The use client directive has been added at the top of all components inside @channel.io/bezier-react. ([#2468](#2468)) by @nayounsang ### Patch Changes - Remove 1px gap between icon and text in `s` size `AlphaButton`, `AlphaFloatingButton`. ([#2484](#2484)) by @yangwooseong - Apply hovered color token for `AlphaButton`, `AlphaFloatingButton`, and `AlphaFloatingIconButton`. ([#2471](#2471)) by @yangwooseong - Add `ToggleEmojiButtonGroup` and `ToggleEmojiButtonSource` component. ([#2301](#2301)) by @yangwooseong - Updated dependencies - @channel.io/bezier-tokens@0.2.11 ## @channel.io/bezier-tokens@0.2.11 ### Patch Changes - Change `alpha-color-green-300-0` alpha token. ([#2471](#2471)) by @yangwooseong Modify hovered color generating formula so that they are more visible in dark theme. ## @channel.io/stylelint-bezier@0.2.5 ### Patch Changes - Updated dependencies - @channel.io/bezier-tokens@0.2.11 ## bezier-figma-plugin@0.6.13 ### Patch Changes - Updated dependencies - @channel.io/bezier-react@2.4.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

nayounsang requested review from sungik-choi and yangwooseong as code owners October 29, 2024 09:53

sungik-choi reviewed Oct 30, 2024

View reviewed changes

yangwooseong assigned nayounsang Oct 31, 2024

yangwooseong added the enhancement Issues or PR related to making existing features better label Oct 31, 2024

sungik-choi changed the title ~~Add use client driectvie at top of client-component files~~ Add use client directive at top of client-component files Oct 31, 2024

yangwooseong reviewed Nov 1, 2024

View reviewed changes

yangwooseong approved these changes Nov 4, 2024

View reviewed changes

sungik-choi approved these changes Nov 4, 2024

View reviewed changes

nayounsang added 8 commits November 4, 2024 11:24

refactor: add use client driectvie at top of client-component files

823e9f9

components that use react hook or memo...

chore: ignore MODULE_LEVEL_DIRECTIVE warning

18f41a0

"Module level directives cause errors when bundled" warnings

Revert "chore: ignore MODULE_LEVEL_DIRECTIVE warning"

d4b1d83

I wrote the commit message incorrectly. This reverts commit 53114f2

chore: ignore MODULE_LEVEL_DIRECTIVE warning

9cc550d

refactor: add use client directvie to all the rest

244f821

chore: add the changeset

3552937

chore: change version bump to minor

bcd997b

feat: ignore warnings specific to 'use client' directive

c5f601f

sungik-choi force-pushed the use-client-directive branch from cac1556 to c5f601f Compare November 4, 2024 02:24

sungik-choi merged commit 34b38af into channel-io:main Nov 4, 2024
9 checks passed

ch-builder mentioned this pull request Nov 4, 2024

ci(changesets): version packages #2472

Merged

yangwooseong linked an issue Nov 4, 2024 that may be closed by this pull request

Add 'use client' directive to Client components #2456

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add use client directive at top of client-component files #2468

Add use client directive at top of client-component files #2468

nayounsang commented Oct 29, 2024 •

edited

Loading

changeset-bot bot commented Oct 29, 2024 •

edited

Loading

channeltalk bot commented Oct 29, 2024

github-actions bot commented Oct 30, 2024 •

edited

Loading

codecov bot commented Oct 30, 2024 •

edited

Loading

sungik-choi left a comment

nayounsang commented Oct 30, 2024

nayounsang commented Oct 30, 2024 •

edited

Loading

yangwooseong commented Oct 31, 2024 •

edited

Loading

sungik-choi commented Oct 31, 2024

nayounsang commented Oct 31, 2024

yangwooseong Nov 1, 2024

nayounsang Nov 1, 2024

sungik-choi commented Nov 4, 2024

	if (warning.code === 'MODULE_LEVEL_DIRECTIVE') {
	if (warning.code === 'MODULE_LEVEL_DIRECTIVE' && warning.message.includes("use client")) {

Add use client directive at top of client-component files #2468

Add use client directive at top of client-component files #2468

Conversation

nayounsang commented Oct 29, 2024 • edited Loading

Self Checklist

Related Issue

Summary

Details

Breaking change? (Yes/No)

References

changeset-bot bot commented Oct 29, 2024 • edited Loading

🦋 Changeset detected

channeltalk bot commented Oct 29, 2024

github-actions bot commented Oct 30, 2024 • edited Loading

Chromatic Report

codecov bot commented Oct 30, 2024 • edited Loading

Codecov Report

sungik-choi left a comment

Choose a reason for hiding this comment

nayounsang commented Oct 30, 2024

nayounsang commented Oct 30, 2024 • edited Loading

yangwooseong commented Oct 31, 2024 • edited Loading

sungik-choi commented Oct 31, 2024

nayounsang commented Oct 31, 2024

yangwooseong Nov 1, 2024

Choose a reason for hiding this comment

nayounsang Nov 1, 2024

Choose a reason for hiding this comment

sungik-choi commented Nov 4, 2024

nayounsang commented Oct 29, 2024 •

edited

Loading

changeset-bot bot commented Oct 29, 2024 •

edited

Loading

github-actions bot commented Oct 30, 2024 •

edited

Loading

codecov bot commented Oct 30, 2024 •

edited

Loading

nayounsang commented Oct 30, 2024 •

edited

Loading

yangwooseong commented Oct 31, 2024 •

edited

Loading