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

Angular: global style entry points are bundled into JS instead of separate CSS files #15351

Closed
JamesHenry opened this issue Jun 24, 2021 · 9 comments

Comments

@JamesHenry
Copy link

Describe the bug
Honestly, this is one of those things where it seems so obviously an issue that I am half expecting you to tell me I am misunderstanding 😅

As you know, in angular.json you provide "styles": [] on your project - these are global entry points and when you run ng build they will be spat out as individual CSS files via the webpack build.

When building storybook, however, even though I can see it is using angular devkit's webpack config for styles behind the scenes, it does not respect this and instead bundles the global CSS files into JS bundles. This is definitely not what I want.

To Reproduce

I created a brand new Nx workspace from scratch using the angular preset and simply added storybook, nothing else is needed: https://github.com/JamesHenry/storybook-test

To reproduce, simply compare the relevant dist entries after running nx build vs nx build-storybook.

System

 System:
    OS: macOS 11.4
    CPU: (8) x64 Apple M1
  Binaries:
    Node: 14.17.0 - ~/.volta/tools/image/node/14.17.0/bin/node
    Yarn: 1.22.10 - ~/.volta/tools/image/yarn/1.22.10/bin/yarn
    npm: 7.12.1 - ~/.volta/tools/image/npm/7.12.1/bin/npm
  Browsers:
    Safari: 14.1.1
  npmPackages:
    @storybook/angular: ^6.2.7 => 6.3.0 
    @storybook/builder-webpack5: ^6.2.7 => 6.3.0 
    @storybook/manager-webpack5: ^6.3.0 => 6.3.0 

Additional context

If you end up telling me this is the intended behaviour in storybook, then please provide some guidance on how I can work around it. I have a use-case where I very much need to spit out individual global CSS files (client-based theming).

@JamesHenry
Copy link
Author

JamesHenry commented Jun 27, 2021

@shilman You have removed bug and added support, so does that mean you are saying this is the intended behaviour and storybook is intentionally overriding angular CLI’s standard behaviour?

@shilman
Copy link
Member

shilman commented Jun 27, 2021

This needs some discussion before we agree it's a bug -- for now it seems more like a question to me. @ThibaudAV @kroeder any thoughts? Happy to call it a bug once we've confirmed it.

@ThibaudAV
Copy link
Contributor

I want to help you to find a solution, but I don't understand very well 😁 How do you handle the change of theme?

If you have several themes and only one project with angular styles put everything in one file?
you do not have a parent class for example dark-theme

@JamesHenry
Copy link
Author

JamesHenry commented Jun 28, 2021

@ThibaudAV Allow me to break it all down, hopefully that will help:

How and why the Angular CLI build piece works

  • The application, let's call it my-app is developed by a company for several clients/customers who have their own theme for the application which is appropriate for their brand - logos, colour differences etc. Therefore things need to be developed in such a way that not all the possible styles are included in every deployment of the application.

  • We therefore definitely do not want a theming approach based on toggling a class because that implies shipping all possible CSS to all possible deployments.

  • The Angular CLI has a "styles" configuration property on projects which allows you to specify global entrypoints in your webpack compilation.

Here is an anonymized example of real configuration:

image

  • Thanks to the Angular CLI's webpack configuration, these entrypoints end up as .css files when you build the app, they are not wrapped in JS bundles.

image

This is exactly what we want.

apps/my-app/src/styles.scss -> styles.css
The global foundational styles of the application that are always present/required. This file is injected into the index.html automatically by the Angular CLI.

libs/shared/client-configuration/src/lib/default/styles.scss -> styles-default.css
libs/shared/client-configuration/src/lib/foo/styles.scss -> styles-foo.css
libs/shared/client-configuration/src/lib/bar/styles.scss -> styles-bar.css
The theming-specific styles (colours etc) of the application that are differentiated per client/customer deployment ("default" is just a default theme for customers that don't want any customizations). These files are not injected into the HTML markup automatically (thanks to "inject": false) and will be included by us only where appropriate for a particular client/customer deployment. E.g. styles-bar.css will never be used for the customer called "foo".


Relevance and issue with Storybook

  • When developing components for this and the other client-themed applications in Storybook, we naturally want to be able toggle the client theme we are working on from time to time so that we can see the realistic end results of our work.

  • I therefore created a very simple Storybook toolbar add-on (thanks for making this so easy to achieve) which is a dropdown of client/theme names which on change ultimately swaps in a <link> tag with the appropriate stylesheet for the selected theme. It works great - so far, so good!

  • However, the reason for reporting this issue is down to when it comes to serving and building Storybook - for some reason Storybook is changing the behaviour of the Angular CLI's resolved webpack configuration somewhere internally because instead of producing .css files from the above "styles" config, it is wrapping all the global CSS sources in JS bundles.

  • The only way we can work around this currently is to build the full application first, then serve or build storybook while pointing it to the dist of the app as a static dir. Naturally, this is very inefficient and will only get worse as the application grows!

  • I have not been able to think of a reason why you would need to change the Angular CLI's logic in this regard to make Storybook work, so hopefully this is something which can be amended.


Hopefully everything is clear now, please let me know if not. If we can adjust the logic within Storybook Angular so that it stops overriding the Angular CLI's webpack config in this regard then this issue will be resolved.

@ThibaudAV
Copy link
Contributor

Top ! I understand much better 👍

For another topic I have slightly detailed the logic of angular here
#15246 (comment)

Indeed. I don't know if it's a bug or a missing feature ^^

Storybook uses some angular.json part read by some part of angular-cli code. But not complete only what it needs to complete a global webpack configuration. I think this is an unplanned case 🤷‍♂️

One more question ^^

Why add all the styles of your clients in the same project+configuration in angular.json?
I understand that in one build you can do all your client's styles.
but you will have to do a manual action after the build to add the right styles, right?

Why not either do :

  • several projects (one per client configuration) in angular.json
  • or several configurations and use the fileReplacements or styles to complete, override those by default

In both cases you would have one build per client with directly the right style files

This doesn't really solve the problem unless you want to start a storybook per client 🤷‍♂️

Sorry I don't have perfect workaround 😞

I'll look when I have time to find what is missing in storybook to do this. But I'm worried that it's not just from the angular part ; but the behavior of storybook that wraps angular 🤷‍♂️

@elenachiosa
Copy link

As @JamesHenry perfectly described, our company has a very similar use case.
We are a frontend library that works with many different themes (colors, light/dark mode, different contrast ratios). We also need a theme switcher in our Angular application (so in the same app) to showcase these themes.

In angular.json we specify that these themes should not be injected in the application, we load them dynamically via a theme switcher. But Storybook loads all of them at once, each one overwriting the previous one.
We are currently blocked by this as we do not want to drastically change our Storybook app because of a functionality we need on the Angular app.

@fbaba-nibtravel
Copy link

Firstly, thanks @JamesHenry for creating this and explaining the issue very well! My team and I are facing a similar problem.

@ThibaudAV, in our project, the lazy load css resolution does not happen during compilation time. We compile and deploy all the assets once. We have a ngnix router that understands the dns request and apply the tenant id as request header. Then on the app init, we resolve the tenant id by reading the header and dynamically append the appropriate css file during runtime.

@JamesHenry
Copy link
Author

JamesHenry commented Aug 2, 2021

@ThibaudAV

Why not either do :

  • several projects (one per client configuration) in angular.json
  • or several configurations and use the fileReplacements or styles to complete, override those by default

The application is functionally the same - building it multiple times (which both of these suggestions imply) is definitely not desirable even if there were only 2 clients and it were not much extra time (in our case, however, there are many more than two so from a time perspective it does not scale well at all). We should be building once, deploying N times. In our case which theme css file to package with the app is a deployment-time concern.

Just to update everyone here, what I ended up doing was building a completely custom (therefore I'm afraid unshareable) Angular CLI builder/Nx executor) in order to just compile the style files using the same kind of angular-devkit webpack styles resolution which storybook does internally. I run that builder just before storybook runs and point storybook to the compiled css files as a --static-dir. I then have a custom storybook toolbar add-on to allow me to switch to a different theme file in the storybook UI when it runs.

Again, apologies I will not be able to share any of this as it is under a consultancy contract, so please don't ask 😅

FWIW @ThibaudAV, even though the type information in angular-devkit did not agree, the only significant thing I had to do on top of your existing storybook logic was to add extractCss: true to the buildOptions property of the object you pass to getStylesConfig(...).

I found this simply by looking at the angular-devkit source by seeing what it used to determine if it should produce css files or js files for styles.

Hope that helps!

@shilman
Copy link
Member

shilman commented Jun 8, 2023

We’re cleaning house! Storybook has changed a lot since this issue was created and we don’t know if it’s still valid. Please open a new issue referencing this one if:

@shilman shilman closed this as not planned Won't fix, can't repro, duplicate, stale Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants