fix(cli): fix Expo Go downloading indicator (#19817)

* fix(cli): fix Expo Go downloading indicator * Update packages/@expo/cli/CHANGELOG.md Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com> Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
expo · Nov 4, 2022 · 82f3de7 · 82f3de7
1 parent 64c662b
commit 82f3de7
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 50 deletions.
diff --git a/packages/@expo/cli/CHANGELOG.md b/packages/@expo/cli/CHANGELOG.md
@@ -8,6 +8,8 @@
 
 ### 🐛 Bug fixes
 
+- Fix Expo Go download loading bar. ([#19817](https://github.com/expo/expo/pull/19817) by [@EvanBacon](https://github.com/EvanBacon))
+
 ### 💡 Others
 
 ## 0.4.6 — 2022-11-02

diff --git a/packages/@expo/cli/src/api/rest/client.ts b/packages/@expo/cli/src/api/rest/client.ts
@@ -11,6 +11,7 @@ import { wrapFetchWithCache } from './cache/wrapFetchWithCache';
 import { FetchLike } from './client.types';
 import { wrapFetchWithBaseUrl } from './wrapFetchWithBaseUrl';
 import { wrapFetchWithOffline } from './wrapFetchWithOffline';
+import { wrapFetchWithProgress } from './wrapFetchWithProgress';
 import { wrapFetchWithProxy } from './wrapFetchWithProxy';
 
 export class ApiV2Error extends Error {
@@ -96,14 +97,14 @@ const fetchWithBaseUrl = wrapFetchWithBaseUrl(fetchWithOffline, getExpoApiBaseUr
 
 const fetchWithProxy = wrapFetchWithProxy(fetchWithBaseUrl);
 
-const fetchWithCredentials = wrapFetchWithCredentials(fetchWithProxy);
+const fetchWithCredentials = wrapFetchWithProgress(wrapFetchWithCredentials(fetchWithProxy));
 
 /**
  * Create an instance of the fully qualified fetch command (auto authentication and api) but with caching in the '~/.expo' directory.
  * Caching is disabled automatically if the EXPO_NO_CACHE or EXPO_BETA environment variables are enabled.
  */
 export function createCachedFetch({
-  fetch,
+  fetch = fetchWithCredentials,
   cacheDirectory,
   ttl,
   skipCache,
@@ -115,11 +116,11 @@ export function createCachedFetch({
 }): FetchLike {
   // Disable all caching in EXPO_BETA.
   if (skipCache || env.EXPO_BETA || env.EXPO_NO_CACHE) {
-    return fetch ?? fetchWithCredentials;
+    return fetch;
   }
 
   return wrapFetchWithCache(
-    fetch ?? fetchWithCredentials,
+    fetch,
     new FileSystemCache({
       cacheDirectory: path.join(getExpoHomeDirectory(), cacheDirectory),
       ttl,
@@ -128,4 +129,4 @@ export function createCachedFetch({
 }
 
 /** Instance of fetch with automatic base URL pointing to the Expo API, user credential injection, and API error handling. Caching not included.  */
-export const fetchAsync = wrapFetchWithCredentials(fetchWithProxy);
+export const fetchAsync = wrapFetchWithProgress(wrapFetchWithCredentials(fetchWithProxy));
diff --git a/packages/@expo/cli/src/api/rest/wrapFetchWithBaseUrl.ts b/packages/@expo/cli/src/api/rest/wrapFetchWithBaseUrl.ts
@@ -9,6 +9,7 @@ const debug = require('debug')('expo:api:fetch:base') as typeof console.log;
  * This implementation works like the browser fetch, applying the input to a prefix base URL.
  */
 export function wrapFetchWithBaseUrl(fetch: FetchLike, baseUrl: string): FetchLike {
+  // NOTE(EvanBacon): DO NOT RETURN AN ASYNC WRAPPER. THIS BREAKS LOADING INDICATORS.
   return (url, init) => {
     if (typeof url !== 'string') {
       throw new TypeError('Custom fetch function only accepts a string URL as the first parameter');

diff --git a/packages/@expo/cli/src/api/rest/wrapFetchWithOffline.ts b/packages/@expo/cli/src/api/rest/wrapFetchWithOffline.ts
@@ -5,7 +5,8 @@ const debug = require('debug')('expo:api:fetch:offline') as typeof console.log;
 
 /** Wrap fetch with support for APISettings offline mode. */
 export function wrapFetchWithOffline(fetchFunction: FetchLike): FetchLike {
-  return async function fetchWithOffline(url, options = {}) {
+  // NOTE(EvanBacon): DO NOT RETURN AN ASYNC WRAPPER. THIS BREAKS LOADING INDICATORS.
+  return function fetchWithOffline(url, options = {}) {
     if (APISettings.isOffline) {
       debug('Skipping network request: ' + url);
       options.timeout = 1;

diff --git a/packages/@expo/cli/src/api/rest/wrapFetchWithProgress.ts b/packages/@expo/cli/src/api/rest/wrapFetchWithProgress.ts
@@ -1,42 +1,46 @@
 import * as Log from '../../log';
 import { FetchLike } from './client.types';
+const debug = require('debug')('expo:api:fetch:progress') as typeof console.log;
 
 export function wrapFetchWithProgress(fetch: FetchLike): FetchLike {
-  return async (url, init) => {
-    const res = await fetch(url, init);
-    if (res.ok && init?.onProgress) {
-      const totalDownloadSize = res.headers.get('Content-Length');
-      const total = Number(totalDownloadSize);
+  return (url, init) => {
+    return fetch(url, init).then((res) => {
+      if (res.ok && init?.onProgress) {
+        const totalDownloadSize = res.headers.get('Content-Length');
+        const total = Number(totalDownloadSize);
 
-      if (!totalDownloadSize || isNaN(total) || total < 0) {
-        Log.warn(
-          'Progress callback not supported for network request because "Content-Length" header missing or invalid in response from URL:',
-          url.toString()
-        );
-        return res;
-      }
-
-      let length = 0;
-
-      res.body.on('data', (chunk) => {
-        length += chunk.length;
-        onProgress();
-      });
+        debug(`Download size: ${totalDownloadSize}`);
+        if (!totalDownloadSize || isNaN(total) || total < 0) {
+          Log.warn(
+            'Progress callback not supported for network request because "Content-Length" header missing or invalid in response from URL:',
+            url.toString()
+          );
+          return res;
+        }
 
-      res.body.on('end', () => {
-        onProgress();
-      });
+        let length = 0;
 
-      const onProgress = () => {
-        const progress = length / total;
+        debug(`Starting progress animation for ${url}`);
+        res.body.on('data', (chunk) => {
+          length += Buffer.byteLength(chunk);
+          onProgress();
+        });
 
-        init.onProgress?.({
-          progress,
-          total,
-          loaded: length,
+        res.body.on('end', () => {
+          debug(`Finished progress animation for ${url}`);
+          onProgress();
         });
-      };
-    }
-    return res;
+
+        const onProgress = () => {
+          const progress = length / total || 0;
+          init.onProgress?.({
+            progress,
+            total,
+            loaded: length,
+          });
+        };
+      }
+      return res;
+    });
   };
 }
diff --git a/packages/@expo/cli/src/api/rest/wrapFetchWithProxy.ts b/packages/@expo/cli/src/api/rest/wrapFetchWithProxy.ts
@@ -7,7 +7,8 @@ const debug = require('debug')('expo:api:fetch:proxy') as typeof console.log;
 
 /** Wrap fetch with support for proxies. */
 export function wrapFetchWithProxy(fetchFunction: FetchLike): FetchLike {
-  return async function fetchWithProxy(url, options = {}) {
+  // NOTE(EvanBacon): DO NOT RETURN AN ASYNC WRAPPER. THIS BREAKS LOADING INDICATORS.
+  return function fetchWithProxy(url, options = {}) {
     const proxy = env.HTTP_PROXY;
     if (!options.agent && proxy) {
       debug('Using proxy:', proxy);

diff --git a/packages/@expo/cli/src/utils/downloadAppAsync.ts b/packages/@expo/cli/src/utils/downloadAppAsync.ts
@@ -6,7 +6,6 @@ import { promisify } from 'util';
 
 import { createCachedFetch, fetchAsync } from '../api/rest/client';
 import { FetchLike, ProgressCallback } from '../api/rest/client.types';
-import { wrapFetchWithProgress } from '../api/rest/wrapFetchWithProgress';
 import { ensureDirectoryAsync } from './dir';
 import { CommandError } from './errors';
 import { extractAsync } from './tar';
@@ -40,7 +39,7 @@ async function downloadAsync({
   }
 
   debug(`Downloading ${url} to ${outputPath}`);
-  const res = await wrapFetchWithProgress(fetchInstance)(url, {
+  const res = await fetchInstance(url, {
     timeout: TIMER_DURATION,
     onProgress,
   });

diff --git a/packages/@expo/cli/src/utils/downloadExpoGoAsync.ts b/packages/@expo/cli/src/utils/downloadExpoGoAsync.ts
@@ -1,9 +1,11 @@
 import { getExpoHomeDirectory } from '@expo/config/build/getUserState';
 import path from 'path';
+import ProgressBar from 'progress';
 
 import { getVersionsAsync, SDKVersion } from '../api/getVersions';
 import { downloadAppAsync } from './downloadAppAsync';
 import { CommandError } from './errors';
+import { ora } from './ora';
 import { profile } from './profile';
 import { createProgressBar } from './progress';
 
@@ -44,13 +46,9 @@ export async function downloadExpoGoAsync(
 ): Promise<string> {
   const { getFilePath, versionsKey, shouldExtractResults } = platformSettings[platform];
 
-  const bar = createProgressBar('Downloading the Expo Go app [:bar] :percent :etas', {
-    width: 64,
-    total: 100,
-    clear: true,
-    complete: '=',
-    incomplete: ' ',
-  });
+  const spinner = ora({ text: 'Fetching Expo Go', color: 'white' }).start();
+
+  let bar: ProgressBar | null = null;
 
   if (!url) {
     if (!sdkVersion) {
@@ -84,14 +82,28 @@ export async function downloadExpoGoAsync(
       cacheDirectory: 'expo-go',
       outputPath,
       extract: shouldExtractResults,
-      onProgress({ progress }) {
-        if (bar) {
-          bar.tick(1, progress);
+      onProgress({ progress, total }) {
+        if (progress && total) {
+          if (!bar) {
+            if (spinner.isSpinning) {
+              spinner.stop();
+            }
+            bar = createProgressBar('Downloading the Expo Go app [:bar] :percent :etas', {
+              width: 64,
+              total: 100,
+              // clear: true,
+              complete: '=',
+              incomplete: ' ',
+            });
+          }
+          bar!.update(progress, total);
         }
       },
     });
     return outputPath;
   } finally {
+    spinner.stop();
+    // @ts-expect-error
     bar?.terminate();
   }
 }