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

[CLI] Fix issues with StyleX usage within external packages #731

Open
4 tasks
nmn opened this issue Oct 9, 2024 · 8 comments
Open
4 tasks

[CLI] Fix issues with StyleX usage within external packages #731

nmn opened this issue Oct 9, 2024 · 8 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@nmn
Copy link
Contributor

nmn commented Oct 9, 2024

At it's core, the CLI transforms an entire folder to to transform the JS and extract the CSS. This works well.

You can also manually declare external package dependencies that should be pre-compiled. However, this process is somewhat buggy today.

The task is to fix the problems with pre-compiling external packages.

Details of the task

The CLI lets you define specific packages that should be compiled with StyleX. It is important these packages are not pre-compiled and should only have source files and not compiled files.

The CLI pre-compiles the configured packages into the output folder.

  • Investigate the limitations of the current approach
  • Investigate why the packages are sometimes not compiled correctly on edits. This is specially relevant to monorepos.
  • Fix the bugs to ensure that the CLI reliably compiles configured packages.
  • Ensure that changes in mono-repo packages are tracked and the files re-compiled on edits
@nmn nmn added bug Something isn't working good first issue Good for newcomers labels Oct 9, 2024
@sudheerDev
Copy link

Hey, @nmn Can I pick this up?

@nmn
Copy link
Contributor Author

nmn commented Oct 11, 2024

@sudheerDev You're welcome to.

@devhd9
Copy link

devhd9 commented Dec 6, 2024

@nmn, It's been a while so I'm assuming @sudheerDev is not currently working on this.

I would like to take up this issue and work on fixing the CLI's handling of pre-compiling external package dependencies. Could you please share more details about the specific bugs or challenges currently being faced during this process?

Additionally, it would be helpful if you could elaborate (or share any relevant resources) on the primary goals or motivations for pre-compiling external package dependencies.

Thanks.

@nmn
Copy link
Contributor Author

nmn commented Dec 10, 2024

@devhd9 You can work on it. Note: we want to start sharing code between the CLI and the postcss-plugin going forward so please try to make sure your code is not tightly coupled to the CLI.

@devhd9
Copy link

devhd9 commented Dec 10, 2024

we want to start sharing code between the CLI and the postcss-plugin going forward so please try to make sure your code is not tightly coupled to the CLI.

Sure, I will keep this in mind.

Can you please also give me clarity on following?

the specific bugs or challenges currently being faced during this process?

Additionally, it would be helpful if you could elaborate (or share any relevant resources) on the primary goals or motivations for pre-compiling external package dependencies.

@nmn
Copy link
Contributor Author

nmn commented Dec 10, 2024

@devhd9 I wrote down more details above. Lmk, if there's anything I else I should elaborate on.

the primary goals or motivations for pre-compiling external package dependencies.

The goal is to allow you to use external packages that use stylex within. e.g. component library packages.

@nmn nmn assigned devhd9 and unassigned sudheerDev Dec 10, 2024
@Jta26
Copy link
Contributor

Jta26 commented Dec 10, 2024

Thought I'd chime in on this one as the CLI's original creator to give you some additional context.

Currently the CLI does 2 main functions with regard to external packages prior to compiling the input directories.

  1. Temporarily copies them to the input directory
  2. rewrites the import declarations to reference the newly copied version

I originally attempted to do it by just rewriting imports and copying them directly to the output directory, however when StyleX compiled this way the hashes it compiled to were incorrect because I'm pretty sure it uses the import declaration and source directory to create the CSS var hashes.

Most of the issues probably lie in the fact that it is copying (and then deleting post-compile) the external packages to the input directory, but there is probably other reasons too. I think a lot of the issues are specifically with running the CLI in watch mode, which might be fixable by moving the CLI off of node-watchman and to something like chokidar.

Also, the CLI really needs some more tests, but that can be a separate issue

@devhd9
Copy link

devhd9 commented Dec 12, 2024

Thanks @nmn and @Jta26, I appreciate your input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants