Serve styles and assets using .angular-cli webpack configuration #2735

Quramy · 2018-01-12T18:13:04Z

What I did

Add ability to configure webpack using @angular/cli and user's .angular-cli.json.

So @storybook/anguar serves according to the .angular-cli.json:

Global styles
Static asset files

And if user don't have .angular-cli.json, the minimum configuration is provided as before.

How to test

See example/angular-cli manually 🙏 . I added new component and story under examples/angular-cli/src/stories/component-with-style. Open this story, you can see the following styled DOM:

Scoped CSS(with templateURL)
Scoped SCSS(with templateURL)
Global CSS

And open localhost:9008/assets/common.css, you can confirm that static files listed .angular-cli.json are served.

Quramy · 2018-01-12T18:17:56Z

@igor-dv @alterx how about this?

alterx · 2018-01-12T18:52:33Z

cc @amcdnl :D

alterx · 2018-01-12T19:13:43Z

app/angular/src/server/angular-cli_config.js

+  }
+  const appConfig = cliConfig.apps[appIndex];
+
+  // FIXME dummy value


Which of these are dummy values?

it's outputPath: 'outPutpath'. I'll amend comment 🙇

ndelangen · 2018-01-12T19:45:09Z

app/angular/src/server/config.js

+import {
+  getAngularCliWebpackConfigOptions,
+  applyAngularCliWebpackConfig,
+} from './angular-cli_config';

 // avoid ESLint errors
 const logger = console;


This should use @storybook/node-logger

@Quramy, I think this is the last comment to take care of

ndelangen · 2018-01-12T19:51:56Z

app/angular/src/server/angular-cli_config.js

+    // We should not require('@angular/cli') at the top script level because user project might not have `@angular/cli`.
+    // eslint-disable-next-line global-require, import/no-extraneous-dependencies
+    ngcliConfigFactory = require('@angular/cli/models/webpack-configs');
+  } catch (e) {


I'd like to see this changed a bit.

I'd like to extract a function that detects the installment of @angular/cli.

If it's not installed, we just merge the base, we might log that.
If it's installed we try to get their webpack config.
This might fail, since it's a deep file in a npm module.
We should log in either case.

amcdnl

This is great!

codecov · 2018-01-12T20:32:23Z

Codecov Report

Merging #2735 into master will decrease coverage by 0.24%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2735      +/-   ##
==========================================
- Coverage   35.75%   35.51%   -0.25%     
==========================================
  Files         433      434       +1     
  Lines        9441     9501      +60     
  Branches      995     1010      +15     
==========================================
- Hits         3376     3374       -2     
- Misses       5414     5455      +41     
- Partials      651      672      +21

Impacted Files	Coverage Δ
app/angular/src/server/babel_config.js	`75.86% <ø> (-0.81%)`	⬇️
app/angular/src/server/index.js	`0% <0%> (ø)`	⬆️
app/angular/src/server/build.js	`0% <0%> (ø)`	⬆️
app/angular/src/server/ts_config.js	`38.09% <0%> (-2.82%)`	⬇️
app/angular/src/server/angular-cli_config.js	`0% <0%> (ø)`
app/angular/src/server/config.js	`0% <0%> (ø)`	⬆️
app/react/src/server/config/babel.js	`0% <0%> (-100%)`	⬇️
app/react/src/server/utils.js	`0% <0%> (-53.58%)`	⬇️
lib/ui/src/modules/ui/configs/init_panels.js	`25% <0%> (ø)`	⬆️
... and 66 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 694d5d9...25d2fdb. Read the comment docs.

igor-dv · 2018-01-12T20:59:34Z

app/angular/src/server/config/webpack.config.js

@@ -76,10 +76,6 @@ export default function(configDir) {
          loader: 'raw-loader',
          exclude: /\.async\.css$/,
        },
-        {
-          test: /\.scss$/,


Don't we want to configure scss in case there is no angular-cli ?

I think we should do this:

angular-cli present -> use whatever's in their config

not present? Just fallback to good ol' css.

Technically, if they're not using the cli we aren't officially supporting it. So they should config everything manually. This feels consistent.

@amcdnl what do you think?

I think the vast majority of people will be using the cli so its pretty safe bet. There is the 1% case when they eject from the CLI we might need to think about ( https://github.com/angular/angular-cli/wiki/eject ) but that just nets the 'guts' of the CLI in a basic webpack config file as if you configured it manually yourself.

Personally, I'd fall under the thought process of try to handle as much as you can w/o config (seems to be a trend now i.e. parcel) so leaving the scss wouldn't hurt. Just my 2 cents tho.

so leaving the scss wouldn't hurt

Thanks your feedback, I'll revet this change 😄

…/Quramy/storybook into merge-angular-cli-webpack-config

igor-dv · 2018-01-15T18:05:48Z

app/angular/src/server/angular-cli_config.js

+
+const logger = console;
+
+export function isAngularCliInstalled() {


Why exporting it?

igor-dv · 2018-01-15T18:11:13Z

app/angular/src/server/angular-cli_config.js

+
+  const mod = {
+    ...baseConfig.module,
+    rules: [...cliStyleConfig.module.rules, ...styleRules],


Do we actually need these styleRules if they should be filtered out?

Sorry, the variable name styleRules is so bad 🙇
This array includes rules for .jsx, .ts, ..., so we need it.

I'll change the variable's name.

igor-dv · 2018-01-17T19:01:23Z

@Quramy , you need to update snapshots to fix tests =)

Quramy · 2018-01-18T02:58:28Z

@igor-dv storyshots for Angular 🎉 !
I added the shapshot file 😄

igor-dv · 2018-01-19T17:24:42Z

@Quramy , As I understand, after this fix we can't already import global css into config.js, because now I see this in the example app:

Having this config:

import { configure } from '@storybook/angular';
import addCssWarning from '../src/cssWarning';
import '../src/assets/common.css' // <--- this

addCssWarning();

function loadStories() {
  // put welcome screen at the top of the list so it's the first one displayed
  require('../src/stories');

  // automatically import all story ts files that end with *.stories.ts
  const req = require.context('../src/stories', true, /\.stories\.ts$/)
  req.keys().forEach((filename) => req(filename))
}

configure(loadStories, module);

If it's ok I'll just put the css from common.css to the style.css

Quramy · 2018-01-20T09:59:20Z

@igor-dv

Importing global CSS file manually(import '../src/assets/common.css') is no longer needed.

For instance, in example app src/style.css is listed in .angular-cli.json as global style files:

  "apps": [
    {
      :
      "styles": [
        "styles.css"
      ],
      :
      }
    }
  ],

So it's imported automatically as ng serve does . Specifically, this procedure adds src/styles.css to webpack context.

So, I think we can remove the following lines from example's config:

import '../src/assets/common.css' // <--- this

And move the following selector to src/styles.css from assets/common.css :

.css-rules-warning {
  display: none;
}

therepo90 · 2018-06-27T08:26:51Z

Good job mate. Does it support angular 6?

Quramy added 2 commits January 13, 2018 02:56

add ability to merge ng cli webpack config

3ad322f

add example stories to test styles pattern

b91744c

alterx requested review from amcdnl, alterx and igor-dv January 12, 2018 18:52

alterx reviewed Jan 12, 2018

View reviewed changes

ndelangen reviewed Jan 12, 2018

View reviewed changes

ndelangen requested changes Jan 12, 2018

View reviewed changes

amcdnl approved these changes Jan 12, 2018

View reviewed changes

Merge branch 'master' into merge-angular-cli-webpack-config

91e5353

igor-dv reviewed Jan 12, 2018

View reviewed changes

Quramy and others added 5 commits January 13, 2018 19:16

extract a func to detect wheter ngcli installed

2a5f4bc

chore: modify comment

fb7bd69

Merge branch 'merge-angular-cli-webpack-config' of https://github.com…

6155a0d

…/Quramy/storybook into merge-angular-cli-webpack-config

Merge branch 'master' into merge-angular-cli-webpack-config

1ccbe21

revert to drop scss rules

037c08b

igor-dv reviewed Jan 15, 2018

View reviewed changes

Quramy and others added 6 commits January 16, 2018 10:36

change variable name

a21777f

Merge branch 'master' into merge-angular-cli-webpack-config

3ad33cc

Merge branch 'master' into merge-angular-cli-webpack-config

dc8c138

Merge branch 'master' into merge-angular-cli-webpack-config

d20da16

use node-logger instead of console

44ec036

Merge branch 'master' into merge-angular-cli-webpack-config

d973880

alterx added the angular label Jan 17, 2018

alterx and others added 2 commits January 17, 2018 13:15

Merge branch 'master' into merge-angular-cli-webpack-config

8039b0a

add snapshot file

2539764

igor-dv approved these changes Jan 18, 2018

View reviewed changes

Merge branch 'master' into merge-angular-cli-webpack-config

25d2fdb

alterx approved these changes Jan 18, 2018

View reviewed changes

ndelangen approved these changes Jan 18, 2018

View reviewed changes

alterx merged commit e6e4a0a into storybookjs:master Jan 19, 2018

Quramy deleted the merge-angular-cli-webpack-config branch January 19, 2018 05:38

Quramy mentioned this pull request Jan 19, 2018

Add custom rules Quramy/storybook-angular-cli-helper#1

Closed

Hypnosphi added the feature request label Jan 19, 2018

This was referenced Jan 20, 2018

Fix css warning in angular-cli example #2789

Merged

Webpack configuration - Webpack SCSS loader rule looks wrong #2804

Closed

This was referenced Jan 24, 2018

Angular storybook render blank components #2823

Closed

extending webpack section is no longer needed for the common usage #2826

Merged

alterx mentioned this pull request Feb 1, 2018

Failed when first time trying out storybook-angular. Could you please provide more comprehensive example for Angular? #2616

Closed

igor-dv mentioned this pull request Feb 16, 2018

@storybook/angular cannot load scss file on stories index #2994

Closed

jonrimmer mentioned this pull request Feb 19, 2018

Add warning about importing CSS in Angular CLI projects #3022

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Serve styles and assets using .angular-cli webpack configuration #2735

Serve styles and assets using .angular-cli webpack configuration #2735

Quramy commented Jan 12, 2018 •

edited

Loading

Quramy commented Jan 12, 2018

alterx commented Jan 12, 2018

alterx Jan 12, 2018

Quramy Jan 13, 2018

ndelangen Jan 12, 2018

igor-dv Jan 16, 2018

ndelangen Jan 12, 2018

amcdnl left a comment

codecov bot commented Jan 12, 2018 •

edited

Loading

igor-dv Jan 12, 2018

alterx Jan 12, 2018 •

edited

Loading

amcdnl Jan 14, 2018

Quramy Jan 15, 2018

igor-dv Jan 15, 2018

igor-dv Jan 15, 2018

Quramy Jan 16, 2018

igor-dv commented Jan 17, 2018

Quramy commented Jan 18, 2018

igor-dv commented Jan 19, 2018

Quramy commented Jan 20, 2018 •

edited

Loading

therepo90 commented Jun 27, 2018


		const logger = console;

		export function isAngularCliInstalled() {

Serve styles and assets using .angular-cli webpack configuration #2735

Serve styles and assets using .angular-cli webpack configuration #2735

Conversation

Quramy commented Jan 12, 2018 • edited Loading

What I did

How to test

Quramy commented Jan 12, 2018

alterx commented Jan 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amcdnl left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 12, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

alterx Jan 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igor-dv commented Jan 17, 2018

Quramy commented Jan 18, 2018

igor-dv commented Jan 19, 2018

Quramy commented Jan 20, 2018 • edited Loading

therepo90 commented Jun 27, 2018

Quramy commented Jan 12, 2018 •

edited

Loading

codecov bot commented Jan 12, 2018 •

edited

Loading

alterx Jan 12, 2018 •

edited

Loading

Quramy commented Jan 20, 2018 •

edited

Loading