Added type annotation to helpers, added ts declaration files for angu… #2459

ralzinov · 2017-12-10T17:51:37Z

Issue:

Modules "storybook/angular" and "storybook/anguar/demo" has no type declaration files and
angular helpers has no proper typings. ts-loader throws many errors with "noExplicitAny" flag enabled. Issue

What I did

Added type declaration fles for "storybook/angular" and "storybook/anguar/demo" modules. Annotated angular helpers functions

How to test

Set "noExplicitAny": true in tsconfig.json

Is this testable with jest or storyshots?
no

Does this need a new example in the kitchen sink apps?
no

Does this need an update to the documentation?
no

…lar and angular/demo modules

codecov · 2017-12-11T11:22:23Z

Codecov Report

Merging #2459 into release/3.3 will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           release/3.3    #2459   +/-   ##
============================================
  Coverage        20.18%   20.18%           
============================================
  Files              398      398           
  Lines             8838     8838           
  Branches           940      952   +12     
============================================
  Hits              1784     1784           
+ Misses            6358     6324   -34     
- Partials           696      730   +34

Impacted Files	Coverage Δ
app/vue/src/server/utils.js	`0% <0%> (-53.58%)`	⬇️
...ents/stories_panel/stories_tree/tree_decorators.js	`34.37% <0%> (ø)`	⬆️
lib/ui/src/modules/ui/configs/handle_routing.js	`24.21% <0%> (ø)`	⬆️
addons/knobs/src/components/types/Color.js	`8.21% <0%> (ø)`	⬆️
addons/actions/src/lib/types/function/index.js	`33.33% <0%> (ø)`	⬆️
addons/knobs/src/components/types/Array.js	`34.14% <0%> (ø)`	⬆️
addons/actions/src/lib/util/canConfigureName.js	`30% <0%> (ø)`	⬆️
addons/actions/src/lib/decycle.js	`36.61% <0%> (ø)`	⬆️
...react-native/src/manager/components/PreviewHelp.js	`0% <0%> (ø)`	⬆️
addons/a11y/src/components/Report/Info.js	`0% <0%> (ø)`	⬆️
... and 49 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 b41f04a...0df588b. Read the comment docs.

danielduan · 2017-12-11T16:31:53Z

Not sure if this is you or on master but it looks like the tests are broken:

WARNING in /tmp/storybook/node_modules/@angular/platform-browser-dynamic/node_modules/@angular/common/esm5/common.js
122:24-38 "export 'InjectionToken' was not found in '@angular/core'
 @ /tmp/storybook/node_modules/@angular/platform-browser-dynamic/node_modules/@angular/common/esm5/common.js
 @ /tmp/storybook/node_modules/@angular/platform-browser-dynamic/esm5/platform-browser-dynamic.js
 @ /tmp/storybook/app/angular/dist/client/preview/angular/helpers.ts
 @ /tmp/storybook/app/angular/dist/client/preview/render.js
 @ /tmp/storybook/app/angular/dist/client/preview/index.js
 @ /tmp/storybook/app/angular/dist/client/index.js
 @ ./.storybook/config.js
 @ multi /tmp/storybook/app/angular/dist/server/config/polyfills.js /tmp/storybook/app/angular/dist/server/config/globals.js /tmp/storybook/app/angular/node_modules/webpack-hot-middleware/client.js?reload=true ./.storybook/config.js

WARNING in /tmp/storybook/node_modules/@angular/platform-browser-dynamic/esm5/platform-browser-dynamic.js
123:57-89 "export 'ɵCodegenComponentFactoryResolver' was not found in '@angular/core'
 @ /tmp/storybook/node_modules/@angular/platform-browser-dynamic/esm5/platform-browser-dynamic.js
 @ /tmp/storybook/app/angular/dist/client/preview/angular/helpers.ts
 @ /tmp/storybook/app/angular/dist/client/preview/render.js
 @ /tmp/storybook/app/angular/dist/client/preview/index.js
 @ /tmp/storybook/app/angular/dist/client/index.js
 @ ./.storybook/config.js
 @ multi /tmp/storybook/app/angular/dist/server/config/polyfills.js /tmp/storybook/app/angular/dist/server/config/globals.js /tmp/storybook/app/angular/node_modules/webpack-hot-middleware/client.js?reload=true ./.storybook/config.js

WARNING in /tmp/storybook/node_modules/@angular/platform-browser-dynamic/esm5/platform-browser-dynamic.js
128:45-49 "export 'ɵcmf' was not found in '@angular/core'
 @ /tmp/storybook/node_modules/@angular/platform-browser-dynamic/esm5/platform-browser-dynamic.js
 @ /tmp/storybook/app/angular/dist/client/preview/angular/helpers.ts
 @ /tmp/storybook/app/angular/dist/client/preview/render.js
 @ /tmp/storybook/app/angular/dist/client/preview/index.js
 @ /tmp/storybook/app/angular/dist/client/index.js
 @ ./.storybook/config.js
 @ multi /tmp/storybook/app/angular/dist/server/config/polyfills.js /tmp/storybook/app/angular/dist/server/config/globals.js /tmp/storybook/app/angular/node_modules/webpack-hot-middleware/client.js?reload=true ./.storybook/config.js

@ndelangen who can review these angular PRs? there's a few of these just sitting around now

ralzinov · 2017-12-12T15:38:42Z

@danielduan I think it is some npm packages conflict in parent branch: https://stackoverflow.com/questions/44656265/export-%C9%B5cmf-was-not-found-in-angular-core

Hypnosphi · 2017-12-12T22:12:10Z

FYI: test passes on upstream branch: https://circleci.com/gh/storybooks/storybook/24979

Hypnosphi · 2017-12-12T22:13:40Z

Here it fails because of errors not warnings:

ERROR in /tmp/storybook/app/angular/src/client/preview/angular/helpers.ts
(52,5): error TS2322: Type 'Timer' is not assignable to type 'number'.

ERROR in /tmp/storybook/app/angular/src/client/preview/angular/helpers.ts
(151,5): error TS90010: Type 'PlatformRef' is not assignable to type 'PlatformRef'. Two different types with this name exist, but they are unrelated.

ERROR in /tmp/storybook/app/angular/dist/client/preview/angular/helpers.ts
(52,5): error TS2322: Type 'Timer' is not assignable to type 'number'.

ERROR in /tmp/storybook/app/angular/dist/client/preview/angular/helpers.ts
(151,5): error TS90010: Type 'PlatformRef' is not assignable to type 'PlatformRef'. Two different types with this name exist, but they are unrelated.
  Property '_injector' is missing in type 'PlatformRef'.

ndelangen · 2017-12-14T17:55:27Z

app/angular/index.d.ts

@@ -0,0 +1,28 @@
+export interface IStorybookStory {


I think we also have type definitions in the @types/... npm organisation.

Should we move these here? Or add them there also?
Are these even the same type of definitions?

👍

I didn't find any typings for apps at https://microsoft.github.io/TypeSearch/, only for addons.
I think app and type definitioins for it should be in same package, at least written with typescript like angular. It's not the same typings.

ndelangen · 2017-12-16T12:56:26Z

@alterx do you want to approve and merge this?

alterx · 2017-12-20T01:02:46Z

@ndelangen lets wait until we approve and merge #2467

ndelangen · 2017-12-21T19:41:04Z

Seems like the CLI tests are breaking over this:

ERROR in /tmp/storybook/app/angular/src/client/preview/angular/helpers.ts
(139,5): error TS2345: Argument of type '(PipeTransform | IComponent | typeof AppComponent)[]' is not assignable to parameter of type '(any[] | Type<any>)[]'.
  Type 'PipeTransform | IComponent | typeof AppComponent' is not assignable to type 'any[] | Type<any>'.
    Type 'PipeTransform' is not assignable to type 'any[] | Type<any>'.
      Type 'PipeTransform' is not assignable to type 'Type<any>'.

ERROR in /tmp/storybook/app/angular/src/client/preview/angular/helpers.ts
(179,68): error TS2345: Argument of type '{ component: any; props: { message: string; stack: string; }; propsMeta: {}; }' is not assignable to parameter of type 'Data'.
  Property 'pipes' is missing in type '{ component: any; props: { message: string; stack: string; }; propsMeta: {}; }'.

ERROR in /tmp/storybook/app/angular/dist/client/preview/angular/helpers.ts
(139,5): error TS2345: Argument of type '(IComponent | PipeTransform | typeof AppComponent)[]' is not assignable to parameter of type '(any[] | Type<any>)[]'.
  Type 'IComponent | PipeTransform | typeof AppComponent' is not assignable to type 'any[] | Type<any>'.
    Type 'PipeTransform' is not assignable to type 'any[] | Type<any>'.
      Type 'PipeTransform' is not assignable to type 'Type<any>'.
        Property 'apply' is missing in type 'PipeTransform'.

ERROR in /tmp/storybook/app/angular/dist/client/preview/angular/helpers.ts
(179,68): error TS2345: Argument of type '{ component: any; props: { message: string; stack: string; }; propsMeta: {}; }' is not assignable to parameter of type 'Data'.
  Property 'pipes' is missing in type '{ component: any; props: { message: string; stack: string; }; propsMeta: {}; }'.
Child html-webpack-plugin for "index.html":
         Asset    Size  Chunks  Chunk Names
    index.html  569 kB       0  
       [0] /tmp/storybook/node_modules/html-webpack-plugin/lib/loader.js!/tmp/storybook/app/angular/dist/server/index.html.ejs 1.7 kB {0} [built]
       [1] /tmp/storybook/node_modules/lodash/lodash.js 540 kB {0} [built]
       [2] (webpack)/buildin/global.js 509 bytes {0} [built]
       [3] (webpack)/buildin/module.js 517 bytes {0} [built]
Child html-webpack-plugin for "iframe.html":
          Asset    Size  Chunks  Chunk Names
    iframe.html  568 kB       0  
       [0] /tmp/storybook/node_modules/html-webpack-plugin/lib/loader.js!/tmp/storybook/app/angular/dist/server/iframe.html.ejs 886 bytes {0} [built]
       [1] /tmp/storybook/node_modules/lodash/lodash.js 540 kB {0} [built]
       [2] (webpack)/buildin/global.js 509 bytes {0} [built]
       [3] (webpack)/buildin/module.js 517 bytes {0} [built]
error Command failed with exit code 1.

ralzinov · 2017-12-22T10:37:45Z

@ndelangen @alterx I fixed conflicts, but i think we should enable "noImplicitAny" flag in tsconfig, because without it typescript is useless, and we will get broken typings after each merge.

ndelangen · 2017-12-22T12:57:02Z

What do you think @alterx @igor-dv ?

alterx · 2017-12-22T14:35:32Z

Yeah, I agree. If we're going to have type definitions we need to enforce its use.
On a non-related note, I think we need to fix IGetStory to include the new moduleMetadata
@ralzinov

ndelangen · 2017-12-23T00:26:05Z

👏

ralzinov added 2 commits December 10, 2017 20:43

Added type annotation to helpers, added ts declaration files for angu…

5cefa7c

…lar and angular/demo modules

Merge branch 'release/3.3' into fix-angular-typings

a3a5aa7

Hypnosphi requested a review from alterx December 10, 2017 23:13

Hypnosphi added the angular label Dec 10, 2017

Merge branch 'release/3.3' into fix-angular-typings

5d09eed

danielduan assigned alterx Dec 11, 2017

Merge branch 'release/3.3' into fix-angular-typings

57f3084

ralzinov and others added 5 commits December 13, 2017 19:40

fix typing errors in tests

ce8544a

Merge branch 'release/3.3' into fix-angular-typings

c8b9016

Merge branch 'release/3.3' into fix-angular-typings

2ce7d89

Merge branch 'release/3.3' into fix-angular-typings

6a6027c

Merge branch 'release/3.3' into fix-angular-typings

804e6de

ndelangen reviewed Dec 14, 2017

View reviewed changes

Merge branch 'release/3.3' into fix-angular-typings

2cd7306

ndelangen approved these changes Dec 16, 2017

View reviewed changes

ralzinov added 2 commits December 20, 2017 21:37

Merge branch 'release/3.3' into fix-angular-typings

841f260

Merge branch 'release/3.3' into fix-angular-typings

ab75f9a

ralzinov added 2 commits December 22, 2017 12:42

Merge branch 'release/3.3' into fix-angular-typings

edaee12

fixed conflicts, added annotation to merged files

e003561

Merge branch 'release/3.3' into fix-angular-typings

4650309

ndelangen and others added 2 commits December 22, 2017 15:58

Merge branch 'release/3.3' into fix-angular-typings

2a90437

added "moduleMetadata" property to IGetStory interface at index.d.ts

0df588b

alterx approved these changes Dec 22, 2017

View reviewed changes

alterx merged commit 2e77342 into storybookjs:release/3.3 Dec 22, 2017

ralzinov deleted the fix-angular-typings branch December 22, 2017 18:59

shilman mentioned this pull request Dec 23, 2017

Angular 2 support #269

Closed

dependencies bot mentioned this pull request May 14, 2018

Update 80 dependencies from npm #3584

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added type annotation to helpers, added ts declaration files for angu… #2459

Added type annotation to helpers, added ts declaration files for angu… #2459

ralzinov commented Dec 10, 2017

codecov bot commented Dec 11, 2017 •

edited

Loading

danielduan commented Dec 11, 2017

ralzinov commented Dec 12, 2017

Hypnosphi commented Dec 12, 2017

Hypnosphi commented Dec 12, 2017

ndelangen Dec 14, 2017

ralzinov Dec 16, 2017 •

edited

Loading

ndelangen commented Dec 16, 2017

alterx commented Dec 20, 2017 •

edited

Loading

ndelangen commented Dec 21, 2017

ralzinov commented Dec 22, 2017

ndelangen commented Dec 22, 2017

alterx commented Dec 22, 2017 •

edited

Loading

ndelangen commented Dec 23, 2017

Added type annotation to helpers, added ts declaration files for angu… #2459

Added type annotation to helpers, added ts declaration files for angu… #2459

Conversation

ralzinov commented Dec 10, 2017

Issue:

What I did

How to test

codecov bot commented Dec 11, 2017 • edited Loading

Codecov Report

danielduan commented Dec 11, 2017

ralzinov commented Dec 12, 2017

Hypnosphi commented Dec 12, 2017

Hypnosphi commented Dec 12, 2017

ndelangen Dec 14, 2017

Choose a reason for hiding this comment

ralzinov Dec 16, 2017 • edited Loading

Choose a reason for hiding this comment

ndelangen commented Dec 16, 2017

alterx commented Dec 20, 2017 • edited Loading

ndelangen commented Dec 21, 2017

ralzinov commented Dec 22, 2017

ndelangen commented Dec 22, 2017

alterx commented Dec 22, 2017 • edited Loading

ndelangen commented Dec 23, 2017

codecov bot commented Dec 11, 2017 •

edited

Loading

ralzinov Dec 16, 2017 •

edited

Loading

alterx commented Dec 20, 2017 •

edited

Loading

alterx commented Dec 22, 2017 •

edited

Loading