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

Add 'use client' directive to Client components #2456

Closed
sungik-choi opened this issue Oct 11, 2024 · 6 comments · Fixed by #2468
Closed

Add 'use client' directive to Client components #2456

sungik-choi opened this issue Oct 11, 2024 · 6 comments · Fixed by #2468
Assignees
Labels
enhancement Issues or PR related to making existing features better

Comments

@sungik-choi
Copy link
Contributor

sungik-choi commented Oct 11, 2024

Description

Add 'use client' directive to Client components

Reasons for suggestion

대표적으로 Next.js App Router 등 React Server Component를 사용하는 라이브러리 사용처에서 bezier-react 컴포넌트를 쉽게 사용할 수 있도록 합니다.

Proposed solution

Often in a React app, you’ll leverage third-party libraries to handle common UI patterns or logic.

These libraries may rely on component Hooks or client APIs. Third-party components that use any of the following React APIs must run on the client:

위 요소를 충족하는 컴포넌트에 'use client' 지시어를 추가합니다.

References

@sungik-choi sungik-choi added the enhancement Issues or PR related to making existing features better label Oct 11, 2024
@github-project-automation github-project-automation bot moved this to 📌 Backlog in Bezier React Oct 11, 2024
Copy link

channeltalk bot commented Oct 11, 2024

@nayounsang
Copy link
Contributor

안녕하세요. 대시보드에 추천이 나와 관심이 생겨 구경중인데 이 이슈는 해결이 되면 무척 편리할 것 같네요. 채널톡외 개발자에게도 컨트리뷰션이 열려있는 것 같아요. 특정 파트를 지정해주시면 저도 같이 도움을 주면 어떨까 싶습니다.

@sungik-choi
Copy link
Contributor Author

안녕하세요. 대시보드에 추천이 나와 관심이 생겨 구경중인데 이 이슈는 해결이 되면 무척 편리할 것 같네요. 채널톡외 개발자에게도 컨트리뷰션이 열려있는 것 같아요. 특정 파트를 지정해주시면 저도 같이 도움을 주면 어떨까 싶습니다.

@nayounsang 안녕하세요. 내부 논의 결과 해당 이슈 맡아서 진행해주셔도 좋을 거 같습니다. 기여 방법 문서를 참고하여 작업 후 풀 리퀘스트를 올려주세요. 관심 가셔주셔서 감사합니다!

@nayounsang
Copy link
Contributor

nayounsang commented Oct 29, 2024

@sungik-choi
안녕하세요.
지시문을 추가하다 rollup 번들러단에서 Module level directives cause errors when bundled, "use client" was ignored.
warning이 나옵니다.

import { defineConfig } from "vite";

export default defineConfig({
  build: {
    rollupOptions: {
      onwarn(warning, warn) {
        // Suppress "Module level directives cause errors when bundled" warnings
        if (warning.code === "MODULE_LEVEL_DIRECTIVE") {
          return;
        }
        warn(warning);
      },
    },
  },
  // your other Vite config stuff...
});

해당 주의를 안나오게 할 수는 있지만(출처)아마 무시될 것이라고 생각되긴 합니다.
React19를 사용한다면 어떨지 모르겠으나, 더 알아보니 use client는 롤업이나 웹팩이 아닌 next와 같은 프레임워크에서 제공하는 빌드도구가 현재는 자체적으로 처리하는 것 같긴 하더라고요.
롤업에선 정식기능으론 추가하지 않을 것 같고 해당 이슈에서 여러 방법들이 논의됐네요.
우선 지시문을 추가한뒤 번들러 설정을 다른 이슈 & PR로 처리하면 어떨까 싶은데 어떠신가요?

@nayounsang
Copy link
Contributor

nayounsang commented Oct 29, 2024

제 의견은

  1. use client를 추가하고 생테계가 이를 수용하길 기다린다.
  2. client component가 되여아햘 컴포넌트에 관한 구조를 분리한다. 그 뒤 사용자는
'use client'

import ClientComponentExports from "bezier-react"

export ClientComponentExports

// ....
import { AutoFocus } from ClientComponentExports


이렇게 약간 번거롭게(?)사용하는 방법이 있겠네요.
이런 방법이 MUI가 서버컴포넌트 지원되기 이전에 논의됐던 것 같긴한데, 시도까진 안해봐서 돌아가는지는 모르겠어요.

@sungik-choi
Copy link
Contributor Author

2번은 굉장히 번거로운 일이 될 거 같고, 마찬가지로 'use client' 지시어를 추가해야해서 동일한 문제가 발생하는 것으로 이해했어요.

1번안을 선택하고, 말씀주신대로 https://github.com/remix-run/remix/pull/8897/files 와 같은 접근 방식으로 빌드 시 발생하는 해당 에러를 무시하도록 설정해두면 좋을 거 같습니다!

sungik-choi pushed a commit that referenced this issue Nov 4, 2024
components that use react hook or memo...

<!--
  How to write a good PR title:
- Follow [the Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/).
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

## Self Checklist

- [x] I wrote a PR title in **English** and added an appropriate
**label** to the PR.
- [x] I wrote the commit message in **English** and to follow [**the
Conventional Commits
specification**](https://www.conventionalcommits.org/en/v1.0.0/).
- [x] I [added the
**changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)
about the changes that needed to be released. (or didn't have to)
- [x] I wrote or updated **documentation** related to the changes. (or
didn't have to)
- [x] I wrote or updated **tests** related to the changes. (or didn't
have to)
- [x] 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
#2456 (not close or fixes)
<!-- Please link to issue if one exists -->

<!-- Fixes #0000 -->

## Summary

<!-- Please brief explanation of the changes made -->
- Add `use client` directives

## Details

<!-- Please elaborate description of the changes -->
- 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.
```typescript
// 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)

<!-- If Yes, please describe the impact and migration path for users -->
No

## References

<!-- Please list any other resources or points the reviewer should be
aware of -->
remix-run/remix#8891
rollup/rollup#4699
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues or PR related to making existing features better
Projects
No open projects
Status: 📌 Backlog
Development

Successfully merging a pull request may close this issue.

3 participants