Storybook: TypeScript migration organization

Created on 18 Dec 2018  路  40Comments  路  Source: storybookjs/storybook

What is this?

I want to use this issue for organizing the TypeScript migration process.

What is going to be migrated

I started with addon-notes and noticed it depends on channels and addons so I migrated them as well.
I'm not sure if we should migrate every lib but at least those that are used in addons or provide a public api that is used by other apps.

Let's discuss this if someone has other thoughts on this!

General

| Package | Task owner | PR | Status |
|-----------------------------------------------------------------------------------|------------|------------------------------------------------------------|-------------|
| babel-typescript | @kroeder | #5109 | MERGED |

Apps

| Package | Task owner | PR | Status |
|--------------------------------|-------------|----------------------------------------------------|-------------|
| @storybook/angular | @kroeder | https://github.com/storybooks/storybook/pull/6570 | MERGED |
| @storybook/ember | @aromanarguello | https://github.com/storybookjs/storybook/pull/9020 | MERGED |
| @storybook/html | @emilio-martinez | https://github.com/storybookjs/storybook/pull/7338 | MERGED |
| @storybook/marko | | | |
| @storybook/mithril | @hcz | https://github.com/storybookjs/storybook/pull/8320 | MERGED |
| @storybook/polymer | @kroeder | #8102 | MERGED |
| @storybook/preact | @lonyele | #7527 | MERGED |
| @storybook/rax | | | |
| @storybook/react-native-server | | | |
| @storybook/react-native | @benoitdion | #6448 | MERGED |
| @storybook/react | @kroeder | https://github.com/storybookjs/storybook/pull/7054 | MERGED |
| @storybook/riot | | | |
| @storybook/svelte | @aromanarguello | https://github.com/storybookjs/storybook/pull/8770 | MERGED |
| @storybook/vue | @ndelangen | #7578 | MERGED |

Libs

| Package | Task owner | PR | Status |
|-----------------------------------------------------------------------------------|------------|------------------------------------------------------------|-------------|
| @storybook/channels | @kroeder | #4977 | MERGED |
| @storybook/channels-websocket | @kroeder | #5046 | MERGED |
| @storybook/channels-postmessage | @dandean | #5154 | MERGED |
| @storybook/addons | @kroeder | #5018 | MERGED |
| @storybook/core-events | @dandean | #5140 | MERGED |
| @storybook/node-logger | @dandean | #5153 | MERGED |
| @storybook/client-logger | @dandean | #5151 | MERGED |
| @storybook/components | @gaetanmaisse | #6095 #6500 | MERGED |
| @storybook/api | @ndelangen | #5402 | MERGED |
| @storybook/core | @gaetanmaisse | https://github.com/storybookjs/storybook/pull/12839 | MERGED |
| @storybook/ui | @ndelangen | https://github.com/storybookjs/storybook/pull/9791 | MERGED |
| @storybook/cli | @gaetanmaisse | https://github.com/storybookjs/storybook/pull/10802 | MERGED |

Addons

| Package | Task owner | PR | Status |
|-----------------------------------------------------------------------------------|------------|------------------------------------------------------------|-------------|
| @storybook/addon-a11y | @gaetanmaisse | #5738 | MERGED |
| @storybook/addon-actions | @gaetanmaisse | #5459 | MERGED |
| @storybook/addon-backgrounds | @gaetanmaisse | #5535 | MERGED |
| @storybook/addon-centered | @gaetanmaisse | #6772 | MERGED |
| @storybook/addon-cssresources | @gaetanmaisse | #5380 | MERGED |
| @storybook/addon-events | @lonyele | #7190 | MERGED |
| @storybook/addon-google-analytics | @gaetanmaisse | #5307 | MERGED |
| @storybook/addon-graphql | @lonyele | #6935 | MERGED |
| @storybook/addon-info | @gaetanmaisse | | Replaced by addons-docs |
| @storybook/addon-jest | @sairus2k | #6403 | MERGED |
| @storybook/addon-knobs | @emilio-martinez | #7180 | MERGED |
| @storybook/addon-links | @sairus2k | #6246 | MERGED |
| @storybook/addon-notes | @kroeder | #4758 | MERGED |
| @storybook/addon-ondevice-backgrounds | @lonyele | #7736 | MERGED |
| @storybook/addon-ondevice-knobs | @wuweiweiwu | | WIP |
| @storybook/addon-ondevice-notes | @lonyele | #7737 | MERGED |
| @storybook/addon-options | @kroeder | https://github.com/storybooks/storybook/pull/6428 | MERGED |
| @storybook/addon-storyshots | @gaetanmaisse | https://github.com/storybooks/storybook/pull/7674 | MERGED |
| @storybook/addon-storysource | will be done in v2 | | PAUSED |
| @storybook/addon-viewport | @gaetanmaisse | #7177 | MERGED |

Deletion of DefinitelyTyped packages

As sources will now be written in TS storybook packages will export their own types definitions so we will be allowed to delete types from DefinitelyTyped (i.e. @types/storybook-XXX).

鈿狅笍 Be sure that addon/lib was merged and publically available with an official release of Storybook before removing any types from DefinitelyTyped.

| Package | Task owner | PR | Status |
|-----------------------------------------------------------------------------------|------------|------------------------------------------------------------|-------------|
| storybook__addon-notes | @gaetanmaisse | #34111 | MERGED |
| storybook__addon-a11y | @gaetanmaisse | #37358 | MERGED |
|storybook__addon-actions | @MichaelDeBoey | #38447 | MERGED |
|storybook__addon-backgrounds | @MichaelDeBoey | #38447 | MERGED |
|storybook__addon-centered | @MichaelDeBoey | #38447 | MERGED |
|storybook__addon-info | - | - | - |
|storybook__addon-jest | @MichaelDeBoey | #38447 | MERGED |
|storybook__addon-knobs | @MichaelDeBoey | #38447 | MERGED |
|storybook__addon-links | @MichaelDeBoey | #38447 | MERGED |
|storybook__addon-options |@MichaelDeBoey | #38447 | MERGED |
|storybook__addon-storyshots | @gaetanmaisse | #41583 | MERGED |
|storybook__addon-viewport | @MichaelDeBoey | #38447 | MERGED |
|storybook__addons | @MichaelDeBoey | #38447 | MERGED |
|storybook__channels | @MichaelDeBoey | #38447 | MERGED |
|storybook__preact | @MichaelDeBoey | #38447 | MERGED |
|storybook__react | @MichaelDeBoey | #39365 | MERGED |
|storybook__react-native | @MichaelDeBoey | #38447 | MERGED |
|storybook__vue | @MichaelDeBoey | #38447 | MERGED |

Activate TS strict mode

Why use TS strict mode?

  • strict mode is included in the default tsconfig.json generated by tsc CLI
  • Stricter rules help to find bugs faster than ever, for instance I found and fix #9179 by using some of the rules I activated in tsconfig.json
  • Some of our users are using TS in strict mode and are expecting that the types we are exposing are working in strict mode (e.g. #8233)
  • noUnusedLocals put in light the fact that some variables were not used for months but still there and still requiring maintenance.

Example on a11y addon: https://github.com/storybookjs/storybook/pull/9180/files

Initial steps for every migration

  1. Copy tsconfig.json from e.g. addon-notes into the root of the package
  2. Edit package.json of the package (TODO* We need to figure out how to handle tree shaking)
    2.1 Remove jsnext:main
    2.2 Add "types": "dist/%my-entry-point%.d.ts. If you have multiple files that export things, see: "My package has multiple files with exports"
  3. Rename
    3.1 .js files to .ts
    3.2 files that use jsx to .tsx

FAQ

How do I compile TypeScript?

Storybook already compiles TypeScript by using yarn dev.
I suggest using yarn dev:ts because you don't want to compile everything but TypeScript right now.

Could not find declaration file for module '...'

This means a package you are importing does not ship declaration files (types) for TypeScript.
This message is caused by noImplicitAny: true https://basarat.gitbooks.io/typescript/docs/options/noImplicitAny.html

image

Example:

import { WebSocket } from 'global';

For non-storybook packages

Install additional devDeps from @types, if available: @types/package-name
If there's no @types package, see "Write your own declaration"

For storybook packages

If a certain package is not in the list above, please add it.
See "Write your own declaration"

Write your own declaration

Add a typings.d.ts file to your src directory.
At least declare a module for a package that does not contain declarations.

// This makes anything that is importet from 'global' "any" so you don't have any type hints or checks
declare module 'global';

You can provide additional information. Depends on how important and how often a certain package is used.

See https://www.typescriptlang.org/docs/handbook/declaration-files/introduction.html
Examples: https://www.typescriptlang.org/docs/handbook/declaration-files/templates.html

My package has multiple files with exports

If your package has exports from multiple files you need to create one file that exports all your public api things.

Example:

// file a
export { someFunction, PropertyInterface }

// file b
export { somethingElse, MoreToExport }
  1. Create a public_api.ts with
export * from "file-a.ts"
export * from "file-b.ts"
  1. Edit package.json
    2.1 Set
  "main": "dist/public_api.js",
  "types": "dist/public_api.d.ts",

My packages has a default export but it can't be imported

This likely means that you use a public_api.ts as mentioned above and another file in your package has a default export.

In @storybook/addons it is solved this way

// index.ts
export const addons = getAddonsStore();

// public_api.ts
import { addons } from '.';
export default addons;

One package can have only one default export and it seems to be that it has to be in your main file of your package.

in progress maintenance typescript

Most helpful comment

DefinitelyTyped/DefinitelyTyped#40475 is merged, which means @storybook/react is deleted too 馃檪馃帀

All 40 comments

Looks promising! 馃槃
Can I help on something? Maybe try to migrate an addon to TS after https://github.com/storybooks/storybook/pull/5018 and https://github.com/storybooks/storybook/pull/4977 will be merged?

Of course! I hope to get them finished by tomorrow. Just pick whatever addon you want and open a PR 馃憤

If you want to start right now you also can just migrate the updates into your branch later on

@kroeder you can update your post to set addon-google-analytics and addon-cssresources as migrated.

@gaetanmaisse done!

i can offer to do the @storybook/vue migration once the core package is done

@ndelangen regarding @storybook/core... I had in mind that you don't want to migrate it but I might be wrong. What's your current opinion on this?

Also for the record: I had some conflicts between eslint and tslint (I heard that from @ndelangen as well) I will try to use https://github.com/typescript-eslint/typescript-eslint in a branch. Best case scenario: We get rid of the root tslint.json and also have only one configuration for linting as well as only one build system (babel) f眉r typescript (except angular that needs to be compiled with tsc)

FYI, I started to work on TS migration of @storybook/components as it used in almost every addon. Hope a PR soon 馃槈

Here's a list of NPM download counts to help prioritize for the 5.1 release. I'd suggest we try to finish everything above @storybook/angular?

package | count
--- | ---
@storybook/components | 7827790
@storybook/addons | 7462926
@storybook/channels | 7325019
@storybook/client-logger | 6555723
@storybook/ui | 6213746
@storybook/node-logger | 6193581
@storybook/channel-postmessage | 6178465
@storybook/core | 5883110
@storybook/react | 5553373
@storybook/addon-actions | 5513319
@storybook/core-events | 5322609
@storybook/addon-links | 4140679
@storybook/addon-knobs | 3566819
@storybook/addon-options | 2381731
@storybook/addon-info | 2107128
@storybook/addon-viewport | 1345186
@storybook/theming | 1245448
@storybook/router | 1215311
@storybook/addon-storyshots | 1126958
@storybook/addon-storysource | 1114802
@storybook/client-api | 1062469
@storybook/cli | 870218
@storybook/codemod | 769390
@storybook/addon-notes | 743171
@storybook/addon-centered | 623646
@storybook/addon-a11y | 514104
@storybook/addon-backgrounds | 432852
@storybook/channel-websocket | 358800
@storybook/vue | 354620
@storybook/react-native | 331146
@storybook/addon-jest | 167282
@storybook/angular | 122801
@storybook/addon-storyshots-puppeteer | 96929
@storybook/addon-ondevice-knobs | 63792
@storybook/html | 49620
@storybook/addon-ondevice-notes | 32850
@storybook/addon-events | 25584
@storybook/polymer | 18380
@storybook/addon-cssresources | 14089
@storybook/addon-graphql | 10379
@storybook/marko | 9901
@storybook/addon-ondevice-backgrounds | 6471
@storybook/api | 5874
@storybook/ember | 2809
@storybook/preact | 2709
@storybook/addon-google-analytics | 2589
@storybook/riot | 2235
@storybook/mithril | 2230
@storybook/svelte | 1851
@storybook/react-native-server | 416

I believe we later might want to come back to add more powerful type annotation with generic or conditional type into @storybook/addon. The MakeDecoratorResult type is just an arbitrary function and may not really have typed something since it escaped by any...

Reason behind it is to provide intellisense into IDE. The parameter __here__ in storiesOf().add(, __here__) is highly associated with activated addons in the user side, and we need to find proper type casting here to improve DX. That is the whole point to add type annotations. But I think there are some overheads due to how story wrapper (decorator) is used... This I don't know and may worth to investigate.

Yay!! I just released https://github.com/storybooks/storybook/releases/tag/v5.1.0-alpha.21 containing PR #5109 that references this issue. Upgrade today to try it out!

Because it's a pre-release you can find it on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman can we deactivate github-actions for certain issues? I just edited my post (btw. I started working on app/angular) and github-actions immediately is pinging people

@kroeder that's what it's for :) how would you envision disabling it?

@shilman, @kroeder: I'd like to migrate @storybook/ui into TypeScript, can I do it?! I would also do some small refractor to make the code more maintainable and perhaps kill some class component by hooked functional component.

@kroeder I set it up so that when an issue/PR is assigned to somebody, it won't ping

@leoyli go for it!

Can I help with something on here? I was going to work on app/react, but I see that has already started

@Jessica-Koch Would you be interested migrating app/vue? Or perhaps @kroeder would like to trade, so you can take app/react?

One that is missing on the list is lib/client-api could be perfect for you @Jessica-Koch ?

@ndelangen i'll take a stab at the lib/client-api

If any @types packages are missing, e.g., @types/react-color, is there a process to add those?

Incidentally, I can take on addon-knobs, if nobody's on it.

7180 ready for review. Rebased on top of next@HEAD at the time of this writing.

Thanks @emilio-martinez !

once https://github.com/storybookjs/storybook/pull/7323 is merged, i will start the vue migration.

edit: could we pin this issue? i am always searching for it -.-

@shilman could you ignore this specific PR in your "Jeehaa this got fixed" script?

@ndelangen i'd recommend not mentioning this issue in the first line of the PR. only the first line is considered, so even if you add the reference on the second line you'd be fine.

Don't mentioning it means it's harder to keep track.

@kroeder Please re-read my comment.

@kroeder I can pick up the following!

@storybook/addon-ondevice-backgrounds
@storybook/addon-ondevice-knobs
@storybook/addon-ondevice-notes

I created DefinitelyTyped/DefinitelyTyped#38447 to delete the typings of the following packages:

  • @storybook/addon-actions
  • @storybook/addon-backgrounds
  • @storybook/addon-centered
  • @storybook/addon-jest
  • @storybook/addon-knobs
  • @storybook/addon-links
  • @storybook/addon-options
  • @storybook/addon-viewport
  • @storybook/addons
  • @storybook/channels
  • @storybook/html
  • @storybook/preact
  • @storybook/react
  • @storybook/react-native
  • @storybook/vue

Awesome @MichaelDeBoey that is such a huge help!

DefinitelyTyped/DefinitelyTyped#38447 is merged, which means all of the above (except the @storybook/react one) typings were deleted.

@storybook/react typing still needs to be deleted, 'cause it was causing errors.

I've created a new PR (DefinitelyTyped/DefinitelyTyped#38961) to remove the @storybook/react typing.
Hopefully somebody from the @storybookjs team can help me figure out what's causing the error and help me getting the PR merged.

Yowza!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.0-alpha.23 containing PR #8320 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

Zoinks!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.0-alpha.44 containing PR #8770 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

DefinitelyTyped/DefinitelyTyped#40475 is merged, which means @storybook/react is deleted too 馃檪馃帀

@MichaelDeBoey You're a hero!

@ndelangen I just love to do some OSS work now and then 馃槈

Great work @MichaelDeBoey @gaetanmaisse 馃檶 馃檶 馃檶

Ooh-la-la!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-beta.9 containing PR #10802 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

We've just released zero-config typescript support in 6.0-beta. Please upgrade and test it!

Thanks for your help and support getting this stable for release!

Yee-haw!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.1.0-beta.2 containing PR #12839 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

Was this page helpful?
0 / 5 - 0 ratings