Update tsup.config.ts to support "use client" of Next.js

[williamli]

After digging into this, there seems to be 2 3 ways to do this:

Option 1: Build first, use onSuccess hook to prepend use client to the start of ui/client package entry points.

import { defineConfig, Options } from "tsup";
import fs from "fs/promises";
import path from "path";

export default defineConfig((options: Options) => ({
  treeshake: true,
  splitting: true,
  entry: ["src/index.tsx", "src/client/index.tsx"],
  format: ['esm', 'cjs'],
  dts: true,
  minify: true,
  clean: true,
  external: ["react"],
  ...options,
  async onSuccess() {
    // add "use client" banner to /dist/client entry point
    await fs.writeFile(
      path.join(__dirname, "dist", "client", "index.js"),
      '"use client";\n' + await fs.readFile(path.join(__dirname, "dist", "client", "index.js"))
    );
    await fs.writeFile(
      path.join(__dirname, "dist", "client", "indexm.js"),
      '"use client";\n' + await fs.readFile(path.join(__dirname, "dist", "client", "index.mjs"))
    );

  },
}));

Pro:

• components are built once
• treeshake can set to true

Con:

• when the client components are compiled, before onSuccess is completed, there is a brief moment where there is no ‘use client’ for ui/client, which might cause Next.js app to flash an error to complain about the missing ‘use client’… the error will go away once onSuccess is completed.
• if there are build errors, onSuccess is not executed, client components will not get the use client directive, all client components will be broken

Option 2: Use a separate config for tsup for a new /dist/use-client file structure in which use client is added as a js banner.

import type { Options } from "tsup";
import { defineConfig } from "tsup";

  const cfg = {
    splitting: false,
    sourcemap: true,
    clean: true,
    treeshake: false,
    dts: true,
    format: ['esm', 'cjs'],
    external: ['react'],
  };
  
  export default defineConfig([
    {
      ...cfg,
      entry: ["src/client/index.tsx"],
      esbuildOptions: (options) => {
        // Append "use client" to the top of the react entry point
        options.banner = {
          js: '"use client";',
        };
      },
      outDir: 'dist/use-client',
    },
    {
      ...cfg,
      entry: ["src/index.tsx", "src/client/index.tsx"],
      outDir: 'dist',
    }
  ]);

Pro:

• Next.js will not complain

Con:

• treeshake has to be false
• Components in /client are built twice. Note: I tried to skip /client from being processed in the 2nd tsup task, but doing that will isolate styles defined in /client from tailwind’s stylesheet building task. classNames defined in /client will not get included in style.css until files outside of /client changes and trigger another tsup build.

Option 3: Based on Option 1, we prepend use client in a staging area (/dist/server/client) before moving the folder to /dist/client. This addresses all the cons of option 1. An extra package (fs-extra) is required to handle the moving.

// tsup.config.js

import { defineConfig, Options } from "tsup";
import fs from "fs-extra";
import path from "path";

export default defineConfig((options: Options) => ({
  treeshake: true,
  splitting: true,
  entry: ["src/index.tsx", "src/client/index.tsx"],
  outDir: 'dist/server',
  format: ['esm', 'cjs'],
  dts: true,
  minify: true,
  clean: true,
  external: ["react"],
  ...options,
  async onSuccess() {
    // add "use client" banner to /dist/client entry point
    await fs.writeFile(
      path.join(__dirname, "dist", "server", "client", "index.js"),
      '"use client";\n' + await fs.readFile(path.join(__dirname, "dist", "server", "client", "index.js"))
    );
    await fs.writeFile(
      path.join(__dirname, "dist", "server", "client", "indexm.js"),
      '"use client";\n' + await fs.readFile(path.join(__dirname, "dist", "server", "client", "index.mjs"))
    );
      
    // move /dist/server/client to /dist/client
    try {
      fs.moveSync(path.join(__dirname, "dist", "server", "client"), path.join(__dirname, "dist", "client"), { overwrite: true })
      
    } catch (err) {
      console.error(err)
    }
  },
}));

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
examples-native-web	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	Nov 6, 2023 7:11am
examples-vite-web	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	Nov 6, 2023 7:11am

9 Ignored Deployments

Name	Status	Preview	Comments	Updated (UTC)
examples-basic-web	⬜️ Ignored (Inspect)	Visit Preview		Nov 6, 2023 7:11am
examples-cra-web	⬜️ Ignored (Inspect)	Visit Preview		Nov 6, 2023 7:11am
examples-designsystem-docs	⬜️ Ignored (Inspect)	Visit Preview		Nov 6, 2023 7:11am
examples-gatsby-web	⬜️ Ignored (Inspect)	Visit Preview		Nov 6, 2023 7:11am
examples-kitchensink-blog	⬜️ Ignored (Inspect)	Visit Preview		Nov 6, 2023 7:11am
examples-nonmonorepo	⬜️ Ignored (Inspect)	Visit Preview		Nov 6, 2023 7:11am
examples-svelte-web	⬜️ Ignored (Inspect)	Visit Preview		Nov 6, 2023 7:11am
examples-tailwind-web	⬜️ Ignored (Inspect)	Visit Preview		Nov 6, 2023 7:11am
turbo-site	⬜️ Ignored (Inspect)	Visit Preview		Nov 6, 2023 7:11am

[github-actions]

🟢 Turbopack Benchmark CI successful 🟢

Thanks

[github-actions]

🟢 CI successful 🟢

Thanks

[magnusriga]

I had written a couple of other comments, but ended up deleting them as I had missed the fact that once a component is marked with use client all its children automatically also become client components (without having to explicitly mark them with the use client directive).

However, the options above still fail in a common scenario:

In the packages/ui/src folder, import a client component (for example Counter) into a server component (for example Card). That is common practice. Unfortunately, that will not work here because the client component Counter is split out into its own chunk by the bundler, without the use client directive.

Screenshots from the branch in this PR follow below, modified with the above import of a client component into a server component.

First the Counter client component. Notice that it has the use client directive, so all its exported children (like ./chunk-MPUGKNXI.mjs) are also automatically client components without the need to explicitly mark them with the use client directive:

Then the Counter chunk. Notice it is a client component, but is missing the use client directive: That is OK, as long as the Counter component is always imported from ./client/counter (which has the use client directive, as seen above).

Below is the server component Card. As seen, it imports the client component Counter from the new chunk instead of from ./client/counter. Herein lies the problem. Had it imported from ./client/counter it would have worked fine.

Turning splitting to false did not solve the issue.

Have you seen how the React team did it, by the way? They seem to build the folders separately: https://github.com/vercel/analytics/blob/main/packages/web/tsup.config.js

[williamli]

@magnusriga nice catch on Server Components importing Client Components. I will update the scripts to walk through the client entry points and insert use client if it's missing.

Have you seen how the React team did it, by the way? They seem to build the folders separately: https://github.com/vercel/analytics/blob/main/packages/web/tsup.config.js

Yes, I have tried that but it wouldn't work because Tailwind CSS requires reading both server and client components to create the stylesheet, so you will need to have a build with both server and client to create the style sheet. So you will end up with 3 builds: server only, client only, and server + client (for CSS)... which is something I am trying to avoid.

[williamli]

@magnusriga added support for using Client Components inside Server Components within the same (ui) package.

[magnusriga]

@williamli Awesome, that actually works. Two questions, if I may:

1. Why do we need transpilePackages: true in next.config.mjs, when tsup has already converted the ts to js?
2. Did you encounter an issue where the CSS from tailwind ended up being included twice in your final site, once because it was imported in layout.ts via globals.js, and once due to the import layout.ts via ui/styles.css? PS: The latter seems odd, because in ui/dist the file is called index.css and not styles.css.

[arlyon]

I am approving this with a 'fixship'; I think a comment explaining the problem and motivation as well as why it doesn't work out of the box (either in the PR, or in the tsup.config.ts itself) would go a long way to explain what is going on here.

[williamli]

@arlyon yeah, an update in README with details is in the work. I was waiting for @anthonyshew to come back to help on that but I can try to update it next week.

[magnusriga]

@williamli Have you seen this, by Jared Palmer?: https://turbo.build/blog/you-might-not-need-typescript-project-references#nextjs

Seems we might not need to build the internal packages at all. I.e. they can simply be imported from the src directory and next.js (or whichever framework one uses) can handle all the transpiling and bundling.

The turbo docs suggest the same approach: https://turbo.build/repo/docs/handbook/sharing-code/internal-packages

[anthonyshew]

The larger answer here is that we don't want to bundle internal packages.

Our examples today are indeed using tsup but that needs to change. I'm back from paternity leave now and my first major project is to update all of our examples. I still have some design/architecture work to think about with these but will update here once we've got some decisions made.

Appreciate the work!

[magnusriga]

The larger answer here is that we don't want to bundle internal packages. Rather, they should be compiled with tsc. The difficulties of this PR show why; it's an additional layer of complexity that we don't actually need.

Our examples today are indeed using tsup but that needs to change. I'm back from paternity leave now and my first major project is to update all of our examples. I still have some design/architecture work to think about with these but will update here once we've got some decisions made.

Appreciate the work!

@anthonyshew thank you for the input. Quick question: Can we just import the files from src and forget about the entire compilation and bundling step. I.e. just let Next.js handle all that, like the turbo docs explain in the internal packages section?

If we do it that way, why do we need to compile anything with tsc? Can't we just let next.js (for instance) handle it all? It seems that's what the basic Next.js example you have in the docs show.

Thanks a lot for the guidance.

[anthonyshew]

@magnusriga That is also an option - but it comes with a tradeoff. You won't be able to do as much caching since there aren't any outputs to cache on that internal package.

Depending on the size of your repository, this could have a significant impact on your build times.

EDIT: I was wrong about this. Can I blame Return from Parental Leave Brain? 😄

[magnusriga]

@magnusriga That is also an option - but it comes with a tradeoff. You won't be able to do as much caching since there aren't any outputs to cache on that internal package.

Depending on the size of your repository, this could have a significant impact on your build times.

I finally figured out a setup that works in every way, including with Intellisense on the RHS paths of imports. The setup, and an example repo based on the basic turborepo starter, can be found here: microsoft/TypeScript#56412 (comment)

[anthonyshew]

Y'all, I'm incredibly sorry that I forgot we had this PR out. I'm coming back from parental leave and trying to get my head on straight still.

While I was looking down the list of work I had out, I didn't realize that this PR was for the with-tailwind example. I read the title and skimmed the description, not realizing the work @williamli had done.

I worked on the with-tailwind example in #6575, solving the same problems as this PR:

• Supporting a mix of Server and Client Components
• Auto-imports across packages
• Types staying updated in IDE (cc @magnusriga)
• Tree-shaking

I'm going to favor #6575 for the sake of simplicity so if you'd like to leave critiques there, happy to have them!

[magnusriga]

Y'all, I'm incredibly sorry that I forgot we had this PR out. I'm coming back from parental leave and trying to get my head on straight still.

While I was looking down the list of work I had out, I didn't realize that this PR was for the with-tailwind example. I read the title and skimmed the description, not realizing the work @williamli had done.

I worked on the with-tailwind example in #6575, solving the same problems as this PR:

• Supporting a mix of Server and Client Components
• Auto-imports across packages
• Types staying updated in IDE (cc @magnusriga)
• Tree-shaking

I'm going to favor #6575 for the sake of simplicity so if you'd like to leave critiques there, happy to have them!

@anthonyshew thank you for that update. I see that you decided to not transpile the typescript in package/ui and instead import components from there into the next.js app directly from package/ui/src. You did, however, decide to compile the css with tailwind manually in package/ui (and not via postcss since we no longer build this package using a bundler). Could you please add some color on why that is necessary? Is it not enough to let the importing next.js app handle the tailwind via postcss, as it always does?

As a separate note, it would be nice not having to export each component manually. Meaning we could instead do this in the exports field: ./*: ./src/*. Path resolution should still work (I have not tried it without the file extension on the RHS, but I think it should still work).

[anthonyshew]

You can certainly let tailwindcss + PostCSS compile the output CSS in the consuming application by assigning the content field to capture the ./packages/ui glob but this isn't as "hermetic" and fundamentally sound as the technique in the example. In practice, it's usually fine but I'd like to ensure we're keeping to the fundamentals that each package should be as "self-contained" as possible. If app-a is directly referencing paths in @repo/ui that's not necessarily the most fundamental approach.

Additionally, with the approach we're using, we don't have to bring in a bundler at all (outside of the Next.js Compiler), reducing our repo's general overhead.

Exporting /*: ./src/* would be nice but it doesn't support auto-importing across package boundaries, unfortunately. There are Issues in the Typescript repo that reference this performance tradeoff/limitation.

[magnusriga]

You can certainly let tailwindcss + PostCSS compile the output CSS in the consuming application by assigning the content field to capture the ./packages/ui glob but this isn't as "hermetic" and fundamentally sound as the technique in the example. In practice, it's usually fine but I'd like to ensure we're keeping to the fundamentals that each package should be as "self-contained" as possible. If app-a is directly referencing paths in @repo/ui that's not necessarily the most fundamental approach.

Additionally, with the approach we're using, we don't have to bring in a bundler at all (outside of the Next.js Compiler), reducing our repo's general overhead.

Exporting /*: ./src/* would be nice but it doesn't support auto-importing across package boundaries, unfortunately. There are Issues in the Typescript repo that reference this performance tradeoff/limitation.

Got it. One more question @anthonyshew , when you have time:

I looked through your example, and see that you imported into layout.ts both the produced css in package/ui/dist and the tailwind css in the consuming next.js app (through ./global.css). Will that not potentially duplicate css classes from tailwind in the final output css file?

See for instance here: https://stackoverflow.com/questions/74292749/tailwindcss-duplicate-css-classes-when-using-library-in-app (the OP ended up doing the cross-referencing you advised against).

Side note: Can we fix the auto-import issue by using tsc to transpile the typescript in package/ui to JS, before importing it, like you originally suggested?

Thanks again!

[magnusriga]

@anthonyshew In addition to my comment above, I think the README might be wrong. It says you are compiling the TS to dist.

[clearly-outsane]

I tried this and this fails for me when I have client components imported in a server component. I think it's because the imports in the server component is from a chunk and the chunk does not have a "use client"; on top ( thought the client component file itself does ).

[magnusriga]

@clearly-outsane check out the updated example (this was the PR that updated it)