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 definitions #5873

Closed
dan-cooke opened this issue Apr 16, 2020 · 8 comments
Closed

Typescript definitions #5873

dan-cooke opened this issue Apr 16, 2020 · 8 comments
Assignees

Comments

@dan-cooke
Copy link

dan-cooke commented Apr 16, 2020

I think it would be really cool if we could start writing (or generating) some typings for this project.

Its quite frustrating having to leave my editor everytime i need to check what props are available.
image

PropTypes are great for runtime analysis, but most tools cannot statically parse prop types. Editors like VSCode use TS defintion files to provide "intellisense" to users.

Libraries like material-ui have fully typed components, have a look at what VSCode is able to provide me for their Button component

image

But just because we are using JS does not mean we need to sacrifice static typing for our consumers - .d.ts files can be used to export type defintions for JS libraries

This is something that I would love to see implemented to improve the developer experience with carbon!

Keep up the great work!

@joshblack
Copy link
Contributor

Hi there @dan-cooke! 👋

Definitely agreed that TypeScript would be a great addition. Unfortunately, I think it's just a case of the time/effort involved has not been something we've been able to prioritize over other efforts that we've been trying to do.

Similarly, I think there is hesitation with respect to what maintenance means for types and if we would need to respect semver both for package versions and types.

I would love to hear your perspective on this. We're definitely open to this but it's been something that has been a back seat to other things that we've needed to do first.

@dan-cooke
Copy link
Author

dan-cooke commented Apr 17, 2020

@joshblack
Oh yeah, for sure! that is completely understandable, the initial time investment is certainly quite large, but its very much possible to automate most - if not all of the maintenance.

Some methods of automation:

  1. Use JSDoc, tsc can actually read JSDoc and it automatically generates your d.ts files from any JSDoc in your project. Its just a matter of finding a configuration that works for carbon. Admidetly it would be quite a lot of effort to annotate all your files with JSDoc, but mantainers will benefit from static analysis that PropTypes cannot provide.

Heres a really concicse guide on doing this

  1. PropTypes - some packages even exist to convert from PropTypes to d.ts files , first library I found was https://github.com/KnisterPeter/react-to-typescript-definitions - this would probably be the best fit for this project as every component is already annotated with PropTypes.

Separate @types package

I'm not 100% sure about this, as most libraries I've written have included type defintions with the package. However theres likely a good guide on how to do this effectively in the DefinetlyTyped repo

With CI/CD tools it may be possible to automatically build new definition files and make a PR into DefinetlyTyped

But i'm not too certain about the logistics of that.

And thats my two cents anyway...

:)

@joshblack
Copy link
Contributor

@dan-cooke great insights, thanks so much 🙏

You've definitely hit on some great wins but also pointed out some stuff that I think internally we've (or at least I) was not super confident in.

Prop Types is a great example where we would both want to author the types and still support Component.propTypes for both end-users and would obviously want to keep the two in sync somewhat. There are definitely tools to try and help out with this, but none have seem to be the silver bullet to directly address the parity that we would want.

For DefinitelyTyped, I believe that there are some community-driven efforts for carbon over on there that can be leveraged. Unfortunately we don't keep them up-to-date so there may be missing or incomplete versions, nor could we speak to the quality of the types.

Similarly, I think this is an open-ended question where if we shipped types would these types be beholden to semver in a way like our components are. e.g. removing a type would be a breaking change. If we end up having to take on semver for types then the cost of maintaining it is definitely something that we don't have the capacity for at the moment 😞

@joshblack
Copy link
Contributor

Going to close this out since it's been a while since we've heard back, let me know if you want to keep the conversation going!

@seanlaff
Copy link

seanlaff commented Feb 7, 2022

Hi @joshblack we're a team inside IBM and would like to revisit this issue.

Typescript has a been a lifesaver for us at scale. The types catch a host of different errors before any code gets pushed, and the autocompletions that IDEs provide end up being a huge timesaver with regards to developer-experience.

Given the carbon team manually writes propType definitions, I understand your hesitation to write even more definitions by hand. I like @dan-cooke's suggestion of generating typescript types from propTypes as a quick shim, but propTypes are not that expressive (i.e you can declare a prop is a function... but cant require it matches a certain signature). Thus, the typescript types generated this way would be lackluster (but surely better than nothing).

Stepping back, a better approach might be to have carbon be typescript native, increasingly the resiliency of carbon itself as well as downstream apps. Additionally, we could generate propTypes from the typescript types for non-typescript consumers (since type accuracy is not lost when going from ts->propTypes, while the reverse is not true). There are a few way to do this, such as via babel-plugin https://github.com/milesj/babel-plugin-typescript-to-proptypes.

In any case, it feels like typescript has reached a level of popularity that I think the typings deserve some thinking.

@mbarrer
Copy link
Contributor

mbarrer commented Feb 7, 2022

Im also a FED at IBM working on Spectrum Fusion and can attest to the usefulness of having well maintained typings for component libraries. At the moment we are using carbon 10, leveraging the typings provided by the DefinitelyTyped repo. While these typings satisfy a majority of the component properties, there are cases where these types are either out of date or not sufficient enough and need to be altered. For these cases we usually just create a module that overrides/extends the typings provided by DefinitelyTyped. Our team also does not have the bandwidth at the moment to go through and vet the typings for all the components and maintain them with each new version update. We can certainly update them on a case by case basis, but then the DefinitelyTyped repo would contain a mix of version typings which would likely cause problems/confusion.

Our team also has some experience using tools to generate typing files from propTypes and while they work for the majority of simple components, there starts to be some issues when dealing with function typings and complicated object typings. Usually when we use a component lib that doens't have typings such as the IBM Cloud lib we'll use a tool to generate the typing file and then go back and fix problems and tighten up the generated typings. These tools are a great start to get the majority of the types declared, but there will still be some overhead vetting the generated output and maintaining the typings.

It would be awesome if carbon could transition to TS at some point. As Sean points out this would definitely help the maintainability of carbon packages in general, and the support for going from TS to propTypes is much more accurate. I know packages such as @carbon/charts-react seem to be typescript native and supply their own d.ts files which has made consuming those charts extremely easy, I rarely have to visit the documentation to figure out certain behavior. It would be great to transition some of these other carbon packages.

@daka1510
Copy link

daka1510 commented Feb 8, 2022

Totally agree that adding type definitions to Carbon would be a great move. I worked for 2 years with Carbon within IBM. Tried using https://www.npmjs.com/package/@types/carbon-components-react but stopped because those type definitions were not always in sync with the actual code.

Similarly, I think there is hesitation with respect to what maintenance means for types and if we would need to respect semver both for package versions and types.

My take: if efforts are taken to provide type declarations, include them in the source package itself.

@joshblack
Copy link
Contributor

Hey all! 👋 Saw some activity on this thread and figured we could open up a discussion to keep this going: #10752

I tried to write what I could in that post, including next steps and where the team is at today. I also tried to highlight some of the open areas for bringing TypeScript into Carbon if anyone would like to help us out with that!

I hope it's okay if we use that discussion moving forward, let me know if there are any questions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants