search

storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.22k stars 9.26k forks source link

TypeScript migration organization #5030

Closed kroeder closed 3 years ago

kroeder commented 5 years ago

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 @gaetanmaisse https://github.com/storybookjs/storybook/pull/13449 WIP
@storybook/marionette @gaetanmaisse https://github.com/storybookjs/storybook/pull/13448 WIP
@storybook/mithril @hcz https://github.com/storybookjs/storybook/pull/8320 MERGED
@storybook/polymer @kroeder #8102 MERGED
@storybook/preact @lonyele #7527 MERGED
@storybook/rax @gaetanmaisse https://github.com/storybookjs/storybook/pull/13450 WIP
@storybook/react-native-server OUTDATED
@storybook/react-native @benoitdion #6448 MERGED
@storybook/react @kroeder https://github.com/storybookjs/storybook/pull/7054 MERGED
@storybook/riot @gaetanmaisse https://github.com/storybookjs/storybook/pull/13447 WIP
@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?

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"

  2. 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.

gaetanmaisse commented 5 years ago

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?

kroeder commented 5 years ago

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

gaetanmaisse commented 5 years ago

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

kroeder commented 5 years ago

@gaetanmaisse done!

backbone87 commented 5 years ago

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

kroeder commented 5 years ago

@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)

gaetanmaisse commented 5 years ago

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

shilman commented 5 years ago

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
leoyli commented 5 years ago

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.

shilman commented 5 years ago

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.

kroeder commented 5 years ago

@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

shilman commented 5 years ago

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

leoyli commented 5 years ago

@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.

shilman commented 5 years ago

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

shilman commented 5 years ago

@leoyli go for it!

Jessica-Koch commented 5 years ago

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

ndelangen commented 5 years ago

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

ndelangen commented 5 years ago

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

Jessica-Koch commented 5 years ago

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

emilio-martinez commented 5 years ago

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.

emilio-martinez commented 5 years ago

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

ndelangen commented 5 years ago

Thanks @emilio-martinez !

backbone87 commented 5 years ago

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 -.-

ndelangen commented 5 years ago

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

shilman commented 5 years ago

@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.

kroeder commented 5 years ago

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

shilman commented 5 years ago

@kroeder Please re-read my comment.

wuweiweiwu commented 5 years ago

@kroeder I can pick up the following!


@storybook/addon-ondevice-backgrounds
@storybook/addon-ondevice-knobs
@storybook/addon-ondevice-notes
MichaelDeBoey commented 5 years ago

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

ndelangen commented 5 years ago

Awesome @MichaelDeBoey that is such a huge help!

MichaelDeBoey commented 5 years ago

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.

shilman commented 4 years ago

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.

shilman commented 4 years ago

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.

MichaelDeBoey commented 4 years ago

DefinitelyTyped/DefinitelyTyped#40475 is merged, which means @storybook/react is deleted too 🙂🎉

ndelangen commented 4 years ago

@MichaelDeBoey You're a hero!

MichaelDeBoey commented 4 years ago

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

shilman commented 4 years ago

Great work @MichaelDeBoey @gaetanmaisse 🙌 🙌 🙌

shilman commented 4 years ago

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.

shilman commented 4 years ago

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!

shilman commented 3 years ago

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.

gaetanmaisse commented 3 years ago

🥳 It's over! It's done.

🙇🏻 Thanks to all of you @aromanarguello @emilio-martinez @hcz @lonyele @ndelangen @dandean @sairus2k @wuweiweiwu @MichaelDeBoey! You're all part of this!

❤️ And a special big Thank You to @kroeder for having started and led this at the begginning and help me to make my first contributions to Storybook a few years ago.

🎉 🎆

shilman commented 3 years ago

Great Caesar's ghost!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.2.0-alpha.14 containing PR #13447 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease