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

feat: add TypeScript support #1359

Closed
bcoe opened this issue Jun 10, 2019 · 10 comments
Closed

feat: add TypeScript support #1359

bcoe opened this issue Jun 10, 2019 · 10 comments

Comments

@bcoe
Copy link
Member

bcoe commented Jun 10, 2019

I would love for yargs to ship with TypeScript support.

Sindre has been shipping definitions with all of his modules, which I think is a great pattern to follow:

https://github.com/chalk/chalk/blob/master/index.d.ts

@magicdawn
Copy link

magicdawn commented Jun 10, 2019

Take this first
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/yargs/index.d.ts

And a mistake I found: choice missing number

declare namespace yargs {
  type Choices = ReadonlyArray<string | true | undefined | number>
}

@mleguen
Copy link
Member

mleguen commented Jul 3, 2019

The main problem with typescript support is not to add the type definitions, but to keep them up to date with the javascript code, and to have tests to check they are.

I see, from the example @bcoe gave above, that chalk is using tsd, but I understand this tool only tests the type definitions, not their adequation with the javascript code.

So, in my opinion, our options are as follow:

  1. keeping things as they are, with type definitions in @types/yargs:
    • cons: type definitions are outdated
    • pros: we are not responsible for the type definitions to be outdated
  2. adding type definitions to yargs:
    • cons:
      • type definitions would not be up to date, as no test would break when they would not
      • we would be responsible for the type definitions being outdated
  3. adding type definitions and specific tests to yargs:
    • cons:
      • all tests would have to be written twice (javascript and typescript)
      • most contributions would require typescript skills
    • pros:
      • type definitions would be up to date, as otherwise specific tests would break
  4. adding type definitions and porting yarg's tests to typescript:
    • cons:
      • most contributions would require typescript skills
    • pros:
      • type definitions would be up to date, as otherwise yarg's tests would break
  5. porting the whole project to typescript:
    • cons:
      • more work now
      • all contributions would require typescript skills
    • pros:
      • less work in the future: type definitions would be autogenerated and always up to date
      • the project would gain much from being written in typescript

@bcoe
Copy link
Member Author

bcoe commented Jul 14, 2019

@magicdawn, @mleguen, I'm looping in @nikolay-borzov who appears to have done a lot of the work on @types/yargs and @types/yargs-parser.

I'm wondering if our best bet would be to keep using @types/yargs/@types/yargs-parser (as I've heard the implementation is quite good), but to work with @nikolay-borzov to add a TypeScript section to yargs' documentation.

I'm fairly new to TypeScript myself, so it would be a great learning experience for me to help pull this document together.

@nikolay-borzov
Copy link

I can't imagine what should be said about TypeScript in yargs' documentation. TypeScript definition file itself is a part of the documentation.
I think it's better to keep using @types/ definitions. This way anyone can fix a definition and change can be reviewed by a person who knows TypeScript.

@mleguen
Copy link
Member

mleguen commented Jul 14, 2019 via email

@bcoe
Copy link
Member Author

bcoe commented Jul 14, 2019

@mleguen @nikolay-borzov this was my thinking, for someone who's a relative TypeScript laymen (which is a lot of folks).

  1. demonstrating that one should also run npm i @types/yargs --save-dev; is useful.
  2. demonstrating a basic example of using types in a semi-complex way, e.g., a tiny command-line app that has commands, and a couple other common features that folks use.

@nikolay-borzov
Copy link

Well, an example of using types looks exactly like one w/o types https://github.com/danvk/source-map-explorer/blob/master/src/cli.ts#L27

@bcoe
Copy link
Member Author

bcoe commented Jul 15, 2019

Nuances like the fact that you've defined an interface for the argv response:

/** Parsed CLI arguments */
interface Arguments {
  _: string[];
  json?: string;
  tsv?: string;
  html?: string;
  onlyMapped?: boolean;
  noRoot?: boolean;
  replace?: string[];
  with?: string[];
}

And that your project will now have the @types/yargs dependency, I think are worth pointing out; we don't have to add a large document.

It also helps direct people who finding any bugs with types towards DefinitelyTyped.

@nikolay-borzov
Copy link

Okay, I'll think of something and submit a PR in a week.

Just to clarify my contribution to @types/yargs was minimal - I only added JSDocs and added new changes from v13.

@bcoe bcoe closed this as completed Jul 29, 2019
@bcoe
Copy link
Member Author

bcoe commented Jul 29, 2019

see: #1381

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

4 participants