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

Using Monorepo packages in a Next.JS app #63

Closed
paulm17 opened this issue Dec 6, 2023 · 25 comments
Closed

Using Monorepo packages in a Next.JS app #63

paulm17 opened this issue Dec 6, 2023 · 25 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@paulm17
Copy link

paulm17 commented Dec 6, 2023

Side point, genuinely excited for this project as soon as I heard about it from Theo.

The problem
I know this library is new and not all edge cases have been covered. However, I am trying to use styleX within a pnpm package. Is this a covered use case or a future one? 🤔

I have two pnpm repos that I've tried this with:

  1. a project I'm working on and sadly it's too complex to come up with a simple repo to showcase. But when I instantiate styles.create and build the package, I get the following error:

Error: Unsupported Server Component type: undefined

Without using stylex, the component builds correctly and is able to be imported by the nextjs app just fine.

  1. I'm cloning create-t3-turbo (for a basic pnpm setup) and removing many unused things just to get it working to show the issue.

When I launch nextjs:

../../packages/ui/src/button/button.tsx:21:6
Syntax error: Unexpected token, expected ","

  19 |   return (
  20 |     <button
> 21 |       {...stylex.props(styles.base)}
     |       ^
  22 |       onClick={() => alert(`Hello from your ${appName} app!`)}
  23 |     >
  24 |       {children}

Again, if I remove stylex (and the onClick) there is no issue the button renders.

How to reproduce

  1. clone: https://github.com/paulm17/stylex
  2. pnpm i
  3. pnpm run dev

Expected behavior
should be able to run both as a package and then when built and finally when on npmjs.


Please let me know if I have not followed any documentation or have done something incorrectly!

Many thanks for this library!

@nmn nmn added the bug Something isn't working label Dec 6, 2023
@nmn
Copy link
Contributor

nmn commented Dec 6, 2023

The root cause of the problem here is that the webpack plugin created for Next isn't running on files outside your next app. I think Next.js aggressively disables custom transformations on node_modules, which includes other packages within the same mono-repo.

Moving the button component to the app dir fixes the problem. I was able to render a button and style it.

We will need to contact the Next.js team to help resolve this issue.

@nmn nmn changed the title Stylex within pnpm package Using Monorepo packages in a Next.JS app Dec 6, 2023
@kevintyj
Copy link
Contributor

kevintyj commented Dec 6, 2023

@nmn might be a good idea to add a bug:upstream label to label issues that are with dependencies.

@nmn nmn added the help wanted Extra attention is needed label Dec 6, 2023
@nmn
Copy link
Contributor

nmn commented Dec 6, 2023

@kevintyj This is not necessarily a NextJS bug, but rather a bug in our NextJS plugin.

@pheew
Copy link

pheew commented Dec 7, 2023

Have you tried https://nextjs.org/docs/architecture/nextjs-compiler#module-transpilation?

@nmn
Copy link
Contributor

nmn commented Dec 9, 2023

@pheew Not yet. I have been thinking about some other related issues so we fix them all holisically.

#111

@HQ92
Copy link

HQ92 commented Dec 15, 2023

I've also been trying out StyleX within a custom turbo monorepo with a nextjs app inside. Having issues where I don't think stylex is properly configured in the build step, something is missing somewhere as i keep running into "Error: stylex.create should never be called. It should be compiled away.". The important parts of my repo are very similar to the example posted in the original message above.

@nmn
Copy link
Contributor

nmn commented Dec 15, 2023

@HQ92 Your problem seems a bit different since your compilation isn't triggering at all. NextJS, even with a monorepo, should work as long as you're not using the defineVars API.

Would you be able to share some more details about your project?

@HQ92
Copy link

HQ92 commented Dec 20, 2023

@HQ92 Your problem seems a bit different since your compilation isn't triggering at all. NextJS, even with a monorepo, should work as long as you're not using the defineVars API.

Would you be able to share some more details about your project?

I've had a look at the repository over the past week now and found that it was related to something which seems quite silly to be honest. Something that could easily be fixed by updating the docs a little. The issue being that the repository was previously using styled-components and therefore did not have a single css file import for stylex to latch onto. I happened across this issue on the repository, #109, after creating a new nextjs project which happened to come with css modules in which I had no issues until I removed all of the existing css.

I think at the moment the solution would be to just update the docs under installation to make clear that there needs to be some sort of css file imported, even if empty at the moment for it to work with nextjs, I don't know if this is also an issue just generally but in the nextjs case it could potentially help any future styled-components or css-in-js users looking to try stylex as an alternative.

@nmn
Copy link
Contributor

nmn commented Dec 20, 2023

@HQ92 This is a limitation of our NextJS plugin which, along with all our bundler plugins, is still experimental. I've personally reached out to the Next.js team and waiting for someone's help to make the integration better. I'll try to improve the documentation to make this gotcha clear.

Also, I think it should be possible to throw better errors in the current plugin in this case.

@luskin
Copy link

luskin commented Dec 20, 2023

We are receiving this error with the most simple usage (no defineVars, no external component libary). We are running pnpm within a Turbo repo, but this test NextJS app itself is completely isolated.

@nmn
Copy link
Contributor

nmn commented Dec 21, 2023

@luskin Would you be willing to please share an example project with your setup so I can debug. There are some known issues with pnpm at the moment, but they should only affect usage of defineVars from other packages.

@luskin
Copy link

luskin commented Dec 26, 2023

@nmn we solved this by moving from direct typescript import of the stylex ui from packages/ui to having rollup compile our ui into a dist first. It would be nice to be able to dev without having to re-run rollup on every save, but that's how we unblocked ourselves for now.

@luskin
Copy link

luskin commented Dec 26, 2023

@nmn I reproduced for you here: https://github.com/luskin/stylex-issue

@nmn
Copy link
Contributor

nmn commented Dec 26, 2023

@luskin You're running into a problem in the NextJS plugin where the default typescript configuration isn't set up correctly for TSX files. This has been fixed on main and will be fixed in the next release. Sorry about the trouble. There are some other issues, such as the use of next/font which isn't supported, but I'll make a PR to your test repo after the next stylex release.

@nmn
Copy link
Contributor

nmn commented Dec 29, 2023

The issue has been fixed in v0.4.1 and PNPM/NPM monorepos are now correctly working.

I've created a PR to your repo with the fix that I tested locally.

@nmn nmn closed this as completed Dec 29, 2023
@emilgpa
Copy link

emilgpa commented Dec 29, 2023

@nmn in my case, the fix not working with 0.4.1. Before, in 0.3.0 throws: Syntax error: Unexpected token, expected ",". Now, in 0.4.1, throws: stylex.create should never be called. It should be compiled away..
My setup is pretty similar to the repo of @luskin (the only difference is that i not use turbo, I use directly pnpm workspaces) and I have various paths in tsconfig.json

@emilgpa
Copy link

emilgpa commented Dec 29, 2023

for some reason it works now. Maybe it was something cache related, since I've tried to reproduce the error and it no longer appears...

@nmn
Copy link
Contributor

nmn commented Dec 30, 2023

@emilgpa It's likely a cache issue. Please see our "predev" and "prebuild" scripts which delete the .next folder before every run.

@HQ92
Copy link

HQ92 commented Dec 31, 2023

@emilgpa It's likely a cache issue. Please see our "predev" and "prebuild" scripts which delete the .next folder before every run.

I was also having this issue with building stylex on vercel in production. It would be quite temperamental in how it would choose to generate styles and apply them. Nuking the .next folder predev and prebuild fixed this, however is this something that will continue to be the case going forward since it does result in longer builds due to something that seems to be a stylex issue? I'm assuming it's something that will be temporary due to stylex being effectively still in beta?

@HQ92
Copy link

HQ92 commented Feb 2, 2024

The issue has been fixed in v0.4.1 and PNPM/NPM monorepos are now correctly working.

I've created a PR to your repo with the fix that I tested locally.

The original repo from the issue is not available, could you detail the changes you made to get stylex working inside a ui package inside a turborepo?

@nmn
Copy link
Contributor

nmn commented Feb 3, 2024

@HQ92 We're working on a CLI, which will let you pre-compile a folder into the src/ folder for Next.js. This sidesteps the Next build setups and should solve all the various problems we've been dealing with.

@Poolshark
Copy link

Just ran into the same issue at my end. @nmn thanks for pointing the Next.js issue out numerous times! 🙌

We're working on a CLI, which will let you pre-compile a folder into the src/ folder for Next.js.

I personally think developing a CLI is probably the best solution as it will probably bypass this entire plugin integration mess. I don't mind using Babel but I'm still not sure what the @babel-plugin is messing with the build in general.

Is there a timeline of if/when a CLI will be available?

@nmn
Copy link
Contributor

nmn commented Apr 1, 2024

@Poolshark Next.js uses SWC instead of Babel these days. Many new features are buggy in Babel. Hence the need for a CLI instead of a better integration.

SWC plugins are written in Rust which is a much bigger amount of work that we don't need internally so we can't justify investing too much time into that.

@Poolshark
Copy link

Poolshark commented Apr 1, 2024

@nmn totally understand.

Although, I do not understand why Meta is still relying on Babel these days. As you have said, SWC (or any plugin/compiler written in Rust) is much more performant. I can't remember when Babel did not give me a headache when transpiling TS and Relay ...

However, since StyleX now completes the Meta stack (React, Relay, GraphQL, StyleX), it would be a shame not to keep pushing it. I guess with a growing community things will happen automatically.

Long story short, I guess your answer is that we should not expect a CLI soon? Or are you referring to plugins only?

If there is time I'll try to dig into it, maybe I can help out a bit.

@nmn
Copy link
Contributor

nmn commented Apr 1, 2024

Or are you referring to plugins only?

I'm only referring to a SWC plugin. Any JS-based plugins are fair-game.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

8 participants