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

fix(deps): replace ora and log-symbols with tiny dependency picospinner #7026

Merged
merged 2 commits into from
Mar 14, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
320 changes: 25 additions & 295 deletions package-lock.json

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@
"lambda-local": "2.2.0",
"locate-path": "7.2.0",
"lodash": "4.17.21",
"log-symbols": "7.0.0",
"log-update": "6.1.0",
"maxstache": "1.0.7",
"maxstache-stream": "1.0.4",
Expand All @@ -138,7 +137,6 @@
"netlify-redirector": "0.5.0",
"node-fetch": "3.3.2",
"open": "10.1.0",
"ora": "8.2.0",
"p-filter": "4.1.0",
"p-map": "7.0.3",
"p-wait-for": "5.0.2",
Expand All @@ -163,7 +161,8 @@
"uuid": "11.0.5",
"wait-port": "1.1.0",
"write-file-atomic": "5.0.1",
"ws": "8.18.1"
"ws": "8.18.1",
"yocto-spinner": "0.2.0"
},
"devDependencies": {
"@babel/preset-react": "7.26.3",
Expand Down
17 changes: 7 additions & 10 deletions scripts/prepare-for-publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { dirname, join } from 'path'
import { fileURLToPath } from 'url'

import execa from 'execa'
import ora from 'ora'
import yoctoSpinner from 'yocto-spinner'

// These scripts from package.json need to be preserved on publish
const preserveScripts = new Set([
Expand All @@ -15,8 +15,7 @@ const preserveScripts = new Set([
'prepublishOnly',
])

let spinner = ora({
spinner: 'star',
Copy link
Collaborator Author

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

let spinner = yoctoSpinner({
text: 'Patching package.json (removing devDependencies, scripts, etc)',
}).start()

Expand All @@ -41,18 +40,16 @@ pkgJson.scripts.postinstall = pkgJson.scripts['postinstall-pack']
delete pkgJson.scripts['postinstall-pack']

await writeFile(packageJsonPath, JSON.stringify(pkgJson, null, 2))
spinner.succeed()
spinner.success()

spinner = ora({
spinner: 'star',
spinner = yoctoSpinner({
text: 'Running `npm install --no-audit`',
}).start()
await execa('npm', ['install', '--no-audit'])
spinner.succeed()
spinner.success()

spinner = ora({
spinner: 'star',
spinner = yoctoSpinner({
text: 'Running `npm shrinkwrap`',
}).start()
await execa('npm', ['shrinkwrap'])
spinner.succeed()
spinner.success()
30 changes: 10 additions & 20 deletions src/commands/deploy/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { getBootstrapURL } from '../../lib/edge-functions/bootstrap.js'
import { featureFlags as edgeFunctionsFeatureFlags } from '../../lib/edge-functions/consts.js'
import { normalizeFunctionsConfig } from '../../lib/functions/config.js'
import { BACKGROUND_FUNCTIONS_WARNING } from '../../lib/log.js'
import { startSpinner, stopSpinner } from '../../lib/spinner.js'
import { type Spinner, startSpinner, stopSpinner } from '../../lib/spinner.js'
import { detectFrameworkSettings, getDefaultConfig } from '../../utils/build-info.js'
import {
NETLIFYDEV,
Expand All @@ -33,7 +33,7 @@ import {
APIError,
} from '../../utils/command-helpers.js'
import { DEFAULT_DEPLOY_TIMEOUT } from '../../utils/deploy/constants.js'
import { deploySite } from '../../utils/deploy/deploy-site.js'
import { type DeployEvent, deploySite } from '../../utils/deploy/deploy-site.js'
import { getEnvelopeEnv } from '../../utils/env/index.js'
import { getFunctionsManifestPath, getInternalFunctionsDir } from '../../utils/functions/index.js'
import openBrowser from '../../utils/open-browser.js'
Expand Down Expand Up @@ -309,40 +309,30 @@ const reportDeployError = ({ error_, failAndExit }) => {
}

const deployProgressCb = function () {
/**
* @type {Record<string, import('ora').Ora>}
*/
const events = {}
// @ts-expect-error TS(7006) FIXME: Parameter 'event' implicitly has an 'any' type.
return (event) => {
const spinnersByType: Record<DeployEvent['type'], Spinner> = {}
return (event: DeployEvent) => {
switch (event.phase) {
case 'start': {
// @ts-expect-error TS(7053) FIXME: Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
events[event.type] = startSpinner({
spinnersByType[event.type] = startSpinner({
text: event.msg,
})
return
}
case 'progress': {
// @ts-expect-error TS(7053) FIXME: Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
const spinner = events[event.type]
const spinner = spinnersByType[event.type]
if (spinner) {
spinner.text = event.msg
}
return
}
case 'error':
// @ts-expect-error TS(7053) FIXME: Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
stopSpinner({ error: true, spinner: events[event.type], text: event.msg })
// @ts-expect-error TS(7053) FIXME: Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
delete events[event.type]
stopSpinner({ error: true, spinner: spinnersByType[event.type], text: event.msg })
delete spinnersByType[event.type]
return
case 'stop':
default: {
// @ts-expect-error TS(2345) FIXME: Argument of type '{ spinner: any; text: any; }' is... Remove this comment to see the full error message
stopSpinner({ spinner: events[event.type], text: event.msg })
// @ts-expect-error TS(7053) FIXME: Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
delete events[event.type]
stopSpinner({ spinner: spinnersByType[event.type], text: event.msg })
delete spinnersByType[event.type]
}
}
}
Expand Down
13 changes: 9 additions & 4 deletions src/commands/functions/functions-create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -45,6 +45,11 @@ const languages = [
{ name: 'Rust', value: 'rust' },
]

const MOON_SPINNER = {
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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') {
Expand Down
4 changes: 1 addition & 3 deletions src/commands/sites/sites-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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 --json isn't true.

stopSpinner({ spinner })
}

Expand Down
19 changes: 5 additions & 14 deletions src/commands/watch/watch.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { OptionValues } from 'commander'
import pWaitFor from 'p-wait-for'
import prettyjson from 'prettyjson'
import type { NetlifyAPI } from 'netlify'

import { startSpinner, stopSpinner } from '../../lib/spinner.js'
import { type Spinner, startSpinner, stopSpinner } from '../../lib/spinner.js'
import { chalk, error, log } from '../../utils/command-helpers.js'
import BaseCommand from '../base-command.js'
import { init } from '../init/init.js'
Expand All @@ -15,28 +16,18 @@ const BUILD_FINISH_INTERVAL = 1e3
// 20 minutes
const BUILD_FINISH_TIMEOUT = 12e5

/**
*
* @param {import('netlify').NetlifyAPI} api
* @param {string} siteId
* @param {import('ora').Ora} spinner
* @returns {Promise<boolean>}
*/
// @ts-expect-error TS(7006) FIXME: Parameter 'api' implicitly has an 'any' type.
const waitForBuildFinish = async function (api, siteId, spinner) {
const waitForBuildFinish = async function (api: NetlifyAPI, siteId: string, spinner: Spinner) {
let firstPass = true

const waitForBuildToFinish = async function () {
const builds = await api.listSiteBuilds({ siteId })
// build.error
// @ts-expect-error TS(7006) FIXME: Parameter 'build' implicitly has an 'any' type.
const currentBuilds = builds.filter((build) => !build.done)

// if build.error
// @TODO implement build error messages into this

if (!currentBuilds || currentBuilds.length === 0) {
// @ts-expect-error TS(2345) FIXME: Argument of type '{ spinner: any; }' is not assign... Remove this comment to see the full error message
stopSpinner({ spinner })
return true
}
Expand Down Expand Up @@ -64,7 +55,7 @@ export const watch = async (options: OptionValues, command: BaseCommand) => {
if (!siteId) {
// TODO: build init command
const siteData = await init({}, command)
siteId = siteData.id
siteId = siteData.id as string
}

// wait for 1 sec for everything to kickoff
Expand Down Expand Up @@ -101,7 +92,7 @@ export const watch = async (options: OptionValues, command: BaseCommand) => {

const noActiveBuilds = await waitForBuildFinish(client, siteId, spinner)

const siteData = await client.getSite({ siteId: siteId as string })
const siteData = await client.getSite({ siteId })

const message = chalk.cyanBright.bold.underline(noActiveBuilds ? 'Last build' : 'Deploy complete')
log()
Expand Down
2 changes: 1 addition & 1 deletion src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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
Expand Down
4 changes: 2 additions & 2 deletions src/lib/edge-functions/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { FeatureFlags, getFeatureFlagsFromSiteInfo } from '../../utils/feature-f
import { BlobsContextWithEdgeAccess } from '../blobs/blobs.js'
import { getGeoLocation } from '../geo-location.js'
import { getPathInProject } from '../settings.js'
import { startSpinner, stopSpinner } from '../spinner.js'
import { type Spinner, startSpinner, stopSpinner } from '../spinner.js'

import { getBootstrapURL } from './bootstrap.js'
import { DIST_IMPORT_MAP_PATH, EDGE_FUNCTIONS_SERVE_FOLDER } from './consts.js'
Expand All @@ -25,7 +25,7 @@ const headersSymbol = Symbol('Edge Functions Headers')
const LOCAL_HOST = '127.0.0.1'

const getDownloadUpdateFunctions = () => {
let spinner: ReturnType<typeof startSpinner>
let spinner: Spinner

const onAfterDownload = (error_: unknown) => {
stopSpinner({ error: Boolean(error_), spinner })
Expand Down
32 changes: 18 additions & 14 deletions src/lib/spinner.ts
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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the default in ora, which we were implicitly using here.


/**
* 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 }
9 changes: 8 additions & 1 deletion src/utils/deploy/deploy-site.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ const buildStatsString = (possibleParts: Array<string | false | undefined>) => {
return parts.length > 1 ? `${message} and ${parts[parts.length - 1]}` : message
}

// TODO(serhalp) This is alternatingly called "event", "status", and "progress". Standardize.
export interface DeployEvent {
type: string
msg: string
phase: 'start' | 'progress' | 'error' | 'stop'
}

export const deploySite = async (
command: BaseCommand,
api: $TSFixMe,
Expand Down Expand Up @@ -74,7 +81,7 @@ export const deploySite = async (
deployTimeout?: number
draft?: boolean
maxRetry?: number
statusCb?: (status: { type: string; msg: string; phase: string }) => void
statusCb?: (status: DeployEvent) => void
syncFileLimit?: number
tmpDir?: string
fnDir?: string[]
Expand Down
4 changes: 3 additions & 1 deletion src/utils/framework-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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 {
Expand Down
Loading
Loading