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

It won't work without the CSS file generated by the css-loader #109

Open
mrskiro opened this issue Dec 8, 2023 · 15 comments · Fixed by #129
Open

It won't work without the CSS file generated by the css-loader #109

mrskiro opened this issue Dec 8, 2023 · 15 comments · Fixed by #129
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@mrskiro
Copy link

mrskiro commented Dec 8, 2023

The problem

In Next.js, if you don't import any .css file, the styles generated by stylex will not be added to the stylesheet.

How to reproduce

Steps to reproduce:

  1. delete this line
    import './globals.css';
  2. rebuild
  3. not working because not exists.next/static/css

Expected behavior

Is any .css file necessary? Can it be omitted?

Environment (include versions). Did this work in previous versions?

https://github.com/facebook/stylex/blob/c9b205750d835cba82b13ef5cdeaf30bbc683f70/apps/nextjs-example/package.json

@nmn
Copy link
Contributor

nmn commented Dec 8, 2023

Yes. This is a known issue with the Nextjs Plugin. For now, that one CSS file is required. Use it for global styles and resets.

@nmn nmn added good first issue Good for newcomers help wanted Extra attention is needed labels Dec 8, 2023
@timwehrle
Copy link
Contributor

timwehrle commented Dec 11, 2023

@nmn I can work on this.

@nmn
Copy link
Contributor

nmn commented Dec 11, 2023

All yours @timwehrle. There are many problems with the NextJS plugin. Try to fix them in separate small PRs.

Here's an (incomplete) list:

  1. Need for a CSS file
  2. Need for a .babelrc file
  3. Needs to delete .next folder between builds to work consistently.
  4. Need to set a rootDir
  5. Not being able to import @/... paths. (Probably needs a Babel change as well)

@timwehrle
Copy link
Contributor

@nmn

Thank you for the overview. I will get back to you if I need any further help or clarification.

@timwehrle
Copy link
Contributor

@nmn

  1. Need for CSS file

So is the question here just to generate the CSS file if no CSS is found to apply the styles to (and write it in the public directory of Next.js)? Or does it also imply the integration of this CSS file? If so, wouldn't it be smarter to make them inline styles?

@nmn
Copy link
Contributor

nmn commented Dec 11, 2023

@timwehrle I think generating a CSS file when it doesn't exist is probably what we want. This should be possible by importing a virtual module or something like that.

But use your judgement here. I'm not sure what is even possible with the nextjs plugin. I do know that 2. and 3. are probably the most important to fix.

@timwehrle
Copy link
Contributor

@nmn Okay, yes. I wasn't thinking about virtual modules, but yeah, you're right. I will write down how I could do it that way, but mostly I will start on the other tasks.

@nmn
Copy link
Contributor

nmn commented Dec 11, 2023

Thanks for working on this!

@timwehrle
Copy link
Contributor

@nmn

Do you have insights on the developing of the issue #40? Because that's basically the same problem with point 4 and 5. Is there any progress?

@nmn
Copy link
Contributor

nmn commented Dec 12, 2023

@timwehrle Yes. Both of those are currently blocked on changes to the Babel plugin that I will be making. I wrote an RFC about it in Github Discussions

#111

Also, didn't mean to close this task. Reopening.

@nmn nmn reopened this Dec 12, 2023
@timwehrle
Copy link
Contributor

timwehrle commented Dec 12, 2023

@nmn

While figuring out how the Next.js plugin works and how it updates the assets in an existing CSS file, I was thinking about the purpose of adding a CSS file when there is none? Since the CSS file is only generated on build time, the user would have to look where the CSS file is generated. It would be more convenient if we could automatically apply these styles within the Next.js app at build time and on dev.

StyleX isn't like the other CSS-in-JS libraries that all apply styles inline (for now or for ever isn't important tbh). We wouldn't have this problem if we just added the styles to a custom CSS file without updating another CSS file. But how that works is another story.

Regarding virtual modules, their functionality is not entirely clear to me. According to the documentation, we create a module if no CSS file is found. But how can we access the virtual module after building or during development?

I hope I'm not bombing you! Take your time! 😁

@nmn
Copy link
Contributor

nmn commented Dec 12, 2023

It would be more convenient if we could automatically apply these styles within the Next.js app at build time and on dev.

This should be possible by automatically injecting an import for the css file while transforming JS files. I did something similar while making StyleX work with Svelte.
https://github.com/nmn/sveltekit-stylex

Outside of this, I'm not really sure @timwehrle. Sorry, I don't really have more guidance for you.

@timwehrle
Copy link
Contributor

@nmn

Outside of this, I'm not really sure @timwehrle. Sorry, I don't really have more guidance for you.

No problem!

This should be possible by automatically injecting an import for the css file while transforming JS files. I did something similar while making StyleX work with Svelte.
https://github.com/nmn/sveltekit-stylex

Yeah, I'll see what I can do. Thank you!

@timwehrle
Copy link
Contributor

@nmn Other question. Is your Svelte stylex plugin open to use for a personal project?

@nmn
Copy link
Contributor

nmn commented Dec 14, 2023

@timwehrle You don't have to ask me to fork or copy from that repo! It's open source and I've publicly shared it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants