perf: switch to clack for CLI prompts #14589
+83
−206
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,14 @@ | ||
| import fsMod, { existsSync, promises as fs } from 'node:fs'; | ||
| import path from 'node:path'; | ||
| import { fileURLToPath, pathToFileURL } from 'node:url'; | ||
| import * as clack from '@clack/prompts'; | ||
| import { diffWords } from 'diff'; | ||
| import { bold, cyan, dim, green, magenta, red, yellow } from 'kleur/colors'; | ||
| import { type ASTNode, builders, generateCode, loadFile, type ProxifiedModule } from 'magicast'; | ||
| import { getDefaultExportOptions } from 'magicast/helpers'; | ||
| import { detect, resolveCommand } from 'package-manager-detector'; | ||
| import maxSatisfying from 'semver/ranges/max-satisfying.js'; | ||
| import type yargsParser from 'yargs-parser'; | ||
| import { | ||
| loadTSConfig, | ||
| resolveConfig, | ||
|
|
@@ -363,19 +361,19 @@ export async function add(names: string[], { flags }: AddOptions) { | |
| ), | ||
| ); | ||
| if (integrations.find((integration) => integration.integrationName === 'tailwind')) { | ||
| logger.warn( | ||
| 'SKIP_FORMAT', | ||
| msg.actionRequired( | ||
| 'You must import your Tailwind stylesheet, e.g. in a shared layout:\n', | ||
| ), | ||
| ); | ||
| clack.box( | ||
| getDiffContent('---\n---', "---\nimport '../styles/global.css'\n---")!, | ||
| 'src/layouts/Layout.astro', | ||
| { | ||
| rounded: true, | ||
| }, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -561,18 +559,15 @@ async function updateAstroConfig({ | |
| return UpdateResult.none; | ||
| } | ||
|
|
||
| logger.info( | ||
| 'SKIP_FORMAT', | ||
| `\n ${magenta('Astro will make the following changes to your config file:')}`, | ||
| ); | ||
|
|
||
| clack.box(diff, configURL.pathname.split('/').pop(), { | ||
| rounded: true, | ||
| }); | ||
|
|
||
| if (logAdapterInstructions) { | ||
| logger.info( | ||
| 'SKIP_FORMAT', | ||
|
|
@@ -676,20 +671,19 @@ async function tryToInstallIntegrations({ | |
| ); | ||
|
|
||
| const coloredOutput = `${bold(installCommand.command)} ${installCommand.args.join(' ')} ${cyan(installSpecifiers.join(' '))}`; | ||
| logger.info( | ||
| 'SKIP_FORMAT', | ||
| `\n ${magenta('Astro will run the following command:')}\n ${dim( | ||
| 'If you skip this step, you can always run it yourself later', | ||
| )}`, | ||
| ); | ||
| clack.box(coloredOutput, undefined, { | ||
| rounded: true, | ||
| }); | ||
|
|
||
| if (await askToContinue({ flags })) { | ||
| const spinner = clack.spinner(); | ||
| spinner.start('Installing dependencies...'); | ||
| try { | ||
| await exec(installCommand.command, [...installCommand.args, ...installSpecifiers], { | ||
| nodeOptions: { | ||
|
|
@@ -698,10 +692,10 @@ async function tryToInstallIntegrations({ | |
| env: { NODE_ENV: undefined }, | ||
| }, | ||
| }); | ||
| spinner.stop(); | ||
| return UpdateResult.updated; | ||
| } catch (err: any) { | ||
| spinner.stop(undefined, 2); | ||
| logger.debug('add', 'Error installing dependencies', err); | ||
| // NOTE: `err.stdout` can be an empty string, so log the full error instead for a more helpful log | ||
| console.error('\n', err.stdout || err.message, '\n'); | ||
|
|
@@ -716,7 +710,8 @@ async function validateIntegrations( | |
| integrations: string[], | ||
| flags: yargsParser.Arguments, | ||
| ): Promise<IntegrationInfo[]> { | ||
| const spinner = clack.spinner(); | ||
| spinner.start('Resolving packages...'); | ||
| try { | ||
| const integrationEntries = await Promise.all( | ||
| integrations.map(async (integration): Promise<IntegrationInfo> => { | ||
|
|
@@ -734,17 +729,17 @@ async function validateIntegrations( | |
| const firstPartyPkgCheck = await fetchPackageJson('@astrojs', name, tag); | ||
| if (firstPartyPkgCheck instanceof Error) { | ||
| if (firstPartyPkgCheck.message) { | ||
| spinner.message(yellow(firstPartyPkgCheck.message)); | ||
| } | ||
| spinner.message(yellow(`${bold(integration)} is not an official Astro package.`)); | ||
| if (!(await askToContinue({ flags }))) { | ||
| throw new Error( | ||
| `No problem! Find our official integrations at ${cyan( | ||
| 'https://astro.build/integrations', | ||
| )}`, | ||
| ); | ||
| } | ||
| spinner.message('Resolving with third party packages...'); | ||
| pkgType = 'third-party'; | ||
| } else { | ||
| pkgType = 'first-party'; | ||
|
|
@@ -755,7 +750,7 @@ async function validateIntegrations( | |
| const thirdPartyPkgCheck = await fetchPackageJson(scope, name, tag); | ||
| if (thirdPartyPkgCheck instanceof Error) { | ||
| if (thirdPartyPkgCheck.message) { | ||
| spinner.message(yellow(thirdPartyPkgCheck.message)); | ||
| } | ||
| throw new Error(`Unable to fetch ${bold(integration)}. Does the package exist?`); | ||
| } else { | ||
|
|
@@ -813,11 +808,11 @@ async function validateIntegrations( | |
| }; | ||
| }), | ||
| ); | ||
| spinner.stop(); | ||
| return integrationEntries; | ||
| } catch (e) { | ||
| if (e instanceof Error) { | ||
| spinner.stop(e.message, 2); | ||
|
There was a problem hiding this comment. Linking bombshell-dev/clack#405 here as a reminder to revisit if that change to the spinner API goes ahead. |
||
| process.exit(1); | ||
| } else { | ||
| throw e; | ||
|
|
@@ -872,18 +867,15 @@ async function updateTSConfig( | |
| return UpdateResult.none; | ||
| } | ||
|
|
||
| logger.info( | ||
| 'SKIP_FORMAT', | ||
| `\n ${magenta(`Astro will make the following changes to your ${configFileName}:`)}`, | ||
| ); | ||
|
|
||
| clack.box(diff, configFileName, { | ||
| rounded: true, | ||
| }); | ||
|
|
||
| // Every major framework, apart from Vue and Svelte requires different `jsxImportSource`, as such it's impossible to config | ||
| // all of them in the same `tsconfig.json`. However, Vue only need `"jsx": "preserve"` for template intellisense which | ||
| // can be compatible with some frameworks (ex: Solid) | ||
|
|
@@ -935,14 +927,12 @@ function parseIntegrationName(spec: string) { | |
| async function askToContinue({ flags }: { flags: Flags }): Promise<boolean> { | ||
| if (flags.yes || flags.y) return true; | ||
|
|
||
| const response = await clack.confirm({ | ||
| message: 'Continue?', | ||
| initialValue: true, | ||
| }); | ||
|
|
||
| return response === true; | ||
| } | ||
|
|
||
| function getDiffContent(input: string, output: string): string | null { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,10 +1,8 @@ | ||||||||||
| import { createRequire } from 'node:module'; | ||||||||||
| import * as clack from '@clack/prompts'; | ||||||||||
| import ci from 'ci-info'; | ||||||||||
| import { bold, cyan, dim, magenta } from 'kleur/colors'; | ||||||||||
| import { detect, resolveCommand } from 'package-manager-detector'; | ||||||||||
| import type { Logger } from '../core/logger/core.js'; | ||||||||||
| import { exec } from './exec.js'; | ||||||||||
|
|
||||||||||
|
|
@@ -75,34 +73,30 @@ async function installPackage( | |||||||||
| packageNames = packageNames.map((name) => `npm:${name}`); | ||||||||||
| } | ||||||||||
| const coloredOutput = `${bold(installCommand.command)} ${installCommand.args.join(' ')} ${cyan(packageNames.join(' '))}`; | ||||||||||
| logger.info( | ||||||||||
| 'SKIP_FORMAT', | ||||||||||
| `\n ${magenta('Astro will run the following command:')}\n ${dim( | ||||||||||
| 'If you skip this step, you can always run it yourself later', | ||||||||||
| )}`, | ||||||||||
| ); | ||||||||||
| clack.box(coloredOutput, undefined, { | ||||||||||
| rounded: true, | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| let response; | ||||||||||
| if (options.skipAsk) { | ||||||||||
| response = true; | ||||||||||
| } else { | ||||||||||
| response = | ||||||||||
| (await clack.confirm({ | ||||||||||
| message: 'Continue?', | ||||||||||
| initialValue: true, | ||||||||||
| })) === true; | ||||||||||
| } | ||||||||||
|
Comment on lines
-78
to
95
There was a problem hiding this comment. Documenting the visual changes here:
Cancel state:
I quite like how the bold text in the old version helps the “Continue?” question stand out, so could be something for us to create a little set of styled utilities for? For example, something like this would help us use that style consistently: const confirm = ({ message, ...opts }) =>
clack.confirm({ message: bold(message), ...opts });As noted in my main comment, you can see the inconsistency of switching between the Clack style with the left border and the regular logging here. |
||||||||||
|
|
||||||||||
| if (Boolean(response)) { | ||||||||||
| const spinner = clack.spinner(); | ||||||||||
| spinner.start('Installing dependencies...'); | ||||||||||
| try { | ||||||||||
| await exec(installCommand.command, [...installCommand.args, ...packageNames], { | ||||||||||
| nodeOptions: { | ||||||||||
|
|
@@ -111,12 +105,12 @@ async function installPackage( | |||||||||
| env: { NODE_ENV: undefined }, | ||||||||||
| }, | ||||||||||
| }); | ||||||||||
| spinner.stop(); | ||||||||||
|
|
||||||||||
| return true; | ||||||||||
| } catch (err) { | ||||||||||
| logger.debug('add', 'Error installing dependencies', err); | ||||||||||
| spinner.stop(undefined, 2); | ||||||||||
|
|
||||||||||
| return false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,18 +72,17 @@ | |
| "test:types": "tsc --project test/types/tsconfig.json" | ||
| }, | ||
| "dependencies": { | ||
| "@clack/prompts": "1.0.0-alpha.6", | ||
| "@libsql/client": "^0.15.15", | ||
| "deep-diff": "^1.0.2", | ||
| "drizzle-orm": "^0.42.0", | ||
| "kleur": "^4.1.5", | ||
| "nanoid": "^5.1.6", | ||
| "yargs-parser": "^21.1.1", | ||
| "zod": "^3.25.76" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/deep-diff": "^1.0.5", | ||
| "@types/yargs-parser": "^21.0.3", | ||
| "astro": "workspace:*", | ||
| "astro-scripts": "workspace:*", | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import * as clack from '@clack/prompts'; | ||
| import type { AstroConfig } from 'astro'; | ||
| import { sql } from 'drizzle-orm'; | ||
| import type { Arguments } from 'yargs-parser'; | ||
| import { MIGRATION_VERSION } from '../../../consts.js'; | ||
| import { createClient } from '../../../db-client/libsql-node.js'; | ||
|
|
@@ -42,14 +42,12 @@ export async function cmd({ | |
| } | ||
|
|
||
| if (isForceReset) { | ||
| const begin = await clack.confirm({ | ||
| message: `Reset your database? All of your data will be erased and your schema created from scratch.`, | ||
| initialValue: false, | ||
| }); | ||
|
|
||
| if (begin !== true) { | ||
| console.log('Canceled.'); | ||
| process.exit(0); | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Quick comparison of the two states here:
The “Resolving packages” message is preserved currently as a reference to the completed task, whereas it looks like Clack’s
stop()clears that out. I think it would be nice to preserve a message to refer back to.Maybe as simple as?