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

Update typescript definitions (BREAK CHANGES) #82

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

kamontat
Copy link

@kamontat kamontat commented Dec 2, 2018

I update the typescript definitions to support almost every feature in Enquirer.

Features

  • Prompt object for custom prompt
  • Enquirer class for instance created
  • Export Options Type for casting (in some case)
  • Separate the option for individual prompts class
  • Important Make the result able to use Auto completion

Result Auto completion

screen shot 2018-12-03 at 13 39 22

Todo

  • Have some contributors to review the code
  • Remove all Todo comments

@kamontat kamontat changed the title refactor(d.ts): Export type and refactor code Update typescript definitions (BREAK CHANGES) Dec 3, 2018
@jonschlinkert
Copy link
Member

Nice work! We're reviewing the code. I'm still learning typescript, so feedback from others would be great as well. Thanks!

@g-plane
Copy link
Member

g-plane commented Dec 11, 2018

@kamontat Good job!

Copy link
Member

@g-plane g-plane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job!
I have a question: all the methods of the Prompt have the ThisType and they point to this. Are they necessary?
Also, can you add tests? Test files are located at test/types/test.ts. Thanks.

declare namespace EnquirerStatic {
namespace Caller {
type NoParam<R> = () => R;
type OneParam<T, R = T> = (value: T) => R;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the default type of the type parameter R is T?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The R is for the return but I set as an optional since somethings the method will return as the same as input

index.d.ts Outdated
| (PromptOptions | ((this: Enquirer) => PromptOptions))[]
): Promise<T>;
declare namespace EnquirerStatic {
namespace Caller {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can merge Caller with UnionType namespace as a new namespace called TypeUtils?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes.

NumberOptions<N> {}
}

type PromptOptions<N extends string> =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enquirer allows custom prompt, not only built-in prompts.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already add support to custom prompt, but the prompt must extend BaseTypeOptions

@kamontat
Copy link
Author

I will try to solve it problem, but I have another work at least until end of the year. so...

@kamontat
Copy link
Author

kamontat commented Dec 13, 2018

@g-plane @jonschlinkert after end of this year, can you notify me again, otherwise I may forget about this project.

@g-plane
Copy link
Member

g-plane commented Jan 3, 2019

@kamontat Are you still active on this?

@kamontat
Copy link
Author

Yes, sorry for late update.

@Cyberuben
Copy link

What is the status of this PR? I'm looking for TS definitions and it seems pointless writing my own if these definitions work

@kamontat
Copy link
Author

kamontat commented May 5, 2019

@Cyberuben

This is working, but not 100%

@jpike88
Copy link

jpike88 commented May 13, 2019

Can this be made a priority? TS defintion support is the standard nowadays, wouldn't be a bad idea either to convert this project to TS

@g-plane
Copy link
Member

g-plane commented May 21, 2019

@kamontat What's the progress now? Are you still active on this?

@cdaringe
Copy link

Hey friends, what is needed to progress this forward? If there's something concrete that needs to happen, can you enumerate it? Getting some of these types corrected would be a great help

@kamontat
Copy link
Author

@cdaringe There are a lot changes since last time I working on this PR (both enquirer and Typescript).
I can continue on this if @enquirer team still needs this. I didn't use/follow enquirer anymore so I not sure what is the plan for Typescript definitions.

There are a lot of Typescript definitions open on this repository.

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

Successfully merging this pull request may close these issues.

None yet

6 participants