facebook / stylex

StyleX is the styling system for ambitious user interfaces.
https://stylexjs.com
MIT License
8.34k stars 307 forks source link

esbuild plugin with write:false should set file name #500

Closed zpao closed 6 months ago

zpao commented 6 months ago

Describe the issue

I'm using tsup to build some things, which uses esbuild under the hood. It supports using esbuild plugins directly, however stylex's plugin isn't compatible.

The reason is that tsup uses the esbuild api directly and sets write: false, then does work and writes to disk itself. However the plugin here is writing to disk directly itself unless write: false, in which case it seems to do the right thing and pass the contents back. However, it passes <stdout> to the path, which results in tsup writing a file called <stdout> instead of generatedCSSFileName.

Expected behavior

generatedCSSFileName is written

Steps to reproduce

  1. setup basic tsup project
  2. follow stylex esbuild instructions, configuring in tsup.config.js
  3. run tsup

Test case

No response

Additional comments

https://github.com/facebook/stylex/blob/9bd1eecf0fa72a151c74bc7b94959281a73f8af4/packages/esbuild-plugin/src/index.js#L84

I think you can just change this line to generatedCSSFileName. This worked in a quick test modifying inside node_modules, but I'm completely unfamiliar with the expectations of esbuild or plugins here and it may be more complex than this.

zpao commented 6 months ago

Added an example using tsup in my fork (https://github.com/zpao/stylex/tree/tsup-example/apps/tsup-example). It works around the issue reported here (https://github.com/zpao/stylex/blob/tsup-example/apps/tsup-example/tsup.config.ts#L43-L46) but that shouldn't be necessary.

nedjulius commented 6 months ago

hey, I think it should be safe to change the path to generatedCSSFileName, given that there is a use case for it. the only reason path is set to <stdout> is to simply emulate the regular esbuild behavior when it populates the outputFiles array.

nmn commented 6 months ago

I don't have much to add here except that I'm working on some changes that should make most bundler plugins redundant in favour of a LightningCSS or PostCSS plugin.

I'm closing this issue, but if you need further help @zpao, just comment here and I'll re-open it.