refactor(create-app): moves to prompts

[Olyno]

Description

This pull request fixes the duplicate selection issue, and replaces enquirer with prompts as CLI tool.

Additional context

The first thing I tried was to fix the selection issue:

But after a long look in the code of the create-app without being capable to fix the bug, I decided to refactor with a (in my opinion) better CLI tool which is prompts.

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[patak-dev]

Hey @Olyno! Thanks for the PR!

Could you break the PR in smaller changes using different PRs for each?
This one is changing several things at the same time, so it would be difficult to review it and it may make git history less friendly to review later.

I think that changing from enquirer to prompts should be the first PR, to solve the bug that you found (good to make the callback to async changes also). Looks like prompts is more active, and it is 14.5kb vs 21kb (not much for node, but always good to reduce size). I think the best is to avoid a big refactoring though, so it is easier to merge the bug fix, and you can send another refactor PR later.

We could discuss in other PRs or issues about further changes. For example, I don't think that moving the templates in a separate folder is needed at this point. I also have doubts about replacing 'framework' with 'template', I don't think that framework is confusing in this context.

Thanks again!

[Olyno]

Hi @patak-js, thanks you for your feedback!

I personally don't see how I can reduce this pull request into several for the simple reason that prompts to a certain logic that enquirer does not have and vice versa. So I don't think it is possible even though I would have liked to.

Enquirer allows you to create dialogues on the fly, while prompts is more adapted to make all the dialogues in a chain.

Also, should I go back to the term "framework" and put it back?

[patak-dev]

Yes, better to revert to "framework" IMO. And it is ok to refactor what is needed to use prompts, that could be the first PR.

What I think you shouldn't include is renaming templates from template-xxx to templates/template-xxx, and renaming index.js to src/index.js, just leave the file structure as is so the discussion can focus on one change at a time ( in this case enquirer -> prompts)

[Olyno]

I did the revert for the "framework" and the file structure. It seems to have something wrong with the .gitignore file which includes 2 .vscode dirs from templates (template-svelte and template-svelte-ts).

[patak-dev]

Sorry @Olyno, it is still quite hard to review this PR because there are a lot of changes that are unrelated to the migration from enquirer to prompts. For example, moving the utility functions to the top instead of leaving them at the bottom of the file. Looks like there was an issue with the linter that let pass different linting rules (use of ; for example). I think the best would be to start from scratch and only perform the minimal changes needed to move to the new dependency, so we can focus on that. Please check in the generated diff that only the lines related to this move are touched.

[Olyno]

Hey @patak-js sorry for the delay. I reworked the code as you requested, making as few changes as possible. I'll do some more pull requests later to improve the other parts 😃

[antfu]

Can you explain a bit when would the duplicate selection issue appears? It works fine on my side.

And if so, it's more likely a bug of enquirer where it might be better fixed in the upstream. To me, it's not worth to switching tools because of an edge case bug, otherwise we are likely to have duplicated work of switching a to b, then another switching b to c.

[Shinigami92]

Yeah... I am with @antfu in this point

Also why prompts?

According to following statistics, we could directly move to inquirer 🤔
IMO the gap between enquirer vs prompts is not worth it to switch to prompts

https://www.npmtrends.com/enquirer-vs-inquirer-vs-prompts

[Olyno]

Can you explain a bit when would the duplicate selection issue appears? It works fine on my side.

It works fine for me too on Windows, but does not work as expected on my Linux (see screen above)

And if so, it's more likely a bug of enquirer where it might be better fixed in the upstream. To me, it's not worth to switching tools because of an edge case bug, otherwise we are likely to have duplicated work of switching a to b, then another switching b to c.

Well I'm totally agree with you, and I would like to know how to fix it, but it doesn't seem possible to fix it directly here, it's probably an internal issue of enquirer. That's the reason why I did the choice to change the dependency directly to have one which have more chances to work on all OS.

Also why prompts?

If everything should be related to stats, then I guess WindiCSS should not be used since TailwindCSS is much more used, same for Svelte in front of React and much more...
If I chose prompts as a replacement, it's not about replacing with the first tool that comes along. I chose it mainly for:

• Its api which is very easy to use
• Its explicit typing
• Its very well provided and organized documentation

As well as its popularity which only grows with time.

Inquirer remains an interesting choice, but:

• The documentation is less well organized
• The API is not intuitive

I'm talking here from my personal point of view, and one way or another it's up to you to decide what you prefer for Vite. I can of course redo all the code with Inquirer if you prefer.

[Shinigami92]

I'm a bit splitted here 😕

First IMO WindiCSS is an improvement on top of TailwindCSS, like SCSS is for CSS
But let's not discuss this here...

Create React App uses prompts
https://github.com/facebook/create-react-app/blob/fddce8a9e21bf68f37054586deb0c8636a45f50b/packages/create-react-app/package.json#L38

Vue-CLI uses inquirer
https://github.com/vuejs/vue-cli/blob/2d3116ed50de185bee584d98d48cf7223782367d/packages/%40vue/cli/package.json#L45

I would like to let @patak-js or @antfu decide it 😅

[Olyno]

First IMO WindiCSS is an improvement on top of TailwindCSS, like SCSS is for CSS

Yeah I know ahah, I was trying to show you an absurd comparison to explain you that numbers don't mean everything, WindiCSS was maybe not the best example 😅

[patak-dev]

I think it would be a good idea to check if inquirer is a better option here. @sodatea was there a rationale to choose it for vue-cli?

[haoqunjiang]

I've considered switching to prompts in Vue CLI, too. It is much smaller and works as well. (inquirer depends on rxjs, which is huge.)

The only thing that blocks me from doing so is that we've made the inquirer question object format a public API for CLI plugins. I can't just change the dependency and leave the plugin authors to figure out the differences.

[patak-dev]

Thanks @sodatea, I think switching to prompts is a good idea. @antfu what do you think?

@Olyno the PR is still changing more things than just the move to prompts though. The getValidPackageName shouldn't be removed. This function is used to have an extra question when the project name is not valid.
The project could be named "Hello World", and in that case a new question will ask if "hello-world" is ok as the package-name and let the user change it.

[antfu]

Don't get me wrong, I am not against switching tools. As least we should explain a bit about why, otherwise this is like "changing the tool because of a bug" (as there are also like to have other bugs in the tool you are replacing with, there will be no end to this). And I guess that's also why @patak-js is suggesting to make into two PRs.

As you have provided the detailed explanations, and they sounds reasonable to me now. 👍

[Olyno]

This function is used to have an extra question when the project name is not valid.

Well I just did changes using the already existing code and the same logic @patak-js. Here is with the actual create-app package:

And here is with my changes:

[patak-dev]

@Olyno it is not the same. You should be able to create a project name "Hello World" (that is the name of the directory). In the version in this PR you can no longer do that, please check this comment from Evan #2385 (comment)

Also, we should allow user to create directories with any name, we just need to make sure the name used in package.json is valid so that npm/yarn doesn't complain.

[Olyno]

And I'm with @antfu about not switching just cause "changing the tool because of a bug"

Well, I gave more than this argument:

If I chose prompts as a replacement, it's not about replacing with the first tool that comes along. I chose it mainly for:

• Its api which is very easy to use
• Its explicit typing
• Its very well provided and organized documentation

Is this now the new behavior? If so, I really don't like that and we should not print a Stack Trace just when normally cancelled. (e.g. Ctrl+C)

Changed

[antfu]

Updated and handled some edge cases so the tests are now passing

[patak-dev]

The UX is a lot better with prompts, there a lot of little details that make this a good move ❤️

The only thing is that we lost the ability to use a project name that is not a valid package name. See this comment by Evan: #2385 (comment)

we should allow user to create directories with any name, we just need to make sure the name used in package.json is valid so that npm/yarn doesn't complain.

In the current version, after the user enters a project name (we always accepts it), then:

• if it is a valid package name, we use it as is
• if it is invalid, we add an extra prompt that starts with a best-guess for a valid package name from the project name (replacing all invalid chars by -) as a placeholder and we let the user modify it

[patak-dev]

Pushed a commit to support non-valid project names. It also allows for users to set project name to ., which targets the current dir (so the prev message was re-instated). Some users were using this already.

Added also a throw if the user doesn't confirm that it is ok to delete the target dir files. I don't know if this is the correct pattern for prompts.

Edit: Fixing the tests now 😅

[patak-dev]

@Olyno @antfu I left one TODO because I don't know how to properly cancel the prompts if the user doesn't confirm that the target directory can be emptied.

[Olyno]

Done @patak-js 👍🏻

[patak-dev]

LGTM now, except for the failing tests. @jamesgeorge007 maybe you could help us here?

[jamesgeorge007]

Migrating to prompts doesn't call for a major refactor as it offers a similar API surface.

diff --git a/packages/create-app/index.js b/packages/create-app/index.js
index 1cbfb075..5f369d0e 100755
--- a/packages/create-app/index.js
+++ b/packages/create-app/index.js
@@ -5,7 +5,7 @@ const fs = require('fs')
 const path = require('path')
 const argv = require('minimist')(process.argv.slice(2))
 // eslint-disable-next-line node/no-restricted-require
-const { prompt } = require('enquirer')
+const prompts = require('prompts')
 const {
   yellow,
   green,
@@ -131,11 +131,18 @@ async function init() {
     /**
      * @type {{ projectName: string }}
      */
-    const { projectName } = await prompt({
-      type: 'input',
+    const { projectName } = await prompts({
+      type: 'text',
       name: 'projectName',
       message: `Project name:`,
-      initial: 'vite-project'
+      initial: 'vite-project',
+      onState: ({ aborted }) => {
+        if (aborted) {
+          process.nextTick(() => {
+            process.exit(1)
+          })
+        }
+      }
     })
     targetDir = projectName
   }
@@ -150,7 +157,7 @@ async function init() {
       /**
        * @type {{ yes: boolean }}
        */
-      const { yes } = await prompt({
+      const { yes } = await prompts({
         type: 'confirm',
         name: 'yes',
         initial: 'Y',
@@ -159,7 +166,14 @@ async function init() {
             ? 'Current directory'
             : `Target directory ${targetDir}`) +
           ' is not empty.\n' +
-          'Remove existing files and continue?'
+          'Remove existing files and continue?',
+        onState: ({ aborted }) => {
+          if (aborted) {
+            process.nextTick(() => {
+              process.exit(1)
+            })
+          }
+        }
       })
       if (yes) {
         emptyDir(root)
@@ -184,41 +198,44 @@ async function init() {
     /**
      * @type {{ framework: string }}
      */
-    const { framework } = await prompt({
+    const { framework } = await prompts({
       type: 'select',
       name: 'framework',
       message,
-      format(name) {
-        const framework = FRAMEWORKS.find((v) => v.name === name)
-        return framework
-          ? framework.color(framework.display || framework.name)
-          : name
-      },
       choices: FRAMEWORKS.map((f) => ({
-        name: f.name,
-        value: f.name,
-        message: f.color(f.display || f.name)
-      }))
+        title: f.color(f.display || f.name),
+        value: f.name
+      })),
+      onState: ({ aborted }) => {
+        if (aborted) {
+          process.nextTick(() => {
+            process.exit(1)
+          })
+        }
+      }
     })
+
     const frameworkInfo = FRAMEWORKS.find((f) => f.name === framework)

     if (frameworkInfo.variants) {
       /**
        * @type {{ name: string }}
        */
-      const { name } = await prompt({
+      const { name } = await prompts({
         type: 'select',
         name: 'name',
-        format(name) {
-          const variant = frameworkInfo.variants.find((v) => v.name === name)
-          return variant ? variant.color(variant.display || variant.name) : name
-        },
         message: 'Select a variant:',
         choices: frameworkInfo.variants.map((v) => ({
-          name: v.name,
-          value: v.name,
-          message: v.color(v.display || v.name)
-        }))
+          title: v.color(v.display || v.name),
+          value: v.name
+        })),
+        onState: ({ aborted }) => {
+          if (aborted) {
+            process.nextTick(() => {
+              process.exit(1)
+            })
+          }
+        }
       })
       template = name
     } else {
@@ -288,13 +305,20 @@ async function getValidPackageName(projectName) {
     /**
      * @type {{ inputPackageName: string }}
      */
-    const { inputPackageName } = await prompt({
-      type: 'input',
+    const { inputPackageName } = await prompts({
+      type: 'text',
       name: 'inputPackageName',
       message: `Package name:`,
       initial: suggestedPackageName,
       validate: (input) =>
-        packageNameRegExp.test(input) ? true : 'Invalid package.json name'
+        packageNameRegExp.test(input) ? true : 'Invalid package.json name',
+      onState: ({ aborted }) => {
+        if (aborted) {
+          process.nextTick(() => {
+            process.exit(1)
+          })
+        }
+      }
     })
     return inputPackageName
   }