Address comments and feedback

src/remote-config/remote-config-api.ts

@@ -375,12 +375,17 @@ export interface InitServerTemplateOptions extends GetServerTemplateOptions {
    * example, customers can reduce initialization latency by pre-fetching and
    * caching template data and then using this option to initialize the SDK with
    * that data.
-   * The template can be initialized with either a {@link ServerTemplateData}
-   * object or a JSON string.
    */
-  template?: ServerTemplateData|string,
+  template?: ServerTemplateDataType,
 }
 
+/**
+ * Represents the type of a Remote Config server template that can be set on
+ * {@link ServerTemplate}. This can either be a {@link ServerTemplateData} object
+ * or a template JSON string.
+ */
+export type ServerTemplateDataType = ServerTemplateData | string;
+
 /**
  * Represents a stateful abstraction for a Remote Config server template.
  */
@@ -400,7 +405,7 @@ export interface ServerTemplate {
    * Sets and caches a {@link ServerTemplateData} or a JSON string representing
    * the server template
    */
-  set(template: ServerTemplateData | string): void;
+  set(template: ServerTemplateDataType): void;
 
   /**
    * Returns a JSON representation of {@link ServerTemplateData}

src/remote-config/remote-config.ts

@@ -40,6 +40,7 @@ import {
   DefaultConfig,
   GetServerTemplateOptions,
   InitServerTemplateOptions,
+  ServerTemplateDataType,
 } from './remote-config-api';
 import { isString } from 'lodash';
 
@@ -326,19 +327,22 @@ class ServerTemplateImpl implements ServerTemplate {
    * Takes in either a {@link ServerTemplateData} or a JSON string
    * representing the template, parses it, and caches it.
    */
-  public set(template: ServerTemplateData | string): void {
+  public set(template: ServerTemplateDataType): void {
+    let parsed;
     if (isString(template)) {
       try {
-        this.cache = new ServerTemplateDataImpl(JSON.parse(template));
+        parsed = JSON.parse(template);
       } catch (e) {
+        // Transforms JSON parse errors to Firebase error.
         throw new FirebaseRemoteConfigError(
           'invalid-argument',
-          `Failed to parse the JSON string: ${template}. ` + e
-        );
+          `Failed to parse the JSON string: ${template}. ` + e);
       }
     } else {
-      this.cache = template;
+      parsed = template;
     }
+    // Throws template parse errors.
+    this.cache = new ServerTemplateDataImpl(parsed);
   }
 
   /**

test/unit/remote-config/remote-config.spec.ts

@@ -955,10 +955,6 @@ describe('RemoteConfig', () => {
     });
 
     describe('set', () => {
-      const INVALID_PARAMETERS: any[] = [null, '', 'abc', 1, true, []];
-      const INVALID_CONDITIONS: any[] = [null, '', 'abc', 1, true, {}];
-      let sourceTemplate = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE);
-
       it('should set template when passed', () => {
         const template = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE) as ServerTemplateData;
         template.parameters = {
@@ -970,13 +966,14 @@ describe('RemoteConfig', () => {
             valueType: 'STRING'
           }
         };
+        template.version = {
+          ...deepCopy(VERSION_INFO),
+          updateTime: new Date(VERSION_INFO.updateTime).toUTCString()
+        } as Version;
         const initializedTemplate = remoteConfig.initServerTemplate();
         initializedTemplate.set(template);
         const parsed = initializedTemplate.toJSON();
-        const expectedVersion = deepCopy(VERSION_INFO);
-        expectedVersion.updateTime = new Date(expectedVersion.updateTime).toUTCString();
-        template.version = expectedVersion as Version;
-        expect(parsed).deep.equals(deepCopy(template));
+        expect(parsed).deep.equals(template);
       });
 
       it('should set and instantiates template when json string is passed', () => {
@@ -990,44 +987,54 @@ describe('RemoteConfig', () => {
             valueType: 'STRING'
           }
         };
+        template.version = {
+          ...deepCopy(VERSION_INFO),
+          updateTime: new Date(VERSION_INFO.updateTime).toUTCString()
+        } as Version;
         const templateJson = JSON.stringify(template);
         const initializedTemplate = remoteConfig.initServerTemplate();
         initializedTemplate.set(templateJson);
         const parsed = initializedTemplate.toJSON();
-        const expectedVersion = deepCopy(VERSION_INFO);
-        expectedVersion.updateTime = new Date(expectedVersion.updateTime).toUTCString();
-        template.version = expectedVersion as Version;
-        expect(parsed).deep.equals(deepCopy(template));
+        expect(parsed).deep.equals(template);
       });
 
-      it('should throw if template is an invalid JSON', () => {
+      describe('should throw error if there are any JSON or tempalte parsing errors', () => {
+        const INVALID_PARAMETERS: any[] = [null, '', 'abc', 1, true, []];
+        const INVALID_CONDITIONS: any[] = [null, '', 'abc', 1, true, {}];
+
+        let sourceTemplate = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE);
         const jsonString = '{invalidJson: null}';
-        const initializedTemplate = remoteConfig.initServerTemplate();
-        expect(() => initializedTemplate.set(jsonString))
-          .to.throw(/Failed to parse the JSON string: ([\D\w]*)\./);
-      });
-
-      INVALID_PARAMETERS.forEach((invalidParameter) => {
-        sourceTemplate.parameters = invalidParameter;
-        it(`should throw if the parameters is ${JSON.stringify(invalidParameter)}`, () => {
-          const jsonString = JSON.stringify(sourceTemplate);
-          const initializedTemplate = remoteConfig.initServerTemplate();
-          expect(() => initializedTemplate.set(jsonString))
+        it('should throw if template is an invalid JSON', () => {
+          expect(() => remoteConfig.initServerTemplate({ template: jsonString }))
             .to.throw(/Failed to parse the JSON string: ([\D\w]*)\./);
         });
-      });
-
-      sourceTemplate = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE);
-      INVALID_CONDITIONS.forEach((invalidConditions) => {
-        sourceTemplate.conditions = invalidConditions;
-        it(`should throw if the conditions is ${JSON.stringify(invalidConditions)}`, () => {
+
+        INVALID_PARAMETERS.forEach((invalidParameter) => {
+          sourceTemplate.parameters = invalidParameter;
           const jsonString = JSON.stringify(sourceTemplate);
-          const initializedTemplate = remoteConfig.initServerTemplate();
-          expect(() => initializedTemplate.set(jsonString))
-            .to.throw(/Failed to parse the JSON string: ([\D\w]*)\./);
+          it(`should throw if the template is invalid - parameters is ${JSON.stringify(invalidParameter)}`, () => {
+            expect(() => remoteConfig.initServerTemplate({ template: jsonString }))
+              .to.throw('Remote Config parameters must be a non-null object');
+          });
+        });
+
+        sourceTemplate = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE);
+        INVALID_CONDITIONS.forEach((invalidConditions) => {
+          sourceTemplate.conditions = invalidConditions;
+          const jsonString = JSON.stringify(sourceTemplate);
+          it(`should throw if the template is invalid - conditions is ${JSON.stringify(invalidConditions)}`, () => {
+            expect(() => remoteConfig.initServerTemplate({ template: jsonString }))
+              .to.throw('Remote Config conditions must be an array');
+          });
         });
       });
 
+      it('should throw if template is an invalid JSON', () => {
+        const jsonString = '{invalidJson: null}';
+        const initializedTemplate = remoteConfig.initServerTemplate();
+        expect(() => initializedTemplate.set(jsonString))
+          .to.throw(/Failed to parse the JSON string: ([\D\w]*)\./);
+      });
     });
 
     describe('evaluate', () => {