Misc source cleanup for Firebase Storage (#5467)

Author: schmidt-sebastian

packages/storage/src/implementation/connection.ts

@@ -15,12 +15,8 @@
  * limitations under the License.
  */
 
-/**
- * Network headers
- */
-export interface Headers {
-  [name: string]: string;
-}
+/** Network headers */
+export type Headers = Record<string, string>;
 
 /**
  * A lightweight wrapper around XMLHttpRequest with a

packages/storage/src/implementation/error.ts

@@ -312,10 +312,7 @@ export function invalidRootOperation(name: string): StorageError {
  * @param format - The format that was not valid.
  * @param message - A message describing the format violation.
  */
-export function invalidFormat(
-  format: string,
-  message: string
-): StorageError {
+export function invalidFormat(format: string, message: string): StorageError {
   return new StorageError(
     StorageErrorCode.INVALID_FORMAT,
     "String does not match format '" + format + "': " + message
@@ -326,10 +323,7 @@ export function invalidFormat(
  * @param message - A message describing the internal error.
  */
 export function unsupportedEnvironment(message: string): StorageError {
-  throw new StorageError(
-    StorageErrorCode.UNSUPPORTED_ENVIRONMENT,
-    message
-  );
+  throw new StorageError(StorageErrorCode.UNSUPPORTED_ENVIRONMENT, message);
 }
 
 /**

packages/storage/src/implementation/request.ts

@@ -28,7 +28,7 @@ import {
   canceled,
   retryLimitExceeded
 } from './error';
-import { RequestInfo } from './requestinfo';
+import { RequestHandler, RequestInfo } from './requestinfo';
 import { isJustDef } from './type';
 import { makeQueryString } from './url';
 import { Headers, Connection, ErrorCode } from './connection';
@@ -48,54 +48,28 @@ export interface Request<T> {
 }
 
 class NetworkRequest<T> implements Request<T> {
-  private url_: string;
-  private method_: string;
-  private headers_: Headers;
-  private body_: string | Blob | Uint8Array | null;
-  private successCodes_: number[];
-  private additionalRetryCodes_: number[];
   private pendingConnection_: Connection | null = null;
   private backoffId_: backoffId | null = null;
   private resolve_!: (value?: T | PromiseLike<T>) => void;
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   private reject_!: (reason?: any) => void;
   private canceled_: boolean = false;
   private appDelete_: boolean = false;
-  private callback_: (p1: Connection, p2: string) => T;
-  private errorCallback_:
-    | ((p1: Connection, p2: StorageError) => StorageError)
-    | null;
-  private progressCallback_: ((p1: number, p2: number) => void) | null;
-  private timeout_: number;
-  private pool_: ConnectionPool;
   promise_: Promise<T>;
 
   constructor(
-    url: string,
-    method: string,
-    headers: Headers,
-    body: string | Blob | Uint8Array | null,
-    successCodes: number[],
-    additionalRetryCodes: number[],
-    callback: (p1: Connection, p2: string) => T,
-    errorCallback:
-      | ((p1: Connection, p2: StorageError) => StorageError)
-      | null,
-    timeout: number,
-    progressCallback: ((p1: number, p2: number) => void) | null,
-    pool: ConnectionPool
+    private url_: string,
+    private method_: string,
+    private headers_: Headers,
+    private body_: string | Blob | Uint8Array | null,
+    private successCodes_: number[],
+    private additionalRetryCodes_: number[],
+    private callback_: RequestHandler<string, T>,
+    private errorCallback_: RequestHandler<StorageError, StorageError> | null,
+    private timeout_: number,
+    private progressCallback_: ((p1: number, p2: number) => void) | null,
+    private pool_: ConnectionPool
   ) {
-    this.url_ = url;
-    this.method_ = method;
-    this.headers_ = headers;
-    this.body_ = body;
-    this.successCodes_ = successCodes.slice();
-    this.additionalRetryCodes_ = additionalRetryCodes.slice();
-    this.callback_ = callback;
-    this.errorCallback_ = errorCallback;
-    this.progressCallback_ = progressCallback;
-    this.timeout_ = timeout;
-    this.pool_ = pool;
     this.promise_ = new Promise((resolve, reject) => {
       this.resolve_ = resolve as (value?: T | PromiseLike<T>) => void;
       this.reject_ = reject;
@@ -107,67 +81,68 @@ class NetworkRequest<T> implements Request<T> {
    * Actually starts the retry loop.
    */
   private start_(): void {
-    const self = this;
-
-    function doTheRequest(
-      backoffCallback: (p1: boolean, ...p2: unknown[]) => void,
+    const doTheRequest: (
+      backoffCallback: (success: boolean, ...p2: unknown[]) => void,
       canceled: boolean
-    ): void {
+    ) => void = (backoffCallback, canceled) => {
       if (canceled) {
         backoffCallback(false, new RequestEndStatus(false, null, true));
         return;
       }
-      const connection = self.pool_.createConnection();
-      self.pendingConnection_ = connection;
+      const connection = this.pool_.createConnection();
+      this.pendingConnection_ = connection;
 
-      function progressListener(progressEvent: ProgressEvent): void {
-        const loaded = progressEvent.loaded;
-        const total = progressEvent.lengthComputable ? progressEvent.total : -1;
-        if (self.progressCallback_ !== null) {
-          self.progressCallback_(loaded, total);
-        }
-      }
-      if (self.progressCallback_ !== null) {
+      const progressListener: (progressEvent: ProgressEvent) => void =
+        progressEvent => {
+          const loaded = progressEvent.loaded;
+          const total = progressEvent.lengthComputable
+            ? progressEvent.total
+            : -1;
+          if (this.progressCallback_ !== null) {
+            this.progressCallback_(loaded, total);
+          }
+        };
+      if (this.progressCallback_ !== null) {
         connection.addUploadProgressListener(progressListener);
       }
 
       // eslint-disable-next-line @typescript-eslint/no-floating-promises
       connection
-        .send(self.url_, self.method_, self.body_, self.headers_)
+        .send(this.url_, this.method_, this.body_, this.headers_)
         .then(() => {
-          if (self.progressCallback_ !== null) {
+          if (this.progressCallback_ !== null) {
             connection.removeUploadProgressListener(progressListener);
           }
-          self.pendingConnection_ = null;
+          this.pendingConnection_ = null;
           const hitServer = connection.getErrorCode() === ErrorCode.NO_ERROR;
           const status = connection.getStatus();
-          if (!hitServer || self.isRetryStatusCode_(status)) {
+          if (!hitServer || this.isRetryStatusCode_(status)) {
             const wasCanceled = connection.getErrorCode() === ErrorCode.ABORT;
             backoffCallback(
               false,
               new RequestEndStatus(false, null, wasCanceled)
             );
             return;
           }
-          const successCode = self.successCodes_.indexOf(status) !== -1;
+          const successCode = this.successCodes_.indexOf(status) !== -1;
           backoffCallback(true, new RequestEndStatus(successCode, connection));
         });
-    }
+    };
 
     /**
      * @param requestWentThrough - True if the request eventually went
      *     through, false if it hit the retry limit or was canceled.
      */
-    function backoffDone(
+    const backoffDone: (
       requestWentThrough: boolean,
       status: RequestEndStatus
-    ): void {
-      const resolve = self.resolve_;
-      const reject = self.reject_;
+    ) => void = (requestWentThrough, status) => {
+      const resolve = this.resolve_;
+      const reject = this.reject_;
       const connection = status.connection as Connection;
       if (status.wasSuccessCode) {
         try {
-          const result = self.callback_(
+          const result = this.callback_(
             connection,
             connection.getResponseText()
           );
@@ -183,22 +158,22 @@ class NetworkRequest<T> implements Request<T> {
         if (connection !== null) {
           const err = unknown();
           err.serverResponse = connection.getResponseText();
-          if (self.errorCallback_) {
-            reject(self.errorCallback_(connection, err));
+          if (this.errorCallback_) {
+            reject(this.errorCallback_(connection, err));
           } else {
             reject(err);
           }
         } else {
           if (status.canceled) {
-            const err = self.appDelete_ ? appDeleted() : canceled();
+            const err = this.appDelete_ ? appDeleted() : canceled();
             reject(err);
           } else {
             const err = retryLimitExceeded();
             reject(err);
           }
         }
       }
-    }
+    };
     if (this.canceled_) {
       backoffDone(false, new RequestEndStatus(false, null, true));
     } else {

packages/storage/src/implementation/requestinfo.ts

@@ -24,14 +24,20 @@ export interface UrlParams {
   [name: string]: string | number;
 }
 
+/**
+ * A function that converts a server response to the API type expected by the
+ * SDK.
+ *
+ * @param I - the type of the backend's network response
+ * @param O - the output response type used by the rest of the SDK.
+ */
+export type RequestHandler<I, O> = (connection: Connection, response: I) => O;
+
 export class RequestInfo<T> {
   urlParams: UrlParams = {};
   headers: Headers = {};
   body: Blob | string | Uint8Array | null = null;
-
-  errorHandler:
-    | ((p1: Connection, p2: StorageError) => StorageError)
-    | null = null;
+  errorHandler: RequestHandler<StorageError, StorageError> | null = null;
 
   /**
    * Called with the current number of bytes uploaded and total size (-1 if not
@@ -51,7 +57,7 @@ export class RequestInfo<T> {
      * Note: The XhrIo passed to this function may be reused after this callback
      * returns. Do not keep a reference to it in any way.
      */
-    public handler: (p1: Connection, p2: string) => T,
+    public handler: RequestHandler<string, T>,
     public timeout: number
   ) {}
 }

packages/storage/src/platform/browser/connection.ts

@@ -50,9 +50,6 @@ export class XhrConnection implements Connection {
     });
   }
 
-  /**
-   * @override
-   */
   send(
     url: string,
     method: string,
@@ -79,19 +76,13 @@ export class XhrConnection implements Connection {
     return this.sendPromise_;
   }
 
-  /**
-   * @override
-   */
   getErrorCode(): ErrorCode {
     if (!this.sent_) {
       throw internalError('cannot .getErrorCode() before sending');
     }
     return this.errorCode_;
   }
 
-  /**
-   * @override
-   */
   getStatus(): number {
     if (!this.sent_) {
       throw internalError('cannot .getStatus() before sending');
@@ -103,43 +94,28 @@ export class XhrConnection implements Connection {
     }
   }
 
-  /**
-   * @override
-   */
   getResponseText(): string {
     if (!this.sent_) {
       throw internalError('cannot .getResponseText() before sending');
     }
     return this.xhr_.responseText;
   }
 
-  /**
-   * Aborts the request.
-   * @override
-   */
+  /** Aborts the request. */
   abort(): void {
     this.xhr_.abort();
   }
 
-  /**
-   * @override
-   */
   getResponseHeader(header: string): string | null {
     return this.xhr_.getResponseHeader(header);
   }
 
-  /**
-   * @override
-   */
   addUploadProgressListener(listener: (p1: ProgressEvent) => void): void {
     if (this.xhr_.upload != null) {
       this.xhr_.upload.addEventListener('progress', listener);
     }
   }
 
-  /**
-   * @override
-   */
   removeUploadProgressListener(listener: (p1: ProgressEvent) => void): void {
     if (this.xhr_.upload != null) {
       this.xhr_.upload.removeEventListener('progress', listener);

packages/storage/src/task.ts

@@ -521,9 +521,7 @@ export class UploadTask {
   /**
    * Equivalent to calling `then(null, onRejected)`.
    */
-  catch<T>(
-    onRejected: (p1: StorageError) => T | Promise<T>
-  ): Promise<T> {
+  catch<T>(onRejected: (p1: StorageError) => T | Promise<T>): Promise<T> {
     return this.then(null, onRejected);
   }
 

packages/storage/test/unit/connection.ts

@@ -19,10 +19,7 @@ import {
   Headers,
   Connection
 } from '../../src/implementation/connection';
-import {
-  StorageError,
-  StorageErrorCode
-} from '../../src/implementation/error';
+import { StorageError, StorageErrorCode } from '../../src/implementation/error';
 
 export type SendHook = (
   connection: TestingConnection,
@@ -67,10 +64,7 @@ export class TestingConnection implements Connection {
     headers?: Headers
   ): Promise<void> {
     if (this.state !== State.START) {
-      throw new StorageError(
-        StorageErrorCode.UNKNOWN,
-        "Can't send again"
-      );
+      throw new StorageError(StorageErrorCode.UNKNOWN, "Can't send again");
     }
 
     this.state = State.SENT;