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

[turborepo] Examples are too simple: missing a real-world example with tree-shakeable, RSC-ready library #6019

Closed
raphaelbadia opened this issue Sep 25, 2023 · 14 comments · Fixed by #6682
Assignees
Labels
area: examples Improvements or additions to examples owned-by: turborepo

Comments

@raphaelbadia
Copy link

raphaelbadia commented Sep 25, 2023

Which project is this feature idea for?

Turborepo

Describe the feature you'd like to request

Hello,

I've skimmed through the turborepo examples and I haven't yet been able to get together a working example that could:

  • have a UI library such as shadcn containing client components and server components without forcing all components to be client. Currently there are tsup workarounds to force-add "use client" to the whole bundle, but when half of your components are just dump components without interactivity, it's a waste.
  • be tree shakeable:
    • the examples with tsup won't tree-shake when you start using npm packages (ie you create a select component with react-select, a dropzone component with react-dropzone, a simple component, you'll end up with the three packages in every page even if you only import the Hello component).
    • the transpilePackages won't tree shake the components: transpilePackages doesn't tree-shake barrel export in turborepo next.js#55943
  • redirect to the appropriate file when doing a cmd+click: currently in the examples, "go to definition" will bring you to a .d.ts. That's not what devs expect when developing in a monorepo.
  • put files in a src/ folder and still work with nextjs transpiling

Describe the solution you'd like

I'd like a kitchen-sink like example but more in-depth: its Link and CounterButton are currently client components due to the banner in the tsup.config.ts, but they could totally be server components.

It would be great if it could support shadcn which is gaining much popularity now. It's also a great idea because shadcn/ui brings in a bit of complexity by installing third party packages. That would make it more "real", at least to me. Because with barrel exports, I've always struggled with bundle size!

Describe alternatives you've considered

I've considered doing it myself but I couldn't make it.

So far I've been able to "fix" the "go-to definition problem" starting from the with-tailwind example, but couldn't do anything about the tree shaking.

I've tried many things from different github issues to support the "use client" at the file-level instead of the package-level, but I wasn't able to make it work. Even tried to create a with-tailwind package using rollup instead of tsup. I failed

Thanks!

@raphaelbadia raphaelbadia added needs: triage New issues get this label. Remove it after triage story labels Sep 25, 2023
@raphaelbadia raphaelbadia changed the title Examples are too simple: missing a real-world example with tree-shakeable, RSC-ready library [turborepo] Examples are too simple: missing a real-world example with tree-shakeable, RSC-ready library Sep 25, 2023
@UlasCanAk
Copy link

Fully agreed, this has been very frustrating for me as well. It's unclear to me how I can get a proper setup with all the requirements you listed working with 2 Next apps and a shared ui library. Have you been able to find a solution or workaround yet? How did you fix the go-to definition problem? Also, have you been able to put components in subdirectories inside the src folder of the ui package? Doing that did not work for me, even when changing the tsup config.

@mehulkar mehulkar added the area: examples Improvements or additions to examples label Oct 10, 2023
@yasssuz
Copy link

yasssuz commented Oct 13, 2023

We need this

@raphaelbadia
Copy link
Author

We indeed need this.

Currently my apps are using transpilePackages and imports are like this to work around the tree-shaking issue:

import FormInput from '@acme/ui/src/forms/FormInput';
import Button from '@acme/ui/src/ui/Button';

Building our packages instead of transpiling would allow us to fasten the build time, as packages would be cached between build. Currently the build process with rebuild all monorepo packages used by my app.

I'd just love to do

import { Button, FormInput } from '@acme/ui';

As seen in the turborepo examples, but it's not real-world compatible...

@bel7aG
Copy link

bel7aG commented Oct 19, 2023

Same here, tried multiple ways but no result, even with nextjs modularizeImports and the experimental optimizePackageImports

@mehulkar
Copy link
Contributor

Thanks for the request! I'll take it to the team to see if we'd be able to create and maintain such an example!

@mehulkar mehulkar added owned-by: turborepo needs: team input Filter for core team meetings and removed needs: triage New issues get this label. Remove it after triage labels Oct 20, 2023
@mehulkar mehulkar removed the needs: team input Filter for core team meetings label Nov 6, 2023
@Neosoulink
Copy link
Contributor

@mehulkar, I'm interested in this issue,
Is someone or a team working on it?

I'll be glad to contribute to this one.

@mehulkar
Copy link
Contributor

@Neosoulink @anthonyshew is owning this, you may be able to collaborate

@Neosoulink
Copy link
Contributor

Hey @anthonyshew, is this issue in the planning?

@yasssuz
Copy link

yasssuz commented Nov 30, 2023

@anthonyshew @mehulkar How can I collaborate?

@anthonyshew
Copy link
Contributor

@Neosoulink @yasssuz Head on over to #6682!

@raphaelbadia
Copy link
Author

@anthonyshew Hello, thanks for taking the time to address this issue, however the basic example you updated sounds more like a workaround 😔

The basic example uses transpilePackages, which technically works but doesn't build at package level so it doesn't leverage full turborepo caching (see #6019 (comment) )
If I have 5 apps (I do at my job), we end up transpiling and recompiling all of our 10 packages for each project.

I also think that the treeshaking suggested is not realistic :
CleanShot 2023-12-04 at 10 52 45@2x

Doing that is going to be hard to maintain IMO !

@anthonyshew
Copy link
Contributor

Compiling a package before the Next.js Compiler bundles it won't result in faster build times. The Next.js Compiler still must parse the third-party code to create bundles. Achieving better granularity is one of the reasons Turbopack is being built.

The tree-shaking shown is realistic. We use this technique in our monorepo at Vercel. If you'd prefer, you can use

"exports": {
  "./*: "./src/*"
}

but you'll lose auto-importing across package boundaries.

anthonyshew added a commit that referenced this issue Dec 6, 2023
Previously, the `basic` example (AKA create-turbo) was entirely Server
Components. In this PR, we're continuing to leverage the Internal
Package pattern such that we have a mix of React Server Components
**and** Client components.

The Client Component is the new `Button` component in the `@repo/ui`
package.

Notably, this PR closes #6019, fulfilling these requirements:
- A mix of Server Components and Client Components
- Editor go-to-definition across packages (already done in previous
work)
- Using an `src` folder in the package but not having to use it when
importing into another package
- Tree-shaking (already done in previous work, check out `exports` in
`./packages/ui.package.json`

Non-goals of this PR:
- Including the entirety of `shadcn-ui` (We're showing you the
fundamentals of how to support `shadcn-ui` here but not bringing in the
components)
- Using `tsup` to create an RSC package (Please open a new issue/add
reactions to an existing issue if you'd like to see this)
@vimtor
Copy link

vimtor commented Dec 9, 2023

Compiling a package before the Next.js Compiler bundles it won't result in faster build times. The Next.js Compiler still must parse the third-party code to create bundles. Achieving better granularity is one of the reasons Turbopack is being built.

The tree-shaking shown is realistic. We use this technique in our monorepo at Vercel. If you'd prefer, you can use

"exports": {
  "./*: "./src/*"
}

but you'll lose auto-importing across package boundaries.

Sorry for the aside, but would compiling the package speed up TypeScript type inference since it can rely on declaration files?

@robknight
Copy link

redirect to the appropriate file when doing a cmd+click: currently in the examples, "go to definition" will bring you to a .d.ts. That's not what devs expect when developing in a monorepo.

I've encountered this issue, and so far as I can tell this is because tsup doesn't generate declaration maps. If you want declaration maps then rather than using tsup --dts, you have to use something like tsup <options> && tsc --emitDeclarationOnly --outDir=dist/types, with "declarationMap": true in your tsconfig.json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: examples Improvements or additions to examples owned-by: turborepo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants