nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.17k stars 2.3k forks source link

Perf: ⚡ Drop existing TailwindCSS support docs in favor of 1000% speed improvements #20422

Open bjornharvold opened 9 months ago

bjornharvold commented 9 months ago

Documentation issue

Is there a specific documentation page you are reporting?

Nx / Angular w. TailwindCSS

Additional context or description

We initially used Nx Tailwind support but switched this week, after stellar performance improvements, suggested by @alan-agius4. It brought both our build and reload times from 600 -> 18 seconds and 296 -> betw. 0.1 - 6 seconds respectively.

information can be found here: https://github.com/angular/angular-cli/issues/26378

In essence

This became necessary after migrating to the new esbuild builder and serve times became unmistakably noticeable because the Nx incremental dev server with esbuild support is not released yet. [I'm actually glad it's not because it forced us to re-visit the existing slow serve times in comparison with the normal Bootstrap SCSS serve times we have.

My recommendation is to drop this type of Tailwind support and just recommend the above-mentioned solution to users.

🍺🦸‍♂️ As always, mucho respect to Team Nx. You make our lives easier. This issue is just one way for me to help out in a small way.

Cheers, Bjorn

michaelangeloio commented 9 months ago

this is pretty good to know. Thank you!

leosvelperez commented 9 months ago

@bjornharvold That's a hell of a perf improvement. Big kudos to @alan-agius4!

I'll spend some more time on this to see if, apart from this, there are additional use cases we should document and if there's anything we can/should do in our generators. This is valuable info. Thanks for bringing it to our attention!

That said, I think there's room for improvement in your setup. I say this based on the provided description and the issue linked from the Angular CLI. I may be missing some context here. If I understood something wrong, please let me know.

In your setup, you mentioned you have the following in all your component stylesheets:

@tailwind components;
@tailwind utilities;

With that, TailwindCSS will treat every component stylesheet as style entry points and output all styles for the components and utilities layers used by the content files in each component stylesheet. You'll end up with duplicated style definitions. Even with the optimization proposed by Alan (which correctly narrows the contents to analyze by TailwindCSS), you probably end up with bigger bundles due to duplicated styles.

Say you have:

<!-- a.component.html -->
<p class="font-bold">Component A</p>
<!-- b.component.html -->
<p class="font-bold">Component B</p>

If their component stylesheets have those TailwindCSS directives, both components will have the font-bold class defined in their output styles. This is because the font-bold class is part of the TailwindCSS utilities layer. Apart from that, the application stylesheet will also have all the TailwindCSS classes used throughout your application if the content includes the dependencies.

That is a contrived example to illustrate the issue, but with 500+ components, you most likely have a lot of duplicated styles across them. Is there a particular reason you're adding those TailwindCSS directives to each component stylesheet? Those directives should only be needed in stylesheets meant to be style entry points (e.g. global styles like the styles.css file).

Please note we don't recommend such a setup in the reported docs. We don't currently have any use case that would require such a setup.

Also, from what you described, your libraries are buildable, but you're not consuming their outputs when building the application with esbuild. It's probably fine not to use incremental builds given esbuild is way faster than webpack, but you have extra setup that's meant to be used by incremental builds and probably added some noise to the perf issue you had.

If you were not using incremental builds (having the app consuming the buildable libraries build outputs), the individual tailwind.config.js files at the root of your projects were not really used by the application build. That didn't impact the performance. What impacted the performance was having each component stylesheet as a TailwindCSS entry point where each of them uses the broad content in the TailwindCSS configuration used by the application.

If you didn't have those @tailwind directives in your component stylesheets, the perf would have been good as well, since the only stylesheet invalidated by changes would have been the application's styles.css. As Alan explained, if each component stylesheet is a TailwindCSS entry point, then you need to narrow the scope of the content for each of them, and have the application's content also narrowed down to only the application project source code.

bjornharvold commented 9 months ago

Hi @leosvelperez

If I didn't have those directives in each component, CSS wouldn't get applied on that component. I might get the CSS that happened to be in another component, but it would be missing the specific stuff.

The tailwind directives in styles.scss only works on components that are in the app. We have no components in our app.

Hope this clarifies.

leosvelperez commented 9 months ago

@bjornharvold maybe I'm not understanding your scenario well enough. In the linked issue from the Angular CLI repo, you mentioned the following in this comment:

All our standalone components have only 2:

@tailwind components; @tailwind utilities;

That's why I mentioned the above. Granted, it could only be for the standalone components, and you might have non-standalone ones, but still, you mentioned you have/had those directives in those components. If that's not the case or I didn't understand your setup correctly, please disregard it.

The changes proposed by Alan and the improvements gained by doing so are great and I'm not disputing that. I really appreciate you contributing back with that valuable info. I'll try to take a deeper look at our guides at some point this week and I'll update them with better setup recommendations for the buildable libraries scenario.

bjornharvold commented 9 months ago

Correct. I have those 2 tailwind directives for every Standalone component. If I don't, tailwind classes do not get picked up. I also had tailwind config entries for every standalone component. The config you get when adding tailwind support w Nx.

leosvelperez commented 9 months ago

When you say "tailwind classes do not get picked up", which classes are you referring to?

Adding the @tailwind directive should have no effect on the former and if it has an effect on the latter, it's an indication of an issue with the tooling (the builder or executor being used).

Please keep in mind that normally TailwindCSS classes are not picked up if they are in files that are not part of the content glob patterns. That would be the first place to troubleshoot to understand why they are not being picked up. I'd be happy to help troubleshoot this if you can put together a reproduction.

Again, you shouldn't need to add those directives to any component stylesheet. That affects your bundle size (TailwindCSS classes will be duplicated across component stylesheets) and build performance.

bjornharvold commented 9 months ago

Hi @leosvelperez

If I have a component and the html contains a TailwindCSS class [all using my custom prefix] that is not used anywhere else, that class will not get applied by the pre-processor unless I have the Tailwind directives included in my .component.css file for that component.

Does that clarify things?

leosvelperez commented 9 months ago

Things are clear to me now, but I might not be conveying the explanation correctly to you. The explanation here https://github.com/nrwl/nx/issues/20422#issuecomment-1850056472 still applies.

TailwindCSS scans all files that match the glob patterns provided in the content option of the configuration. There's no other hidden magic or requirement to have the utility classes detected and generated in the final stylesheet. As long as those glob patterns match all the files where you set TailwindCSS classes, the classes will be picked up. You can see that explained in their docs:

Tailwind CSS works by scanning all of your HTML, JavaScript components, and any other template files for class names, then generating all of the corresponding CSS for those styles.

In order for Tailwind to generate all of the CSS you need, it needs to know about every single file in your project that contains any Tailwind class names.

Configure the paths to all of your content files in the content section of your configuration file

Please note the collected CSS classes (the ones used by the files matched) will be output wherever the corresponding @tailwind directives are. Most of the time, you only want/need that to be in the application global styles.css. They are not going to be generated in the component styles unless you set those directives there.

The fact you say those classes are not getting processed is, again, an indication that the component template is not being picked up by the glob patterns in the content option of the Tailwind CSS configuration. The first place to check is that configuration and confirm that all your source files are correctly matched by the configured glob patterns.

leosvelperez commented 9 months ago

I just re-read through the thread and in https://github.com/nrwl/nx/issues/20422#issuecomment-1847048068 you said:

The tailwind directives in styles.scss only works on components that are in the app. We have no components in our app.

I missed that previously, but, it shouldn't matter if you don't have components in the app. As long as the libs where the components are located and consumed from are part of the dependency graph of the app and you have the createGlobPatternsForDependencies helper in the content option of the app's TailwindCSS configuration, those components should be processed correctly.

So, if they are not processed, it can be due to a number of things:

If you're not using an incremental build setup, you don't need each library to be compiled on its own. Their code should be compiled as part of the application's code.

bjornharvold commented 9 months ago

When we first added Tailwind to our codebase, we followed Nx instructions.

Except for the config file Nx generates when adding Tailwind support, the rest was manual cutting and pasting from an existing Tailwind component. Sometimes we'd forget to paste the Tailwind directives in the css file and we'd wonder for a second why the styles weren't showing. The moment we applied the directives, Tailwind classes got applied.

... but it did get slower and slower. The incremental build was the only thing that saved us from going insane.

With this new setup, that Alan suggested, we are back developing in full force 🦸🏼‍♂️

leosvelperez commented 9 months ago
  • Shared Tailwind config that each standalone component extends from

Mmm, that's not suggested anywhere in the Nx docs. I wonder if it's a terminology or communication issue. What's suggested is to have a TailwindCSS configuration in buildable (or publishable) libraries. So, it's not per standalone component.

That said, that's a suggestion for incremental builds. Part of your issue was that you had a setup for incremental builds, but you were no longer using incremental builds. When you switched to using esbuild, you were no longer using incremental builds, so your setup for TailwindCSS needed to change. In that case, the setup should have been the one described here, but you were still using the one recommended for incremental builds.

The current recommendations in our docs need improvements, no doubt, but you were in a worse state than you should have been because your setup was not the correct one, and it was not recommended.

I'm convinced your setup still needs improvement, and something else was/is wrong that made you add those directives to your components stylesheets. I have worked with applications and created tons of samples using TailwindCSS, and I never needed to do so. You shouldn't need to use the @tailwind directives anywhere other than your application styles.css, but I don't want to sound like a broken record (I have said it many times now 😅). I feel we're going in circles here, and you seem happy with your setup, so let's leave it there.

I'm going to work on updating the documentation taking into account the information you provided and the great tips from Alan. Thanks for the info!

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it hasn't had any activity for 6 months. Many things may have changed within this time. The issue may have already been fixed or it may not be relevant anymore. If at this point, this is still an issue, please respond with updated information. It will be closed in 21 days if no further activity occurs. Thanks for being a part of the Nx community! 🙏