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

fix: update type definitions #261

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

fix: update type definitions #261

wants to merge 103 commits into from

Conversation

unional
Copy link

@unional unional commented Feb 9, 2020

Here is the new type definition.

I have moved the test files to a different folder and keep the existing JS tests

The prompt function support discriminated unions. It means depending on the type, you will get different code completion.

The actual Prompt and their sub-classes have their own types and separate from those used in the prompt function because I'm not very clear on what property, method, and options these classes should expose for customization and for creating custom prompts. So separating them allow them to be evolved and expend separately.

Let me know what do you think. 🍺

unional added 26 commits May 9, 2020 14:59
So that they can be used when declaring custom types
or else cannot support mix Question + Fn.

also restored original typescript test
The skipped test could be a bug, but without a clean way to test the code,
it is more problematic to fix it then not.
@bitjson
Copy link

bitjson commented Aug 4, 2020

Hi @unional – thanks for this PR! I'm noticing a lot of inaccuracies in the current type definitions (I'm having to use // @ts-expect-error in a lot of places), and this PR seems to fix pretty much everything.

Are you still working on this? How can I help get this merged?

@unional
Copy link
Author

unional commented Aug 6, 2020

Hi @unional – thanks for this PR! I'm noticing a lot of inaccuracies in the current type definitions (I'm having to use // @ts-expect-error in a lot of places), and this PR seems to fix pretty much everything.

Are you still working on this? How can I help get this merged?

Thanks @bitjson
The PR was just waiting to be review and merged. So I was not working on it.
Good to see that you are picking it up and merge with the latest code on your PR.

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

Successfully merging this pull request may close these issues.

None yet

8 participants