Angular build time optimization #4118

kroeder · 2018-09-04T21:36:00Z

What I did

Note: This is work-in-progress

Renamed loadTsConfig to getTsLoaderOptions because that is what it actually does (5801057#diff-aaa47dcc2ee48513f45210bb35a92d4cR1)
Set transpileOnly: true as default option for ts-loader
Added fork-ts-checker-webpack-plugin as a dependency in app/angular
Use ForkTsChecker and set option tsconfig to the same that is used for ts-loader (using getTsLoaderOptions)

Checklist

Set transpileOnly: true as default ts-loader option
Add ForkTsChecker to the list of plugins
Verify it works
Check, why ForkTsChecker does check jest (ERROR in D:/04_Development/ng-projects/storybook/node_modules/@types/jest/index.d.ts(1103,9): and many more in this file)
Update documentation
Update test-case ts_config.test.js

How to test

Is this testable with Jest or Chromatic screenshots? yes
Does this need a new example in the kitchen sink apps? no
Does this need an update to the documentation? yes

If your answer is yes to any of these, please make sure to include it in your PR.

Manual test:
Start yarn storybook in examples/angular-cli/

Using ForkTsChecker adds the folloing message when starting storybook

Starting type checking service...
Using 1 worker with 2048MB memory limit

For maintainers only: Please tag your pull request with at least one of the following:
["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

kroeder · 2018-09-04T21:37:16Z

Please note: I had yarn test --core errors right from the beginning when forking master. Is this a known thing?

ndelangen · 2018-09-04T22:12:10Z

@igor-dv please verify if this is compatible with your preset PR

igor-dv · 2018-09-06T20:03:51Z

I will review it soon (or when I will be bored of doing nothing in a vacation)

app/angular/src/server/ts_config.js

igor-dv · 2018-09-10T11:08:31Z

app/angular/src/server/wrapInitialConfig.js

@@ -1,45 +1,55 @@
 import path from 'path';


please solve merge conflicts here, it moved to framework-preset-angular

igor-dv · 2018-09-10T11:10:30Z

app/angular/src/server/wrapInitialConfig.js

+        path.resolve(__dirname, '..')
+      ),
+      new ForkTsCheckerWebpackPlugin({
+        tsconfig: tsLoaderOptions.configFile,


what will happen if there is not a configFile ?

BTW, if tslint is always located in root, we can also add it here (just check if it exists).

kroeder · 2018-09-10T12:07:02Z

Thanks for your comments! Do you think I should revert + change my approach to fit in into the new template feature?

igor-dv · 2018-09-10T12:29:03Z

I don't think you need to revert. The preset has a similar structure, just move your changes there

igor-dv · 2018-09-16T06:27:59Z

@kroeder, do you need help to finish this?

kroeder · 2018-09-16T06:37:06Z

Normally I would say "no" but I spent hours trouble shooting why my project does not build, start, tests on master fail.. Even on a fresh checkout :( btw. I am the one asking lots of questions in slack. I'll try to continue today

# Conflicts: # app/angular/src/server/wrapInitialConfig.js Created a function for ForkTsCheckerWebpackPlugin Added tests for ForkTsCheckerWebpackPlugin

…; Updated tests

kroeder · 2018-09-16T12:01:41Z

I created a seperate file for creating an instance of ForkTsChecker and added tests for it.

app/angular/src/server/framework-preset-fork-ts-checker-plugin.js

igor-dv · 2018-09-18T07:09:25Z

examples/angular-cli/package.json

@@ -9,7 +9,7 @@
    "e2e": "ng e2e",
    "ng": "ng",
    "start": "ng serve",
-    "storybook": "npm run storybook:prebuild && start-storybook -p 9008 -s src/assets",
+    "storybook": "start-storybook -p 9008 -s src/assets",


I've fixed this, please merge from master and revert this

…ion with @types/jest

kroeder · 2018-09-18T20:38:46Z

Checks are fine now, finally! I just need to integrate the cli stuff as well as the updated documentation for this PR. Anything else you want to add/change?

codecov · 2018-09-19T06:59:44Z

Codecov Report

Merging #4118 into master will increase coverage by 0.03%.
The diff coverage is 75%.

@@            Coverage Diff             @@
##           master    #4118      +/-   ##
==========================================
+ Coverage   40.75%   40.78%   +0.03%     
==========================================
  Files         492      493       +1     
  Lines        5845     5852       +7     
  Branches      781      782       +1     
==========================================
+ Hits         2382     2387       +5     
- Misses       3087     3089       +2     
  Partials      376      376

Impacted Files	Coverage Δ
app/angular/src/server/framework-preset-angular.js	`0% <0%> (ø)`	⬆️
app/angular/src/server/ts_config.js	`100% <100%> (ø)`	⬆️
...ngular/src/server/create-fork-ts-checker-plugin.js	`100% <100%> (ø)`

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 a6697e4...6070cb8. Read the comment docs.

kroeder · 2018-09-19T07:07:46Z

I added tsconfig.json to the angular-cli getstorybook template and updated the documentation.

How can I verify, that getstorybook works for angular-cli projects?

igor-dv

A few minor comments

We are almost there =)

igor-dv · 2018-09-20T07:17:32Z

app/angular/src/server/create-fork-ts-checker-plugin.test.js

+  require('fs').__setMockFiles(files);
+};
+
+describe('framework-preset-ts-fork-checker-plugin', () => {


create-fork-ts-checker-plugin.test

igor-dv · 2018-09-20T07:21:10Z

docs/src/pages/basics/guide-angular/index.md

@@ -73,6 +73,27 @@ That'll load stories in `../src/stories/index.ts`.

 Just like that, you can load stories from wherever you want to.

+## Storybook TypeScript configuration
+
+**Note:** You need this only if you are using Storybook `>= 4.0.0-alpha.22`.


I think you can mention that this file is created with a Storybok cli (storybook init that was getstorybook before)
Also, maybe you can mention that there is a use of ForkTsCheckerWebpackPlugin to boost the performance.

I can mention that but the whole guide is more or less a complete "manual installation" guide, isn't it?

https://storybook.js.org/basics/guide-angular/

If you want to set up Storybook manually, this is the guide for you.

Regarding ForkTsCheckerWebpackPlugin I'll add that.

igor-dv · 2018-09-20T07:28:23Z

How can I verify, that getstorybook works for angular-cli projects?

getstorybook is now called storybook init from the @storybook/cli

@Keraito, how are you testing CLI?

Keraito · 2018-09-20T13:14:28Z

@igor-dv As said on discord, atm manually on dummy projects. Still trying to come up with something else that doesn't have the same issues as the removed CLI snapshots.

@kroeder what are you exactly trying to verify with the CLI?

kroeder · 2018-09-20T13:20:58Z

@Keraito I want to check if a custom tsconfig.json is created on storybook init (only for angular cli projects) and that a fresh project with storybook init still works out of the box.

Keraito · 2018-09-20T13:34:46Z

I want to check if a custom tsconfig.json is created on storybook init (only for angular cli projects)

The easiest for now is to probably check this manually for now. Tagging #1108 so I don't forget this.

that a fresh project with storybook init still works out of the box.

You can use a smoke test for this. https://github.com/storybooks/storybook/tree/master/lib/cli/test/fixtures Add a example project here without Storybook installed. Running yarn test --cli will trigger the storybook init command, install deps and then start Storybook in it. It will give an error if the Storybook doesn't start successfully (make sure you merged the changes from #4194 through master already)

kroeder · 2018-09-20T15:39:13Z

I npm linked lib/cli and created a fresh angular project. storybook init now creates a tsconfig.json inside .storybook. I also linked my changes from @storybook/angular. Everything seems to work but I get the following error

Module build failed (from ./node_modules/postcss-loader/lib/index.js):
Syntax Error 

(2:1) Unknown word

  1 | 
> 2 | var content = require("!!../node_modules/css-loader/index.js??ref--17-1!../node_modules/@storybook/core/node_modules/postcss-loader/src/index.js??postcss!./styles.css");
    | ^
  3 | 
  4 | if(typeof content === 'string') content = [[module.id, content, '']];

 @ ./src/styles.css 2:14-325 21:1-42:3 22:19-330
 @ multi ./node_modules/@storybook/core/dist/server/config/polyfills.js ./node_modules/@storybook/core/dist/server/config/globals.js ./.storybook/config.js ./node_modules/webpack-hot-middleware/client.js?reload=true ./src/styles.css
i ｢wdm｣: Failed to compile.

I don't know if it's related to my PR - anyone knows something about this error?

If this is not related to my PR I'm done, so far. 👍

igor-dv · 2018-09-20T15:49:28Z

I think it's not related 👍

kroeder added 3 commits September 4, 2018 22:55

Integrated ForkTsCheckerWebpackPlugin

089c1cc

Fixed tsconfig path for ForkTsChecker

b7a103b

Updated test cases

5801057

kroeder requested review from alterx and igor-dv as code owners September 4, 2018 21:36

ndelangen added feature request angular performance issue labels Sep 4, 2018

igor-dv reviewed Sep 10, 2018

View reviewed changes

kroeder added 3 commits September 16, 2018 12:30

Merge branch 'master' into angular-build-time-optimization

8d7a867

Merge branch 'master' into angular-build-time-optimization

9813ab4

# Conflicts: # app/angular/src/server/wrapInitialConfig.js Created a function for ForkTsCheckerWebpackPlugin Added tests for ForkTsCheckerWebpackPlugin

Updated instantiation behavior of ForkTsCheckerWebpackPlugin function…

d5dce64

…; Updated tests

igor-dv reviewed Sep 17, 2018

View reviewed changes

app/angular/src/server/framework-preset-fork-ts-checker-plugin.js Outdated Show resolved Hide resolved

app/angular/src/server/framework-preset-fork-ts-checker-plugin.js Outdated Show resolved Hide resolved

igor-dv reviewed Sep 18, 2018

View reviewed changes

kroeder added 6 commits September 18, 2018 10:44

Merge branch 'master' into angular-build-time-optimization

b499542

reverted storybook angular-cli example start script

4b39d0d

Removed @types/jasmine and @types/jasminewd2 to resolve a type collis…

8a4eeff

…ion with @types/jest

Renamed fork-ts-checker-plugin file

ec85d10

yarn.lock

3a1ed79

Merge branch 'master' into angular-build-time-optimization

9e3af79

Added custom tsconfig.json due to the ForkTsChecker implementation

6c37e20

Added documentation

d5563ea

kroeder requested review from Hypnosphi, ndelangen, shilman and stijnkoopal as code owners September 19, 2018 06:59

Updated documentation

c43b1a8

igor-dv approved these changes Sep 20, 2018

View reviewed changes

kroeder added 4 commits September 20, 2018 14:41

Merge branch 'master' into angular-build-time-optimization

f969f3c

Updated documentation

c1bbd6e

Updated documentation

78eb562

yarn.lock

c6b9126

Fixed grammar

6070cb8

igor-dv closed this Sep 20, 2018

igor-dv reopened this Sep 20, 2018

igor-dv merged commit 71fb0da into storybookjs:master Sep 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Angular build time optimization #4118

Angular build time optimization #4118

kroeder commented Sep 4, 2018 •

edited

Loading

kroeder commented Sep 4, 2018

ndelangen commented Sep 4, 2018

igor-dv commented Sep 6, 2018

igor-dv Sep 10, 2018

igor-dv Sep 10, 2018

igor-dv Sep 11, 2018

kroeder commented Sep 10, 2018

igor-dv commented Sep 10, 2018

igor-dv commented Sep 16, 2018

kroeder commented Sep 16, 2018 •

edited

Loading

kroeder commented Sep 16, 2018

igor-dv Sep 18, 2018

kroeder commented Sep 18, 2018

codecov bot commented Sep 19, 2018 •

edited

Loading

kroeder commented Sep 19, 2018

igor-dv left a comment

igor-dv Sep 20, 2018

igor-dv Sep 20, 2018

kroeder Sep 20, 2018

igor-dv commented Sep 20, 2018

Keraito commented Sep 20, 2018

kroeder commented Sep 20, 2018 •

edited

Loading

Keraito commented Sep 20, 2018 •

edited

Loading

kroeder commented Sep 20, 2018 •

edited

Loading

igor-dv commented Sep 20, 2018

Angular build time optimization #4118

Angular build time optimization #4118

Conversation

kroeder commented Sep 4, 2018 • edited Loading

What I did

Checklist

How to test

kroeder commented Sep 4, 2018

ndelangen commented Sep 4, 2018

igor-dv commented Sep 6, 2018

igor-dv Sep 10, 2018

Choose a reason for hiding this comment

igor-dv Sep 10, 2018

Choose a reason for hiding this comment

igor-dv Sep 11, 2018

Choose a reason for hiding this comment

kroeder commented Sep 10, 2018

igor-dv commented Sep 10, 2018

igor-dv commented Sep 16, 2018

kroeder commented Sep 16, 2018 • edited Loading

kroeder commented Sep 16, 2018

igor-dv Sep 18, 2018

Choose a reason for hiding this comment

kroeder commented Sep 18, 2018

codecov bot commented Sep 19, 2018 • edited Loading

Codecov Report

kroeder commented Sep 19, 2018

igor-dv left a comment

Choose a reason for hiding this comment

igor-dv Sep 20, 2018

Choose a reason for hiding this comment

igor-dv Sep 20, 2018

Choose a reason for hiding this comment

kroeder Sep 20, 2018

Choose a reason for hiding this comment

igor-dv commented Sep 20, 2018

Keraito commented Sep 20, 2018

kroeder commented Sep 20, 2018 • edited Loading

Keraito commented Sep 20, 2018 • edited Loading

kroeder commented Sep 20, 2018 • edited Loading

igor-dv commented Sep 20, 2018

kroeder commented Sep 4, 2018 •

edited

Loading

kroeder commented Sep 16, 2018 •

edited

Loading

codecov bot commented Sep 19, 2018 •

edited

Loading

kroeder commented Sep 20, 2018 •

edited

Loading

Keraito commented Sep 20, 2018 •

edited

Loading

kroeder commented Sep 20, 2018 •

edited

Loading