nrwl / nx

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

Storybook does not load global styles #9378

Closed hgschwibbe closed 2 years ago

hgschwibbe commented 2 years ago

Current Behavior

After migrating NX from 11 to 13 we have problems to integrate global SASS styles into the Storybook.

Expected Behavior

Global styles are applied in Storybook stories

Steps to Reproduce

Take the test project Test.zip The project contains a single library with a single component, and a storybook. The component has a simple HTML element with a red background (styled in the components *.scss file). The global styles from "libs/theme/styles/theme.scss" overrides the background to green by using !important.

The project configuration contains several approaches to integrate theme.scss globally:

  1. Configuring styles and preprocessor options. I am not sure if the documentation is right here, as far as I know stylePreprocessorOptions is a seperate option and not part of the options section. But to be sure I configured both places.
  2. Configuring SCSS preset for Storybook
  3. Extending Storybook’s webpack config

None of those approaches are working currently.

The App shows the styles in the correct way because theming SASS has been imported into the App's global styles.scss: App

But the Storybook does not take global styling into account: Storybook

The only way to make it work is to import theme.scss into the components scss file as well. But this would import theming multiple times when its used in the Apps. And it would be a maintenance horror if you have to import global theming into all components in the whole NX workspace explicitly.

AgentEnder commented 2 years ago

This looks similar to #7054 , can you run nx report so I can see your versions a bit more clearly?

hgschwibbe commented 2 years ago

   Node : 16.14.0
   OS   : linux x64
   npm  : 8.3.1

   nx : 13.9.1
   @nrwl/angular : 13.9.1
   @nrwl/cypress : 13.9.1
   @nrwl/detox : undefined
   @nrwl/devkit : 13.9.1
   @nrwl/eslint-plugin-nx : 13.9.1
   @nrwl/express : undefined
   @nrwl/jest : 13.9.1
   @nrwl/js : undefined
   @nrwl/linter : 13.9.1
   @nrwl/nest : undefined
   @nrwl/next : undefined
   @nrwl/node : undefined
   @nrwl/nx-cloud : undefined
   @nrwl/nx-plugin : undefined
   @nrwl/react : undefined
   @nrwl/react-native : undefined
   @nrwl/schematics : undefined
   @nrwl/storybook : 13.9.1
   @nrwl/web : undefined
   @nrwl/workspace : 13.9.1
   typescript : 4.5.5
   rxjs : 7.4.0
   ---------------------------------------
   Community plugins:
     @storybook/angular: 6.4.19

(node:6376) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./" in the "exports" field module resolution of the package at /home/hgsch/sourcecode/test/node_modules/tslib/package.json.
Update this package.json to use a subpath pattern like "./*".
(Use `node --trace-deprecation ...` to show where the warning was created)
hgschwibbe commented 2 years ago

And my global installed Angular CLI is:


    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/

[NX] Angular devkit readJsonWorkspace fell back to Nx workspaces logic

Angular CLI: 13.2.6
Node: 16.14.0
Package Manager: npm 8.3.1
OS: linux x64

Angular: 13.2.6
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1302.6
@angular-devkit/build-angular   13.2.6
@angular-devkit/core            13.2.6
@angular-devkit/schematics      13.2.6
@schematics/angular             13.2.6
ng-packagr                      13.2.1
rxjs                            7.4.0
typescript                      4.5.5
webpack                         5.70.0
mandarini commented 2 years ago

Hi there @hgschwibbe ! I am seeing that you are not configuring your project settings in the correct way.

In your libs/theme/project.json you can do the following:

  1. You can remove the styles and stylePreprocessorOptions from the build target
  2. In the storybook and build-storybook targets you can remove stylePreprocessorOptions and only keep styles
  3. In the storybook and build-storybook targets, change projectBuildConfig to theme:build-storybook instead of just theme. So it should be like this: "projectBuildConfig": "theme:build-storybook"

The full result:

libs/theme/project.json ``` { "projectType": "library", "root": "libs/theme", "sourceRoot": "libs/theme/src", "prefix": "test", "targets": { "build": { "executor": "@nrwl/angular:ng-packagr-lite", "outputs": ["dist/libs/theme"], "options": { "project": "libs/theme/ng-package.json" }, "configurations": { "production": { "tsConfig": "libs/theme/tsconfig.lib.prod.json" }, "development": { "tsConfig": "libs/theme/tsconfig.lib.json" } }, "defaultConfiguration": "production" }, "test": { "executor": "@nrwl/jest:jest", "outputs": ["coverage/libs/theme"], "options": { "jestConfig": "libs/theme/jest.config.js", "passWithNoTests": true } }, "lint": { "executor": "@nrwl/linter:eslint", "options": { "lintFilePatterns": [ "libs/theme/src/**/*.ts", "libs/theme/src/**/*.html" ] } }, "storybook": { "executor": "@nrwl/storybook:storybook", "options": { "uiFramework": "@storybook/angular", "port": 4400, "config": { "configFolder": "libs/theme/.storybook" }, "projectBuildConfig": "theme:build-storybook", "styles": ["libs/theme/styles/theme.scss"] }, "configurations": { "ci": { "quiet": true } } }, "build-storybook": { "executor": "@nrwl/storybook:build", "outputs": ["{options.outputPath}"], "options": { "uiFramework": "@storybook/angular", "outputPath": "dist/storybook/theme", "config": { "configFolder": "libs/theme/.storybook" }, "projectBuildConfig": "theme:build-storybook", "styles": ["libs/theme/styles/theme.scss"] }, "configurations": { "ci": { "quiet": true } } } }, "tags": [] } ```
hgschwibbe commented 2 years ago

Thanks, that was helpful, it works.

But only one question, why did you just close this issue? From my understanding @nrwl/angular:storybook-configuration should generate something like "projectBuildConfig": "theme:build-storybook" in the future right?

Therefore some fix should be provided.

mandarini commented 2 years ago

Oh I see what you are saying. I will push for that change, eventually, sure.

SuneRadich commented 2 years ago

I am also struggling with this issue after upgradring from Angular 12 to latest nx/angular.

Do I understand it correctly, that the current fix for this, is to manually edit project.json files for all apps/libraries with storybook. And that I have to manually add the same projectBuildConfig to all new libraries? This would be fine for a small repo, but when. you have several developers working in the same monorepo - this will cause problems, because everyone will forget when and how to do this, and it will probably be done in different ways.

Im starting to wonder if having global styling/sass import path aliases is bad practice?

mandarini commented 2 years ago

Hi there @SuneRadich ! There's no need to do anything manually! If you're using the nx migrate command, we have migrators in place that will add projectBuildConfig to all your projects, automatically! :)

SuneRadich commented 2 years ago

@mandarini Yes, I am aware of that. But the default points to the projets own config, and not a shared storybook app (like previously with defaultProject. So when I need to include the same sass mixins and global variables in each and every storybook - I have to manually update.

And this is fine for existing Storybook configs, but whenever we create a new lib, with its own storybook, we have to make this change - unless there is some config option I missed somewhere. Having to do this manually will lead to some of our junior developers to spend a lot of time debugging why their stories do not include our default font, or knows about our sass import paths.

mandarini commented 2 years ago

Ohhhh @SuneRadich I see what you're saying! We could introduce an extra option to the generator for custom projectBuildConfig, I think this would solve the issue.

mandarini commented 2 years ago

here you go @SuneRadich :)

github-actions[bot] commented 1 year ago

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.