Skip to content

Conversation

@wojtekmaj
Copy link
Contributor

@wojtekmaj wojtekmaj commented Mar 28, 2025

Summary

This PR migrates the entire repository from ora to nanospinner. nanospinner has a 17.5 TIMES smaller footprint [1], [2], and has just one dependency as opposed to 17.

Note that ora is not removed from this repo's dependencies, as it remains a dependency of inquirer. However, it's removed from package dependencies, resulting in faster install and smaller footprint for the end users.

Test Plan

To be frank, I relied on TypeScript alone for this change and I don't know this project well enough to test it, especially the Android part. nanospinner is almost a drop-in replacement for ora so I don't expect major issues though.

Checklist

  • Documentation is up to date.
  • Follows commit message convention described in CONTRIBUTING.md.
  • For functional changes, my test plan has linked these CLI changes into a local react-native checkout (instructions).

@github-actions github-actions bot added the docs Documentation change label Mar 28, 2025
@wojtekmaj wojtekmaj force-pushed the wmaj/nanospinner branch from 42f5ff9 to 5c2826f Compare May 4, 2025 17:10
@github-actions
Copy link

There hasn't been any activity on this pull request in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.

@github-actions github-actions bot added the stale label Aug 19, 2025
@thymikee thymikee removed the stale label Aug 19, 2025
@github-actions
Copy link

There hasn't been any activity on this pull request in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.

@github-actions github-actions bot added the stale label Dec 19, 2025
Copilot AI review requested due to automatic review settings December 19, 2025 08:01
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the codebase from ora to nanospinner for spinner/loader functionality, aiming to reduce bundle size (nanospinner is 17.5x smaller) and dependencies (1 vs 17). The migration involves updating API calls from succeed()/fail() to success()/error(), changing from direct property assignment (loader.text =) to method calls (loader.update()), and updating type imports across multiple packages.

Key changes:

  • Replaced ora with nanospinner in package dependencies for cli-tools, cli-doctor, and cli-config-apple packages
  • Updated loader wrapper in cli-tools to use nanospinner's API
  • Migrated all spinner method calls from ora's API to nanospinner's API across the codebase

Reviewed changes

Copilot reviewed 35 out of 36 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
yarn.lock Added nanospinner@^1.0.0 dependency
packages/cli/src/commands/init/init.ts Updated spinner method calls from succeed() to success()
packages/cli/src/commands/init/git.ts Updated spinner method call from succeed() to success()
packages/cli-tools/src/loader.ts Replaced ora with nanospinner, updated NoopLoader implementation
packages/cli-tools/package.json Replaced ora dependency with nanospinner
packages/cli-platform-android/src/commands/runAndroid/listAndroidTasks.ts Updated spinner method calls to use nanospinner API
packages/cli-doctor/src/types.ts Updated Loader type from Ora to Spinner
packages/cli-doctor/src/tools/runAutomaticFix.ts Removed unsupported prefixText option
packages/cli-doctor/src/tools/install.ts Updated fail() to error()
packages/cli-doctor/src/tools/healthchecks/*.ts Updated spinner API calls across all healthcheck modules
packages/cli-doctor/src/tools/healthchecks/tests/*.test.ts Updated test spies to match new API
packages/cli-doctor/src/tools/downloadAndUnzip.ts Changed from property assignment to update() method
packages/cli-doctor/src/tools/brewInstall.ts Updated succeed() to success()
packages/cli-doctor/package.json Replaced ora dependency with nanospinner
packages/cli-config-apple/src/tools/runBundleInstall.ts Updated type import and spinner methods
packages/cli-config-apple/src/tools/pods.ts Updated getLoader calls to use options object, updated spinner methods
packages/cli-config-apple/src/tools/installPods.ts Updated spinner methods and type annotations (has issues)
packages/cli-config-apple/package.json Replaced ora dependency with nanospinner
packages/cli-clean/src/clean.ts Updated spinner method calls
docs/init.md Updated documentation to reference nanospinner instead of ora
docs/healthChecks.md Updated documentation to reference nanospinner API
Comments suppressed due to low confidence (1)

packages/cli-config-apple/src/tools/installPods.ts:4

  • Unused import chalk.
import chalk from 'chalk';

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +54 to 58
export function getLoader(options?: Options): Loader {
return logger.isVerbose()
? new NanospinnerNoop()
: createSpinner('', options);
}
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The getLoader function incorrectly passes options to createSpinner. According to nanospinner's API, createSpinner takes text as the first parameter and options as the second. However, when getLoader is called with {text: 'some text'}, this implementation passes an empty string as the first parameter and the entire options object (including the text property) as the second parameter. The function should extract the text from options and pass it as the first parameter to createSpinner.

Copilot uses AI. Check for mistakes.
Comment on lines +10 to 11
success() {
return this;
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The success method signature should accept an optional text parameter to match the nanospinner API, which allows updating the spinner text when calling success(text). The same applies to other methods like error, warn, and info.

Copilot uses AI. Check for mistakes.
Comment on lines +25 to 26
start() {
return this;
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The start method signature should accept an optional text parameter to match the nanospinner API, which allows starting or restarting the spinner with updated text by calling start(text).

Copilot uses AI. Check for mistakes.
Comment on lines +28 to 29
update() {
return this;
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The update method signature should accept an optional text parameter to match the nanospinner API, which requires passing text when calling update(text) to update the spinner message.

Copilot uses AI. Check for mistakes.
@@ -1,7 +1,7 @@
import fs from 'fs';
import execa from 'execa';
import type {Ora} from 'ora';
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The import statement still references the old ora package type. This should be updated to import Spinner from nanospinner instead. The type Ora is no longer available after migrating to nanospinner.

Suggested change
import type {Ora} from 'ora';
import type {Spinner} from 'nanospinner';

Copilot uses AI. Check for mistakes.
import execa from 'execa';
import type {Ora} from 'ora';
import pico from 'picocolors';
import chalk from 'chalk';
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The import has been changed from pico (picocolors) to chalk, but the code still uses pico throughout the file (lines 30, 32, 86, 97, 98, 130, 131). This will cause runtime errors. Either revert to pico or update all usages to chalk.

Copilot uses AI. Check for mistakes.
@wojtekmaj
Copy link
Contributor Author

Welp, with so many changes on main this PR is now completely broken. It was worth a try 😢

@wojtekmaj wojtekmaj closed this Dec 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation change stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants