Address PR comments

Author: hsubox76

.changeset/tricky-seahorses-look.md

@@ -3,4 +3,5 @@
 '@firebase/storage': minor
 ---
 
-Add `storage().useEmulator()` method allowing users to set a storage emulator host and port.
+Add `storage().useEmulator()` method to enable emulator mode for storage, allowing users
+to set a storage emulator host and port.

packages/storage/compat/service.ts

@@ -75,7 +75,7 @@ export class StorageServiceCompat implements types.FirebaseStorage {
       );
     }
     try {
-      Location.makeFromUrl(url, this._delegate.emulatorOrigin);
+      Location.makeFromUrl(url, this._delegate.host);
     } catch (e) {
       throw invalidArgument(
         'refFromUrl() expected a valid full URL but got an invalid one.'

packages/storage/src/implementation/metadata.ts

@@ -156,7 +156,7 @@ export function fromResourceString(
 export function downloadUrlFromResourceString(
   metadata: Metadata,
   resourceString: string,
-  emulatorOrigin?: string
+  host?: string
 ): string | null {
   const obj = jsonObjectOrNull(resourceString);
   if (obj === null) {
@@ -177,7 +177,7 @@ export function downloadUrlFromResourceString(
     const bucket: string = metadata['bucket'] as string;
     const path: string = metadata['fullPath'] as string;
     const urlPart = '/b/' + encode(bucket) + '/o/' + encode(path);
-    const base = makeUrl(urlPart, emulatorOrigin);
+    const base = makeUrl(urlPart, host);
     const queryString = makeQueryString({
       alt: 'media',
       token

packages/storage/src/implementation/requests.ts

@@ -89,7 +89,7 @@ export function downloadUrlHandler(
     return downloadUrlFromResourceString(
       metadata as Metadata,
       text,
-      service.emulatorOrigin
+      service.host
     );
   }
   return handler;
@@ -147,7 +147,7 @@ export function getMetadata(
   mappings: Mappings
 ): RequestInfo<Metadata> {
   const urlPart = location.fullServerUrl();
-  const url = makeUrl(urlPart, service.emulatorOrigin);
+  const url = makeUrl(urlPart, service.host);
   const method = 'GET';
   const timeout = service.maxOperationRetryTime;
   const requestInfo = new RequestInfo(
@@ -183,7 +183,7 @@ export function list(
     urlParams['maxResults'] = maxResults;
   }
   const urlPart = location.bucketOnlyServerUrl();
-  const url = makeUrl(urlPart, service.emulatorOrigin);
+  const url = makeUrl(urlPart, service.host);
   const method = 'GET';
   const timeout = service.maxOperationRetryTime;
   const requestInfo = new RequestInfo(
@@ -203,7 +203,7 @@ export function getDownloadUrl(
   mappings: Mappings
 ): RequestInfo<string | null> {
   const urlPart = location.fullServerUrl();
-  const url = makeUrl(urlPart, service.emulatorOrigin);
+  const url = makeUrl(urlPart, service.host);
   const method = 'GET';
   const timeout = service.maxOperationRetryTime;
   const requestInfo = new RequestInfo(
@@ -223,7 +223,7 @@ export function updateMetadata(
   mappings: Mappings
 ): RequestInfo<Metadata> {
   const urlPart = location.fullServerUrl();
-  const url = makeUrl(urlPart, service.emulatorOrigin);
+  const url = makeUrl(urlPart, service.host);
   const method = 'PATCH';
   const body = toResourceString(metadata, mappings);
   const headers = { 'Content-Type': 'application/json; charset=utf-8' };
@@ -245,7 +245,7 @@ export function deleteObject(
   location: Location
 ): RequestInfo<void> {
   const urlPart = location.fullServerUrl();
-  const url = makeUrl(urlPart, service.emulatorOrigin);
+  const url = makeUrl(urlPart, service.host);
   const method = 'DELETE';
   const timeout = service.maxOperationRetryTime;
 
@@ -325,7 +325,7 @@ export function multipartUpload(
     throw cannotSliceBlob();
   }
   const urlParams: UrlParams = { name: metadata_['fullPath']! };
-  const url = makeUrl(urlPart, service.emulatorOrigin);
+  const url = makeUrl(urlPart, service.host);
   const method = 'POST';
   const timeout = service.maxUploadRetryTime;
   const requestInfo = new RequestInfo(
@@ -385,7 +385,7 @@ export function createResumableUpload(
   const urlPart = location.bucketOnlyServerUrl();
   const metadataForUpload = metadataForUpload_(location, blob, metadata);
   const urlParams: UrlParams = { name: metadataForUpload['fullPath']! };
-  const url = makeUrl(urlPart, service.emulatorOrigin);
+  const url = makeUrl(urlPart, service.host);
   const method = 'POST';
   const headers = {
     'X-Goog-Upload-Protocol': 'resumable',

packages/storage/src/reference.ts

@@ -60,7 +60,7 @@ export class Reference {
     if (location instanceof Location) {
       this._location = location;
     } else {
-      this._location = Location.makeFromUrl(location, _service.emulatorOrigin);
+      this._location = Location.makeFromUrl(location, _service.host);
     }
   }
 

packages/storage/src/service.ts

@@ -30,6 +30,7 @@ import {
 } from '@firebase/app-types-exp';
 import {
   CONFIG_STORAGE_BUCKET_KEY,
+  DEFAULT_HOST,
   DEFAULT_MAX_OPERATION_RETRY_TIME,
   DEFAULT_MAX_UPLOAD_RETRY_TIME
 } from '../src/implementation/constants';
@@ -41,6 +42,7 @@ import {
 import { validateNumber } from './implementation/type';
 
 export function isUrl(path?: string): boolean {
+  console.log(path);
   return /^[A-Za-z]+:\/\//.test(path as string);
 }
 
@@ -135,7 +137,7 @@ export function useStorageEmulator(
   host: string,
   port: number
 ): void {
-  storage.emulatorOrigin = `http://${host}:${port}`;
+  storage.host = `http://${host}:${port}`;
 }
 
 /**
@@ -145,7 +147,7 @@ export function useStorageEmulator(
  */
 export class StorageService implements _FirebaseService {
   _bucket: Location | null = null;
-  private _emulatorOrigin?: string;
+  private _host: string = DEFAULT_HOST;
   protected readonly _appId: string | null = null;
   private readonly _requests: Set<Request<unknown>>;
   private _deleted: boolean = false;
@@ -172,16 +174,16 @@ export class StorageService implements _FirebaseService {
     }
   }
 
-  get emulatorOrigin(): string | undefined {
-    return this._emulatorOrigin;
+  get host(): string {
+    return this._host;
   }
 
   /**
-   * Set emulator origin string for this service.
-   * @param origin Origin string in the form of http://[host]:[port]
+   * Set host:port string for this service.
+   * @param origin - Origin string in the form of http://[host]:[port]
    */
-  set emulatorOrigin(origin: string | undefined) {
-    this._emulatorOrigin = origin;
+  set host(origin: string) {
+    this._host = origin;
     if (this._url != null) {
       this._bucket = Location.makeFromBucketSpec(this._url, origin);
     } else {

packages/storage/test/unit/service.compat.test.ts

@@ -191,9 +191,7 @@ describe('Firebase Storage > Service', () => {
         testShared.makePool(newSend)
       );
       service.useEmulator('test.host.org', 1234);
-      expect(service._delegate.emulatorOrigin).to.equal(
-        'http://test.host.org:1234'
-      );
+      expect(service._delegate.host).to.equal('http://test.host.org:1234');
       void service.ref('test.png').getDownloadURL();
     });
   });

packages/storage/test/unit/service.exp.test.ts

@@ -240,7 +240,7 @@ GOOG4-RSA-SHA256`
         testShared.makePool(newSend)
       );
       useStorageEmulator(service, 'test.host.org', 1234);
-      expect(service.emulatorOrigin).to.equal('http://test.host.org:1234');
+      expect(service.host).to.equal('http://test.host.org:1234');
       void getDownloadURL(ref(service, 'test.png'));
     });
   });