Address some PR comments

Author: hsubox76

packages/storage/compat/reference.ts

@@ -6,7 +6,7 @@
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *   http://www.apache.org/licenses/LICENSE-2.0
+ *  http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -42,7 +42,7 @@ import { ListOptions } from '../src/list';
 import { UploadTaskCompat } from './task';
 import { ListResultCompat } from './list';
 import { StorageServiceCompat } from './service';
-import * as errorsExports from '../src/implementation/error';
+import { invalidRootOperation } from '../src/implementation/error';
 
 export class ReferenceCompat implements types.Reference {
   constructor(private readonly delegate: Reference) {}
@@ -62,8 +62,8 @@ export class ReferenceCompat implements types.Reference {
 
   /**
    * @return A reference to the object obtained by
-   *     appending childPath, removing any duplicate, beginning, or trailing
-   *     slashes.
+   * appending childPath, removing any duplicate, beginning, or trailing
+   * slashes.
    */
   child(childPath: string): types.Reference {
     validate('child', [stringSpec()], arguments);
@@ -77,10 +77,9 @@ export class ReferenceCompat implements types.Reference {
 
   /**
    * @return A reference to the parent of the
-   *     current object, or null if the current object is the root.
+   * current object, or null if the current object is the root.
    */
   get parent(): types.Reference | null {
-    validate('parent', [], arguments);
     const reference = getParent(this.delegate);
     if (reference == null) {
       return null;
@@ -92,25 +91,26 @@ export class ReferenceCompat implements types.Reference {
    * Uploads a blob to this object's location.
    * @param data The blob to upload.
    * @return An UploadTask that lets you control and
-   *     observe the upload.
+   * observe the upload.
    */
   put(
     data: Blob | Uint8Array | ArrayBuffer,
     metadata?: Metadata
   ): types.UploadTask {
     validate('put', [uploadDataSpec(), metadataSpec(true)], arguments);
-    this.throwIfRoot_('put');
+    this._throwIfRoot('put');
     return new UploadTaskCompat(
       uploadBytes(this.delegate, data, metadata),
       this
     );
   }
+
   /**
    * Uploads a string to this object's location.
    * @param value The string to upload.
    * @param format The format of the string to upload.
    * @return An UploadTask that lets you control and
-   *     observe the upload.
+   * observe the upload.
    */
   putString(
     value: string,
@@ -122,7 +122,7 @@ export class ReferenceCompat implements types.Reference {
       [stringSpec(), stringSpec(formatValidator, true), metadataSpec(true)],
       arguments
     );
-    this.throwIfRoot_('putString');
+    this._throwIfRoot('putString');
     return new UploadTaskCompat(
       uploadString(this.delegate, value, format, metadata),
       this
@@ -142,9 +142,9 @@ export class ReferenceCompat implements types.Reference {
    * too many results.
    *
    * @return A Promise that resolves with all the items and prefixes under
-   *      the current storage reference. `prefixes` contains references to
-   *      sub-directories and `items` contains references to objects in this
-   *      folder. `nextPageToken` is never returned.
+   *  the current storage reference. `prefixes` contains references to
+   *  sub-directories and `items` contains references to objects in this
+   *  folder. `nextPageToken` is never returned.
    */
   listAll(): Promise<types.ListResult> {
     validate('listAll', [], arguments);
@@ -169,9 +169,9 @@ export class ReferenceCompat implements types.Reference {
    *
    * @param options See ListOptions for details.
    * @return A Promise that resolves with the items and prefixes.
-   *      `prefixes` contains references to sub-folders and `items`
-   *      contains references to objects in this folder. `nextPageToken`
-   *      can be used to get the rest of the results.
+   * `prefixes` contains references to sub-folders and `items`
+   * contains references to objects in this folder. `nextPageToken`
+   * can be used to get the rest of the results.
    */
   list(options?: ListOptions | null): Promise<types.ListResult> {
     validate('list', [listOptionSpec(true)], arguments);
@@ -181,9 +181,9 @@ export class ReferenceCompat implements types.Reference {
   }
 
   /**
-   *     A promise that resolves with the metadata for this object. If this
-   *     object doesn't exist or metadata cannot be retreived, the promise is
-   *     rejected.
+   * A promise that resolves with the metadata for this object. If this
+   * object doesn't exist or metadata cannot be retreived, the promise is
+   * rejected.
    */
   getMetadata(): Promise<Metadata> {
     validate('getMetadata', [], arguments);
@@ -193,11 +193,11 @@ export class ReferenceCompat implements types.Reference {
   /**
    * Updates the metadata for this object.
    * @param metadata The new metadata for the object.
-   *     Only values that have been explicitly set will be changed. Explicitly
-   *     setting a value to null will remove the metadata.
+   * Only values that have been explicitly set will be changed. Explicitly
+   * setting a value to null will remove the metadata.
    * @return A promise that resolves
-   *     with the new metadata for this object.
-   *     @see firebaseStorage.Reference.prototype.getMetadata
+   * with the new metadata for this object.
+   * @see firebaseStorage.Reference.prototype.getMetadata
    */
   updateMetadata(metadata: Metadata): Promise<Metadata> {
     validate('updateMetadata', [metadataSpec()], arguments);
@@ -206,7 +206,7 @@ export class ReferenceCompat implements types.Reference {
 
   /**
    * @return A promise that resolves with the download
-   *     URL for this object.
+   * URL for this object.
    */
   getDownloadURL(): Promise<string> {
     validate('getDownloadURL', [], arguments);
@@ -219,13 +219,13 @@ export class ReferenceCompat implements types.Reference {
    */
   delete(): Promise<void> {
     validate('delete', [], arguments);
-    this.throwIfRoot_('delete');
+    this._throwIfRoot('delete');
     return deleteObject(this.delegate);
   }
 
-  private throwIfRoot_(name: string): void {
+  private _throwIfRoot(name: string): void {
     if (this.delegate.location.path === '') {
-      throw errorsExports.invalidRootOperation(name);
+      throw invalidRootOperation(name);
     }
   }
 }

packages/storage/compat/service.ts

@@ -48,8 +48,6 @@ export function pathValidator(path: unknown): void {
 /**
  * A service that provides firebaseStorage.Reference instances.
  * @param opt_url gs:// url to a custom Storage Bucket
- *
- * @struct
  */
 export class StorageServiceCompat implements types.FirebaseStorage {
   constructor(
@@ -104,20 +102,20 @@ export class StorageServiceCompat implements types.FirebaseStorage {
 }
 
 /**
- * @struct
+ * @internal
  */
 export class ServiceInternals {
-  service_: StorageServiceCompat;
+  _service: StorageServiceCompat;
 
   constructor(service: StorageServiceCompat) {
-    this.service_ = service;
+    this._service = service;
   }
 
   /**
    * Called when the associated app is deleted.
    */
   delete(): Promise<void> {
-    this.service_.delegate.deleteApp();
+    this._service.delegate.deleteApp();
     return Promise.resolve();
   }
 }

packages/storage/rollup.config.exp.js

@@ -24,6 +24,7 @@ import path from 'path';
 const deps = Object.keys(
   Object.assign({}, pkg.peerDependencies, pkg.dependencies)
 ).concat('@firebase/app-exp');
+
 /**
  * ES5 Builds
  */

packages/storage/src/implementation/args.ts

@@ -14,10 +14,16 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import * as errorsExports from './error';
-import * as MetadataUtils from './metadata';
-import * as ListOptionsUtils from './list';
-import * as type from './type';
+import { invalidArgumentCount, invalidArgument } from './error';
+import { metadataValidator } from './metadata';
+import { listOptionsValidator } from './list';
+import {
+  isJustDef,
+  isNativeBlobDefined,
+  isString,
+  isNumber,
+  isFunction
+} from './type';
 
 /**
  * @param name Name of the function.
@@ -40,37 +46,29 @@ export function validate(
   }
   const validLength = minArgs <= passed.length && passed.length <= maxArgs;
   if (!validLength) {
-    throw errorsExports.invalidArgumentCount(
-      minArgs,
-      maxArgs,
-      name,
-      passed.length
-    );
+    throw invalidArgumentCount(minArgs, maxArgs, name, passed.length);
   }
   for (let i = 0; i < passed.length; i++) {
     try {
       specs[i].validator(passed[i]);
     } catch (e) {
       if (e instanceof Error) {
-        throw errorsExports.invalidArgument(i, name, e.message);
+        throw invalidArgument(i, name, e.message);
       } else {
-        throw errorsExports.invalidArgument(i, name, e);
+        throw invalidArgument(i, name, e);
       }
     }
   }
 }
 
-/**
- * @struct
- */
 export class ArgSpec {
   validator: (p1: unknown) => void;
   optional: boolean;
 
   constructor(validator: (p1: unknown) => void, optional?: boolean) {
     const self = this;
     this.validator = function (p: unknown) {
-      if (self.optional && !type.isJustDef(p)) {
+      if (self.optional && !isJustDef(p)) {
         return;
       }
       validator(p);
@@ -94,7 +92,7 @@ export function stringSpec(
   optional?: boolean
 ): ArgSpec {
   function stringValidator(p: unknown): void {
-    if (!type.isString(p)) {
+    if (!isString(p)) {
       throw 'Expected string.';
     }
   }
@@ -112,25 +110,25 @@ export function uploadDataSpec(): ArgSpec {
     const valid =
       p instanceof Uint8Array ||
       p instanceof ArrayBuffer ||
-      (type.isNativeBlobDefined() && p instanceof Blob);
+      (isNativeBlobDefined() && p instanceof Blob);
     if (!valid) {
-      throw 'Expected Blob or File.';
+      throw invalidArgument('Expected Blob or File.');
     }
   }
   return new ArgSpec(validator);
 }
 
 export function metadataSpec(optional?: boolean): ArgSpec {
-  return new ArgSpec(MetadataUtils.metadataValidator, optional);
+  return new ArgSpec(metadataValidator, optional);
 }
 
 export function listOptionSpec(optional?: boolean): ArgSpec {
-  return new ArgSpec(ListOptionsUtils.listOptionsValidator, optional);
+  return new ArgSpec(listOptionsValidator, optional);
 }
 
 export function nonNegativeNumberSpec(): ArgSpec {
   function validator(p: unknown): void {
-    const valid = type.isNumber(p) && p >= 0;
+    const valid = isNumber(p) && p >= 0;
     if (!valid) {
       throw 'Expected a number 0 or greater.';
     }
@@ -143,7 +141,7 @@ export function looseObjectSpec(
   optional?: boolean
 ): ArgSpec {
   function isLooseObjectValidator(p: unknown): void {
-    const isLooseObject = p === null || (type.isDef(p) && p instanceof Object);
+    const isLooseObject = p === null || (p != null && p instanceof Object);
     if (!isLooseObject) {
       throw 'Expected an Object.';
     }
@@ -156,7 +154,7 @@ export function looseObjectSpec(
 
 export function nullFunctionSpec(optional?: boolean): ArgSpec {
   function validator(p: unknown): void {
-    const valid = p === null || type.isFunction(p);
+    const valid = p === null || isFunction(p);
     if (!valid) {
       throw 'Expected a Function.';
     }

packages/storage/src/implementation/error.ts

@@ -239,15 +239,30 @@ export function noDownloadURL(): FirebaseStorageError {
   );
 }
 
+export function invalidArgument(message: string): FirebaseStorageError;
 export function invalidArgument(
-  index: number,
+  index: number | string,
   fnName: string,
   message: string
+): FirebaseStorageError;
+export function invalidArgument(
+  indexOrMessage: number | string,
+  fnName?: string,
+  message?: string
 ): FirebaseStorageError {
-  return new FirebaseStorageError(
-    Code.INVALID_ARGUMENT,
-    'Invalid argument in `' + fnName + '` at index ' + index + ': ' + message
-  );
+  if (typeof indexOrMessage === 'string') {
+    return new FirebaseStorageError(Code.INVALID_ARGUMENT, indexOrMessage);
+  } else {
+    return new FirebaseStorageError(
+      Code.INVALID_ARGUMENT,
+      'Invalid argument in `' +
+        fnName +
+        '` at index ' +
+        indexOrMessage +
+        ': ' +
+        message
+    );
+  }
 }
 
 export function invalidArgumentCount(

packages/storage/src/implementation/failrequest.ts

@@ -19,8 +19,6 @@ import { Request } from './request';
 
 /**
  * A request whose promise always fails.
- * @struct
- * @template T
  */
 export class FailRequest<T> implements Request<T> {
   promise_: Promise<T>;