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

Refactor to TypeScript #1101

Closed
Exortions opened this issue Apr 13, 2022 · 12 comments
Closed

Refactor to TypeScript #1101

Exortions opened this issue Apr 13, 2022 · 12 comments

Comments

@Exortions
Copy link

Refactoring the codebase to TypeScript in my opinion would have a few advantages:

  1. Much easier to understand how things work for new/aspiring contributors
  2. Because TypeScript compiles to minified JavaScript, the bundle size for users would be smaller, and run times would be faster.
  3. Overall, it's just a good idea as Types add a lot of functionality to JavaScript.

Of course, I understand if TypeScript isn't your thing, but if it is, I'd be happy to make a PR for at least some (if not most) of the packages.

@SBoudrias
Copy link
Owner

I'm happy to review PRs starting on the new packages. I wanted to add TS, just doesn't have the time.

@denik1981
Copy link

With a progressive strategy and a nice roadmap, this could be done.

@SBoudrias
Copy link
Owner

The first stab is launched over #1112 - there's a fair amount of work, but I explained which files to focus on there. We can do them one by one!

@Exortions
Copy link
Author

Yep, you should create a new branch for TypeScript.

@Exortions
Copy link
Author

Nevermind, looks like you already did 😅

@Exortions
Copy link
Author

@SBoudrias I'll be working on packages/core/src/lib/screen-manager.js.

@TDP17
Copy link
Contributor

TDP17 commented Jun 19, 2022

@SBoudrias I'd like to help migrate the other packages. Can I work on @inquirer/password and @inquirer/select and others if they are free as well?

I'll take @inquirer/checkbox as a reference

@SBoudrias
Copy link
Owner

SBoudrias commented Jun 20, 2022

@TDP17 that'd be great! I'll leave those to you for at least a week. Feel free to send an incomplete PR if you get stuck somewhere; that way I'll know the progress and can help take the branch over if I want to wrap this up sooner 😄

@mokkabonna
Copy link
Contributor

If I want to convert my plugin to typescript https://github.com/mokkabonna/inquirer-autocomplete-prompt is the approach here the way to go? Import from @inquirer/core and use the createprompt?

type SelectConfig = AsyncPromptConfig & {
  choices: { value: string; name?: string; description?: string; disabled?: boolean }[];
  pageSize?: number;
};

export default createPrompt<string, SelectConfig>((config, done) => {

@SBoudrias
Copy link
Owner

Hey @mokkabonna, yes that'd be the ideal. One thing to note is that afterward your custom prompt will be usable standalone (which I think overall is better.)

But it won't be register-able through inquirer.registerPrompt since I didn't wrap up writing the new core inquirer module yet. If you're curious, the WIP is in #1173, but there's still missing a lot (and I'm considering the need to add a backward compatibility layer since otherwise all custom prompt would stop working with an eventual new major release.)

@mokkabonna
Copy link
Contributor

Ok nice to know. I try to stay as aligned with what you are doing here as I can so I will then probably go down this route as well.

@SBoudrias
Copy link
Owner

I'll mark this one as complete since the new package is out and in typescript #1214

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

No branches or pull requests

5 participants