Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript Support #1166

Closed
1 of 2 tasks
shilman opened this issue May 30, 2017 · 21 comments
Closed
1 of 2 tasks

Typescript Support #1166

shilman opened this issue May 30, 2017 · 21 comments
Assignees
Labels
discussion maintenance User-facing maintenance tasks
Milestone

Comments

@shilman
Copy link
Member

shilman commented May 30, 2017

Why

We keep getting Typescript compatibility issues like #1154 #1159 . We need a standardized way to create and maintain Typescript definitions for all @storybook/* packages

What

@shilman shilman added this to the v3.1.0 milestone May 30, 2017
@tmeasday tmeasday added discussion maintenance User-facing maintenance tasks labels May 30, 2017
@shilman
Copy link
Member Author

shilman commented Jun 1, 2017

@joscha I took a look at @leonidaX 's typescript example

https://github.com/leonidax/storybook-ts-less-example

I got it working on my machine and successfully added a actions.d.ts to test things out, so I basically understand what's going now. It seems like every package needs to have its own .d.ts file that describes the package's public API.

Which leads me to these questions:

  1. Does typescript have any tools to automatically check whether the .d.ts file is in sync with the .js implementation? If so, we should add this to our test pass, so that we can make sure that these get updated as the packages are updated. Most of our contributors are not Typescript users, so it would be easy to forget to do this.
  2. When we update the .d.ts files is there an easy way to get these synchronized into DefinitelyTyped?

I'd love to get this squared away so that we can keep our Typescript users happy!

@joscha
Copy link
Member

joscha commented Jun 1, 2017

Does typescript have any tools to automatically check whether the .d.ts file is in sync with the .js implementation?

Unfortunately not. There are some projects that try and auto-generate TS definitions from the API of a package (see https://github.com/ConquestArrow/dtsmake and https://github.com/Microsoft/dts-gen), but I am not sure how successful that is. The second one might work, but having any everywhere is not great.

When we update the .d.ts files is there an easy way to get these synchronized into DefinitelyTyped?

I was actually going to remove them from this repo altogether. DefinitelyTped would then be the source of truth for the TS definitions of anything storybook.

@shilman
Copy link
Member Author

shilman commented Jun 1, 2017

@joscha OK, that makes sense. So is the typical flow then:

  • Person A creates a library X
  • Typescript user B creates a typing @types/X
  • A (or whoever) updates's X API, and the typings go out of sync
  • B (or whoever) discovers the problem when they upgrade X, and updates the typings?

Or is there typically some coordination done between A and B? I'm assuming for popular libraries like lodash there is some coordination, but maybe for some random library X there is not? What are best practices here?

@joscha
Copy link
Member

joscha commented Jun 1, 2017

  1. In an ideal world the library is written in TS and we don't need to worry about it at all.
  2. In a little less ideal world, the typings would be part of the library and they would be updated with every breaking change.
  3. In a little less ideal world, the typings would be in DefinitelyTyped and updated after every breaking change by the project owner after release.
  4. In a little less ideal world, the typings would be in DefinitelyTyped and updated by a random person that discovers that typings and package are out of sync.

1 is out of question I think, we tried 2 but it didn't seem to work (we have 2 with the random element of 4 currently :-)) . I think storybook is currently in 4, but we can aim for 3. At least then we have lots of people being able to work on the types without any load on the core maintainers.

@shilman
Copy link
Member Author

shilman commented Jun 1, 2017

@joscha thanks for the clarification and sorry for the newbie questions. Are you proposing that Storybook adopt option 3 then? And will you take point for that (at least in the short term)?

@joscha
Copy link
Member

joscha commented Jun 1, 2017

@shilman I think 3 would be good. It will definitely work better than the mixture out of 2 and 4 we currently have. Short term it can be me - the nice thing about DefinitelyTyped is, that usually it works out by itself over a while, as new authors join and the bot for reviewing the pull requests selects one of any users that have previously contributed on a given package.

@shilman
Copy link
Member Author

shilman commented Jun 1, 2017

@joscha That sounds perfect. When you remove the .d.ts from the repo, please tag this issue in the PR! Many thanks for driving this, I think you'll make a bunch of people very happy (including me)! 🏆

@tmeasday
Copy link
Member

tmeasday commented Jun 1, 2017

I wonder if we need to have some kind of process around changes to the API surface. It could be tricky to enforce it for all addons that are part of the main repo though.

@joscha
Copy link
Member

joscha commented Jun 1, 2017

@tmeasday wouldn't all API changes be at least minor or major version bumps?

@tmeasday
Copy link
Member

tmeasday commented Jun 1, 2017

You'd hope so! I'm not sure if there's a lot of religion about it right now though. Factoring in all the addons the surface area is pretty large.

@joscha
Copy link
Member

joscha commented Jun 5, 2017

First of a few PRs to be opened: DefinitelyTyped/DefinitelyTyped#16970 - let's see what the DefinitelyTyped owners say.

@joscha
Copy link
Member

joscha commented Jun 22, 2017

Knobs addon: DefinitelyTyped/DefinitelyTyped#17399

@joscha
Copy link
Member

joscha commented Jun 22, 2017

Links addon: DefinitelyTyped/DefinitelyTyped#17406

@joscha
Copy link
Member

joscha commented Jun 22, 2017

Options addon: DefinitelyTyped/DefinitelyTyped#17408

@joscha
Copy link
Member

joscha commented Jun 22, 2017

Notes addon: DefinitelyTyped/DefinitelyTyped#17409

@joscha joscha self-assigned this Jun 23, 2017
@joscha
Copy link
Member

joscha commented Jun 23, 2017

Release will happen with 3.2

@joscha joscha closed this as completed Jun 23, 2017
@shilman
Copy link
Member Author

shilman commented Jun 24, 2017

Thanks much @joscha ! Fingers crossed this is a big step forward for Typescript users of Storybook!

@chinchang
Copy link

Is @types/storybook/vue coming in near future?

@joscha
Copy link
Member

joscha commented Jan 5, 2018

@chinchang I don't think anyone is working on these types actively at the moment, but the types setup should be fairly similar to the react ones - maybe you can open a PR against definitelytyped and we can take it from there?

@chinchang
Copy link

A colleague of mine (@pntgupta) opened a pull request for the same -> DefinitelyTyped/DefinitelyTyped#23048

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion maintenance User-facing maintenance tasks
Projects
None yet
Development

No branches or pull requests

4 participants