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

feat(build): set some files to the client only for React Server Components use case #2245

Merged
merged 9 commits into from
Nov 25, 2023

Conversation

himself65
Copy link
Collaborator

@himself65 himself65 commented Nov 9, 2023

Related Issues or Discussions

Discussion: #2241 (comment)

Summary

import 'jotai/react'
import 'jotai/utils'

Are not allowed in the server component since it import useEffect and useState. directly or indirectly

You can import jotai/vanilla

Check List

  • yarn run prettier for formatting code and docs

Copy link

vercel bot commented Nov 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
jotai ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 25, 2023 1:31am

@himself65 himself65 changed the title feat: set some files to the client only build: set some files to the client only Nov 9, 2023
Copy link

codesandbox-ci bot commented Nov 9, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit fe7c7f8:

Sandbox Source
React Configuration
React TypeScript Configuration
React Browserify Configuration
React Snowpack Configuration
Next.js Configuration
Next.js with custom Babel config Configuration
React with custom Babel config Configuration

@himself65
Copy link
Collaborator Author

I think there is another approach that could make import 'jotai' work on the server side is to set react-server field in package.json.

You can see https://github.com/vercel/swr/blob/41b061306e2ce8b64e2923b1594e8db525c9368a/package.json#L36 for example

@dai-shi
Copy link
Member

dai-shi commented Nov 10, 2023

Please add the directive only in jotai/react and jotai/react/utils. That's enough.

I don't think react-server condition helps it, though it would improve the DX in addition.

Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

Please add the directive only in jotai/react and jotai/react/utils.

@himself65
Copy link
Collaborator Author

Sorry for the late response. I will update the PR today

@himself65
Copy link
Collaborator Author

btw, yarn now has 4.x version. Have any plans to migrate to the latest or use pnpm? I think yarn 1 is too old.

@dai-shi
Copy link
Member

dai-shi commented Nov 18, 2023

I would personally like to move to pnpm for all zustand/jotai/valtio.

Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

The diff looks good. I'll test it locally later.

@dai-shi
Copy link
Member

dai-shi commented Nov 22, 2023

diff -r dist.orig/esm/react/utils.mjs dist/esm/react/utils.mjs
0a1
> 'use client';
diff -r dist.orig/esm/react.mjs dist/esm/react.mjs
0a1
> 'use client';
diff -r dist.orig/package.json dist/package.json
88c88
<     "react": {
---
>     "@types/react": {
91c91
<     "@types/react": {
---
>     "react": {
94c94,95
<   }
---
>   },
>   "packageManager": "[email protected]"
diff -r dist.orig/react/utils.js dist/react/utils.js
0a1
> 'use client';
diff -r dist.orig/react.js dist/react.js
0a1
> 'use client';
diff -r dist.orig/system/react/utils.development.js dist/system/react/utils.development.js
0a1
> 'use client';
diff -r dist.orig/system/react/utils.production.js dist/system/react/utils.production.js
0a1
> 'use client';
diff -r dist.orig/system/react.development.js dist/system/react.development.js
0a1
> 'use client';
diff -r dist.orig/system/react.production.js dist/system/react.production.js
0a1
> 'use client';
diff -r dist.orig/umd/react/utils.development.js dist/umd/react/utils.development.js
0a1
> 'use client';
diff -r dist.orig/umd/react.development.js dist/umd/react.development.js
0a1
> 'use client';

I'm not sure if we need to modify UMD/SystemJS, but it may not hurt.

@dai-shi dai-shi added this to the v2.6.0 milestone Nov 22, 2023
@dai-shi dai-shi changed the title build: set some files to the client only feat(build): set some files to the client only for React Server Components use case Nov 25, 2023
@dai-shi dai-shi merged commit de67e40 into pmndrs:main Nov 25, 2023
30 checks passed
@himself65 himself65 deleted the himself65/rsc branch November 25, 2023 10:00
@L-Qun
Copy link
Contributor

L-Qun commented May 26, 2024

I just want to understand the reason for adding 'use client' to all files associated with React. From my understanding, it actually only needs to be added to files containing a Provider, since the 'use client' directive acts as a client boundary for components, right? Although it won't break anything. I look forward to your reply, thanks. @dai-shi

@dai-shi
Copy link
Member

dai-shi commented May 26, 2024

it actually only needs to be added to files containing a Provider

Yes, usage-wise I think so too.

it won't break anything.

And, that's why. I'm not sure if there's any drawback, like error message.

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.

3 participants