-
Notifications
You must be signed in to change notification settings - Fork 81
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
chore: support typescript #117
base: master
Are you sure you want to change the base?
Conversation
Please someone merge this! |
Bumpppppp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@moshfeu
Thanks for the PR, I've reviewed it. This needs some work, please see my comments.
I'd like to help out beyond reviewing, but don't have the time right now.
{ | ||
"compilerOptions": { | ||
"types": [ | ||
"inquirer-autocomplete-prompt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this change. See the docs:
If
types
is specified, only packages listed will be included in the global scope. For instance:
Besides, "types": "index.d.ts",
in package.json
already provides the types.
export interface QuestionMap<T extends Answers = Answers> { | ||
autocomplete: AutocompleteQuestion<T>; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind the final newline 😉 It's better for the diffs.
@@ -0,0 +1,22 @@ | |||
import inquirer, { Answers } from 'inquirer'; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it is, I get this error when trying your branch:
error TS2345: Argument of type 'typeof import("./node_modules/inquirer-autocomplete-prompt/index")' is not assignable to parameter of type 'PromptConstructor'.
Type 'typeof import("./node_modules/inquirer-autocomplete-prompt/index")' provides no match for the signature 'new (question: any, readLine: Interface, answers: Answers): PromptBase'.
10 inquirer.registerPrompt(`autocomplete`, autocomplete)
This is my tsconfig.json
:
{
"extends": "@tsconfig/node12/tsconfig.json",
"compilerOptions": {
"esModuleInterop": true,
"moduleResolution": "node",
"strict": true,
"outDir": "./.transpiled",
"noUncheckedIndexedAccess": true,
"noPropertyAccessFromIndexSignature": true,
},
"include": [ "./" ],
}
It seems to me that this file is missing the actual export types, please verify and fix.
export default interface InquirerAutocompletePrompt {}
declare var inquirerAutocompletePrompt: InquirerAutocompletePrompt;
export = inquirerAutocompletePrompt;
I added that 👆 to the end of this file and the error changed to this 👇
error TS2345: Argument of type 'InquirerAutocompletePrompt' is not assignable to parameter of type 'PromptConstructor'.
Type 'InquirerAutocompletePrompt' provides no match for the signature 'new (question: any, readLine: Interface, answers: Answers): PromptBase'.
10 inquirer.registerPrompt(`autocomplete`, autocomplete)
Note how typeof import("./node_modules/inquirer-autocomplete-prompt/index")
changed to InquirerAutocompletePrompt
– that seems to confirm my suspicion.
@@ -0,0 +1,22 @@ | |||
import inquirer, { Answers } from 'inquirer'; | |||
|
|||
declare module 'inquirer' { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To extend the inquirer interfaces, this seems more appropriate to get alignment:
declare module 'inquirer' { | |
declare namespace inquirer { |
See https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/inquirer/index.d.ts#L95
Note: So far, I have not typed across packages myself, so I may be missing something. This comment is really based only on my observation in @types/inquirer
.
Hi guys, @jayeeson made pretty nice typings in here: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/inquirer-autocomplete-prompt If there is a will on both sides (@jayeeson and maintainers here) I can spend some time bringing those typings here. Already did a quick proof of concept and seems to work. The typings themselves needs a bit of love to support everything but I'll hold off for now until we have some agreement here. |
The typings are probably a little outdated but it's a small package. What's the motivation to bring the types in here, if you want TS support just add the DefinitelyTyped package |
Now I'm having second thoughts. All other inquirer plugins (+ the lib itself) have typings in DefinitelyTyped so it doesn't make sense to break this consistency and I definitely don't want to move them all :) |
If I understand the API correctly.
Also, I'm not that expert with TypeScript but this is working for me locally. If anyone else who knows typescript can review it, that will be great.
resolve #107