-
Notifications
You must be signed in to change notification settings - Fork 385
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(deps): replace ora
and log-symbols
with tiny dependency picospinner
#7026
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
This should reduce our package size. I also added some missing types along the way.
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,7 @@ import { findUp } from 'find-up' | |
import fuzzy from 'fuzzy' | ||
import inquirer from 'inquirer' | ||
import fetch from 'node-fetch' | ||
import ora from 'ora' | ||
import yoctoSpinner from 'yocto-spinner' | ||
|
||
import { fileExistsAsync } from '../../lib/fs.js' | ||
import { getAddons, getCurrentAddon, getSiteData } from '../../utils/addons/prepare.js' | ||
|
@@ -45,6 +45,11 @@ const languages = [ | |
{ name: 'Rust', value: 'rust' }, | ||
] | ||
|
||
const MOON_SPINNER = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cute! |
||
interval: 80, | ||
frames: ['🌑 ', '🌒 ', '🌓 ', '🌔 ', '🌕 ', '🌖 ', '🌗 ', '🌘 '], | ||
} | ||
|
||
/** | ||
* prompt for a name if name not supplied | ||
* @param {string} argumentName | ||
|
@@ -541,12 +546,12 @@ const scaffoldFromTemplate = async function (command, options, argumentName, fun | |
|
||
// npm install | ||
if (functionPackageJson !== undefined) { | ||
const spinner = ora({ | ||
const spinner = yoctoSpinner({ | ||
text: `Installing dependencies for ${name}`, | ||
spinner: 'moon', | ||
spinner: MOON_SPINNER, | ||
}).start() | ||
await installDeps({ functionPackageJson, functionPath, functionsDir }) | ||
spinner.succeed(`Installed dependencies for ${name}`) | ||
spinner.success(`Installed dependencies for ${name}`) | ||
} | ||
|
||
if (funcType === 'edge') { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,16 +8,14 @@ import BaseCommand from '../base-command.js' | |
|
||
export const sitesList = async (options: OptionValues, command: BaseCommand) => { | ||
const { api } = command.netlify | ||
/** @type {import('ora').Ora} */ | ||
let spinner | ||
if (!options.json) { | ||
spinner = startSpinner({ text: 'Loading your sites' }) | ||
} | ||
await command.authenticate() | ||
|
||
const sites = await listSites({ api, options: { filter: 'all' } }) | ||
if (!options.json) { | ||
// @ts-expect-error TS(2345) FIXME: Argument of type '{ spinner: Ora | undefined; }' i... Remove this comment to see the full error message | ||
if (spinner) { | ||
Comment on lines
-19
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are equivalent, but this is less leaky and makes TS happy. We only define a spinner (upstream) when |
||
stopSpinner({ spinner }) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ const MAX_PAGES = 10 | |
const MAX_PER_PAGE = 100 | ||
|
||
// @ts-expect-error TS(7023) FIXME: 'listSites' implicitly has return type 'any' becau... Remove this comment to see the full error message | ||
export const listSites = async ({ api, options }): SiteInfo[] => { | ||
export const listSites = async ({ api, options }): Promise<SiteInfo[]> => { | ||
Comment on lines
18
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have the rest of this fixed in another draft PR. I just needed this to unblock errors elsewhere. |
||
const { maxPages = MAX_PAGES, page = FIRST_PAGE, ...rest } = options | ||
const sites = await api.listSites({ page, per_page: MAX_PER_PAGE, ...rest }) | ||
// TODO: use pagination headers when js-client returns them | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,34 +1,38 @@ | ||
import logSymbols from 'log-symbols' | ||
import ora, { Ora } from 'ora' | ||
import yoctoSpinner, { Spinner } from 'yocto-spinner' | ||
|
||
const DOTS_SPINNER = { | ||
interval: 80, | ||
frames: ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏'], | ||
} | ||
|
||
Comment on lines
+3
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was the default in |
||
/** | ||
* Creates a spinner with the following text | ||
*/ | ||
export const startSpinner = ({ text }: { text: string }) => | ||
ora({ | ||
yoctoSpinner({ | ||
text, | ||
spinner: DOTS_SPINNER, | ||
}).start() | ||
|
||
/** | ||
* Stops the spinner with the following text | ||
*/ | ||
export const stopSpinner = ({ error, spinner, text }: { error: boolean; spinner: Ora; text?: string }) => { | ||
export const stopSpinner = ({ error, spinner, text }: { error?: boolean; spinner: Spinner; text?: string }) => { | ||
if (!spinner) { | ||
return | ||
} | ||
// TODO: refactor no package needed `log-symbols` for that | ||
const symbol = error ? logSymbols.error : logSymbols.success | ||
spinner.stopAndPersist({ | ||
text, | ||
symbol, | ||
}) | ||
if (error === true) { | ||
spinner.error(text) | ||
} else { | ||
spinner.stop(text) | ||
} | ||
} | ||
|
||
/** | ||
* Clears the spinner | ||
*/ | ||
export const clearSpinner = ({ spinner }: { spinner: Ora }) => { | ||
if (spinner) { | ||
spinner.stop() | ||
} | ||
export const clearSpinner = ({ spinner }: { spinner: Spinner }) => { | ||
spinner.clear() | ||
} | ||
|
||
export type { Spinner } |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,9 @@ export const startFrameworkServer = async function ({ | |
await rm(settings.dist, { recursive: true, force: true }) | ||
} | ||
|
||
runCommand(settings.command, { env: settings.env, spinner, cwd }) | ||
if (settings.command) { | ||
runCommand(settings.command, { env: settings.env, spinner, cwd }) | ||
} | ||
Comment on lines
-47
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you scroll up you'll see we were already doing this in one branch but not the other. Adding some type safety uncovered this. |
||
|
||
let port: { open: boolean; ipVersion?: 4 | 6 } | undefined | ||
try { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import execa from 'execa' | |
// @ts-expect-error TS(7016) FIXME: Could not find a declaration file for module 'stri... Remove this comment to see the full error message | ||
import stripAnsiCc from 'strip-ansi-control-characters' | ||
|
||
import type { Spinner } from '../lib/spinner.js' | ||
import { chalk, log, NETLIFYDEVERR, NETLIFYDEVWARN } from './command-helpers.js' | ||
import { processOnExit } from './dev.js' | ||
|
||
|
@@ -33,19 +34,15 @@ const cleanupBeforeExit = async ({ exitCode }) => { | |
} | ||
} | ||
|
||
/** | ||
* Run a command and pipe stdout, stderr and stdin | ||
* @param {string} command | ||
* @param {object} options | ||
* @param {import('ora').Ora|null} [options.spinner] | ||
* @param {NodeJS.ProcessEnv} [options.env] | ||
* @param {string} [options.cwd] | ||
* @returns {execa.ExecaChildProcess<string>} | ||
*/ | ||
// @ts-expect-error TS(7006) FIXME: Parameter 'command' implicitly has an 'any' type. | ||
export const runCommand = (command, options = {}) => { | ||
// @ts-expect-error TS(2339) FIXME: Property 'cwd' does not exist on type '{}'. | ||
const { cwd, env = {}, spinner = null } = options | ||
export const runCommand = ( | ||
command: string, | ||
options: { | ||
spinner?: Spinner | ||
env?: NodeJS.ProcessEnv | ||
cwd: string | ||
}, | ||
) => { | ||
const { cwd, env = {}, spinner } = options | ||
const commandProcess = execa.command(command, { | ||
preferLocal: true, | ||
// we use reject=false to avoid rejecting synchronously when the command doesn't exist | ||
|
@@ -62,16 +59,13 @@ export const runCommand = (command, options = {}) => { | |
|
||
// This ensures that an active spinner stays at the bottom of the commandline | ||
// even though the actual framework command might be outputting stuff | ||
// @ts-expect-error TS(7006) FIXME: Parameter 'writeStream' implicitly has an 'any' ty... Remove this comment to see the full error message | ||
const pipeDataWithSpinner = (writeStream, chunk) => { | ||
if (spinner && spinner.isSpinning) { | ||
const pipeDataWithSpinner = (writeStream: NodeJS.WriteStream, chunk: any) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be clear, chunk is literally typed as |
||
if (spinner?.isSpinning) { | ||
spinner.clear() | ||
spinner.isSilent = true | ||
} | ||
writeStream.write(chunk, () => { | ||
if (spinner && spinner.isSpinning) { | ||
spinner.isSilent = false | ||
spinner.render() | ||
if (spinner?.isSpinning) { | ||
spinner.start() | ||
} | ||
}) | ||
} | ||
Comment on lines
+63
to
71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feel free to compare the docs for ora and picospinner and double-check me here, but I think this should reproduce the behaviour. Of course I'll poke at it manually before considering merging. |
||
|
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.
this one is a CI script so I didn't bother preserving the exact styling