Carbon: Discussion: Add typescript types

[paul-sachs]

Add typescript types: Typescript definition files would provide better documentation built into our editors themselves.

Detailed description

This is more of a discussion rather than an issue itself. We've started using typescript over many of our apps and I'm trying to identify what we can do to add more documentation wherever we can. I've found a library react-to-typescript-definitions that provides some ability to generate types from prop types. However, the library doesn't fit to some of our setup and we have a few exceptions that aren't supported by the tool. We would need to manually edit the output somewhat.

How do people feel about supporting type definition files? I was thinking that each component would have a type file (types.d.ts) within the directory to explain it's exports. We would need to do some processing on those files to combine in to a single types definition.

Thoughts?

[joshblack]

I know @asudoh and I had talked about this earlier, mainly the topic of whether or not introducing a type system to the library would align with our project goals/values.

I think supporting at least the definition files would be great, but ultimately would be something that we would take from the community/contributors versus something that we would create. If, or when, we adopt a type system, then would definitely be on us to produce.

[paul-sachs]

So I'm working with the definitions now, getting them to work with our project. I could submit the definition file to DefinititelyTyped or into this repo itself. I prefer types associated to the repo itself because it avoids the separate package install + versioning headaches.

[paul-sachs]

But, on the other hand, it does allow the types to evolve separately from the primary repo.

[joshblack]

@psachs21 the only concern I have if it exists in the project itself is if we make an update that breaks the type definitions, we would have to update the type definitions themselves. I don't think it'd be anything unduly burdensome, but since the project isn't setup to use TS then I would imagine changes would fly under the radar and potentially break consumers.

[paul-sachs]

@joshblack that makes sense, i'm ok with putting them into DefinitelyTyped for now. At least when I get them into a workable condition.

Thanks!

[joshblack]

Thanks so much for taking the time to propose this @psachs21 if it helps, we definitely are looking to use/leverage a type system in the future which should help a lot with this situation.

Thanks again for reaching out and starting this discussion, really appreciate all the work/effort involved in order to help improve the project 😄

[paul-sachs]

@joshblack on a related note, what is our policy on using the spread operator for props:

eg

const Form = ({ className, children, ...other }) => {
  const classNames = classnames('bx--form', className);

  return (
    <form className={classNames} {...other}>
      {' '}
      {children}{' '}
    </form>
  );
};

This behaviour makes it complicated to generate decent typings. I can inherit from the form element props, but that kind of makes documentation a little less useful (basically supports any DOM attribute).

[paul-sachs]

Guess the question is if we want to fall on the side of useful or expressive.

[joshblack]

@psachs21 my bias would be towards being as strict as possible, and avoid using spread unless appropriate. Unfortunately, it's hard to predict what a consumer would _want_ to pass through which is why I believe this pattern is currently being used so that we can be as flexible as possible.

If you have any suggestions/recommendations in this space, would love to hear them 😄

[paul-sachs]

@joshblack I looked into exactly what material-ui is doing, and it seems they err on the side of spread and use the built in types. I personally agree with you, in that I prefer restricting the available options to funnel developers to best practices. But maybe the restriction shouldn't live in the component library? I dunno, I'm flip flopping on this one.

[rohmanhm]

So @psachs21, what's the progress now?

[paul-sachs]

I made some attempts to generate types but higher priority tasks keep on
popping up so I haven't had a chance to return to this.
On Thu, Apr 12, 2018 at 10:36 PM Roman Masyhar [e-mail]
wrote:

So @psachs21 https://github.com/psachs21, what's the progress now?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/carbon-design-system/carbon-components-react/issues/580#issuecomment-381003494,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AK61gDHbLi_HE7s1aXCYsBNmFBzcszK9ks5toA9AgaJpZM4R7p6S
.

[joshblack]

Would it be helpful for us to maintain a dedicated types repo for this so people at least have a place to look? 🙂

[paul-sachs]

@joshblack Given that's it's not in condition to push to DefinitelyTyped, it would be handy to put this code somewhere so it can be worked collectively.

[stram2020]

Hi,

Do we have react.SFC / react.statelesscomponent for carbon-components-react with TypeScript?

Thanks & Regards,
Ram S

[airhorns]

@psachs21 did you end up publishing any type definitions anywhere we could at least get started with? I'd be happy to help contribute to something and eventually get it on DefinitelyTyped but I'd love to start from somewhere! Thanks :)

[Yohanna]

I'd also be interested to help contribute to this! :)

[paul-sachs]

@airhorns I haven't found time to figure out a good way to convert. Was poking at https://github.com/lyft/react-javascript-to-typescript-transform but keep getting distracted by other priorities.

[Yohanna]

@joshblack do you want to get the ball rolling by creating a place we can start committing to? I'm not sure if a new branch in the current repo would be good enough, or a whole new repo would be better?

[asudoh]

CC @emyarod

[emyarod]

yes this is something I am greatly interested in getting started with

regarding DefinitelyTyped, I wonder if providing the typings within our repo itself would be any quicker and easier for updating?

[paul-sachs]

I definitely think types located in the repo itself are much easier to manage and prevent versioning confusion.

[asudoh]

Ref:
https://github.com/IBM/carbon-elements/issues/318#issuecomment-461130865

[vpicone]

@emyarod @psachs21 The current recommendation from Microsoft/DT is to use DefinitelyTyped tooling unless the package itself is authored in TypeScript and the typings are generated using the tsc --declaration flag (which can only be used with the allowJs flag turned off).

dts-gen is a bit clunk but a great tool for generating loose types. I think that's a great place to start. Types can always be made more strict, and even unopinionated types give tremendous improvements to dev experience with autocomplete/intellisense features.

Side note: There's an easy path to removing the DefinitelyTyped types should CCR ever switch over to typescript internally wink wink

[johnnyreilly]

Hey - Definitely Typed maintainer here. I'd be happy to help getting this into DT. Raise a PR and ping me and I'll help with the merge process 👍

[vpicone]

I ported the project to TypeScript so that folks can play with writing types (for DefinitelyTyped or otherwise). It has jest/storybook/compile configured for both ts/js files.

https://github.com/vpicone/carbon-components-react/tree/ts

I'm working my way through the components alphabetically in my spare time to uncover any PropType inconsistencies.

[emyarod]

I've slowed down a bit on this effort, at least until we get tickets like https://github.com/IBM/carbon-components-react/issues/1918 ironed out for the sake of consistency, but this is a great starting point. Hoping to take care of this soon after v10 release

[austinbruch]

Is there any update on this? It looks like the next major release of carbon is a few weeks old now. I'm looking to update to the latest version in our TS application, type info would be great to have. I tried to follow the discussion above and saw the various PRs opened for _other_ carbon repos, but I didn't follow how they mapped to this repo in particular. Any pointers would be greatly appreciated. I'd also be willing to contribute type info, as we end up creating some definitions locally within our codebase to give us nice intellisense with the version of carbon that we're using.

[vpicone]

@austinbruch the typescript team recommends either authoring your library in typescript and using the generated types or to use the DefinitelyTyped repo. Given other project priorities for this quarter, a typescript migration isn’t at the top of our list. DefinitelyTyped is public by default, anyone can submit types. We’d be happy to work with you if you’d like to contribute in that way.

[austinbruch]

@vpicone Thanks for the update. I didn't see a folder in DefinitelyTyped for this library specifically (carbon-components-react), am I just missing it or would I need to get it started? Once I get the migration from 6.x to 7.x going, I'd be happy to contribute type information.

[vpicone]

@austinbruch yeah that’s correct, I did some definition work for Carbon elements but there hasn’t been much done for react due to the V7 release. The next major release is going to be about normalizing the component api and improving code quality throughout. I imagine the release following would be a good opportunity for us to contribute in a more meaningful way.

[vpicone]

I'm going to close this in favor of a more formal proposal #2279

[carbon-bot]

Hi there! :wave: If you're wondering why this issue was moved, we're currently updating our repo structure so that every package is found in the same project.

This should not have any impact for you, but we wanted to give you a heads up in case you were wondering what is going on. If you have any questions, feel free to reach out to us on Slack or contact us at: [email protected]. Thanks!

[stale[bot]]

We've marked this issue as stale because there hasn't been any activity for a couple of weeks. If there's no further activity on this issue in the next three days then we'll close it. You can keep the conversation going with just a short comment. Thanks for your contributions.

[stale[bot]]

As there's been no activity since this issue was marked as stale, we are auto-closing it.