-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
'use client' directive for Client Components #4699
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
Comments
The reason we forbid module level directives is because after bundling, they will end up in a merged chunk with lots of other modules without any module boundaries. So the directive would apply to all other modules as well, which is usually very problematic. If you need such directives in the output, I would recommend to add them via the |
You can ignore this warning/filter it via an |
That makes sense, thanks for the quick response. Okay, we will look at configuration for preconstruct (which is using rollup under the hood). |
Note if you use the banner option AND terser, terser will strip away your The other alternative is to use a banner plugin like |
Saw the warnings as well in our PR today: I'm questioning if we really need |
@jaslong, am I correct in saying that applying rollup-plugin-banner2 with 'use client' then the whole library would be tagged as client side? |
For a bundled build, directives like If that were the case, as library authors we could say "if you want to use the RSC features, you need to use this output format of the library", while still providing a bundled build for backwards compatibility. |
I did some digging into this to see if it was something I could contribute on. I found where this is happening: https://github.com/rollup/rollup/blob/master/src/ast/nodes/ExpressionStatement.ts Being unfamiliar with the code base and being limited on time I ran into two problems though:
Just thought I'd document my findings if someone else wants to take a stab at this (if such a contribution is even desirable?). |
Yes, imports are removed and completely rewritten in the finalizers. To preserve directives, one would need to pass the directive to the finalizer, which is a bigger refactoring of the Rollup code base. |
Thanks, that clarified it! I gave a shot at implementing this as a plugin instead. It's a somewhat rough v0.1 (but feature complete, I think), there are no tests etc but in my own testing it seems to work well for us in the React Query build at least. Since it's my first Rollup plugin I would appreciate more eyes on it and help trying it out, and hope it helps someone else in this thread! https://github.com/Ephem/rollup-plugin-preserve-directives Quick version: // rollup.config.js
import preserveDirectives from "rollup-plugin-preserve-directives";
export default {
output: {
// If this is false, plugin does nothing
preserveModules: true,
},
plugins: [preserveDirectives()],
}; |
@Ephem you're a legend, thank you! Plugin works perfectly for me. Helped that I was already using |
@mryechkin Glad to hear its working well! If that changes, please let me know. 😉 |
Just a heads up for others if you're running your project as ESM (either by having So instead, do: // rollup.config.mjs
import preserveDirectives from "rollup-plugin-preserve-directives";
export default {
output: {
preserveModules: true,
},
plugins: [preserveDirectives.default()],
}; I believe this is due to how TypeScript bundles the library code when the esModuleInterop option is set (as is the case in that plugin): |
Oh, that's something I should fix, mind opening an issue over in the repo to track it? 😄 |
Done! Ephem/rollup-plugin-preserve-directives#1 🙏 |
@Ephem when using |
@joaovpmamede Huh, that's not something I've run into. If this is related to https://github.com/Ephem/rollup-plugin-preserve-directives, would you mind opening an issue over there with more details? |
In addition to this, how to set this up if a React component library that composes of server + client components? If a server component contains a client component. The main server component should staty a server component. Possibly detect imports in a server component, if there are a client component ensure its not bundled together with server component. |
@arkmech did ya try webpack? maybe they would've added a fix for it. |
Is there any configuration needed in a bundler like the one nextjs uses to make it recognize and respect the "use client" directive in a component from an npm package? I've created a component library, used Ephem's plugin to preserve the directives, and published it to a private npm registry . However, when I install it and try to use the compoentns directly in my nextjs pages they are being considered server components. If I wrap them in another component that is marked with "use client" they work but I don't want to have to do that. |
@gmaghame Hey, please keep us updated if ya find any solution to the problem |
We built a plugin rollup-preserve-directives for preservig directives like |
I ran into this issue yesterday and spent quite a bit of time searching for a solution. I'll post mine here for anyone else that gets stuck.
This is what I have: Object.defineProperty(exports, "__esModule", { value: true });
function getRollupOptions(options) {
const defaultGlobals = {
react: 'React',
'react-dom': 'ReactDOM',
'styled-components': 'styled',
'@emotion/react': 'emotionReact',
'@emotion/styled': 'emotionStyled',
};
if (Array.isArray(options.output)) {
options.output.forEach((output) => {
output.preserveModules = true;
output.globals = { ...output.globals, ...defaultGlobals };
});
} else {
const globals = !options.output?.globals
? defaultGlobals
: { ...options.output.globals, ...defaultGlobals };
options.output = {
...options.output,
preserveModules: true,
globals,
};
}
try {
const url = require('@rollup/plugin-url');
const svg = require('@svgr/rollup');
const preserveDirectives = require('rollup-plugin-preserve-directives').preserveDirectives;
options.plugins = [
svg({
svgo: false,
titleProp: true,
ref: true,
}),
url({
limit: 10000, // 10kB
}),
...(Array.isArray(options.plugins) ? options.plugins : []),
preserveDirectives(),
];
} catch {
console.log('Ignored for React Native');
}
options.onwarn = (warning, warn) => {
if (warning.code !== 'MODULE_LEVEL_DIRECTIVE') {
warn(warning);
}
};
return options;
}
module.exports = getRollupOptions; I made a few minor changes, but kept it mostly as I found it in the plugin folder. This is bundling my library with the |
For those who don't understand what to do here, in your rollup config add this:
This adds |
@Ben-at-Catalyst What about components you don't wish to add to the client-side bundle, and want them to benefit from server-side rendering? That's a great solution if every component in your library requires client-side JS to operate, but otherwise will unnecessarily bloat the JS bundle. |
@will-stone I believe in that case you can add "use server" to the top of induvidual components instead of at the module level. My problem is I have an |
You could use the rollup-plugin-preserve-directives plugin to selectively add I wrote about this on my blog here for reference: https://www.misha.wtf/blog/rollup-server-components EDIT: Didn't realize it was actually discussed earlier in this thread, see this comment above. |
I saw this plugin, but in my case I don't really care about the benefits of SSR and just need client components. I just thought with my original comment I'd try and explain the simplest action someone can take to resolve the error messages I was getting before finding this thread. |
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
For anyone looking for a plugin to selectively add 'use client' based on glob pattern matching the module name, here's a simple one I created. Hope it helps! |
Feature Use Case
Next.js recently released v13 with a beta version of Client Components based on React's RFC 227. They recommend that third-party npm packages use the "use client" directive to indicate when a component is a Client Component, to avoid compilation issues.
Currently, builds using Rollup fail with "Module level directives cause errors when bundled" due to not supporting directives on a module.
Feature Proposal
Allow a 'use client' module-level directive to be preserved in bundling of a library.
The text was updated successfully, but these errors were encountered: