Address PR comments

Author: hsubox76

packages-exp/functions-exp/.eslintrc.js

@@ -30,8 +30,7 @@ module.exports = {
       'error',
       {
         'packageDir': [path.resolve(__dirname, '../../'), __dirname],
-        devDependencies: true,
-        peerDependencies: true
+        devDependencies: true
       }
     ]
   }

packages-exp/functions-exp/package.json

@@ -33,7 +33,7 @@
   "license": "Apache-2.0",
   "peerDependencies": {
     "@firebase/app-exp": "0.x",
-    "@firebase/app-exp-types": "0.x"
+    "@firebase/app-types-exp": "0.x"
   },
   "devDependencies": {
     "@firebase/app-exp": "0.0.800",

packages-exp/functions-exp/src/context.ts

@@ -14,7 +14,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { _FirebaseApp } from '@firebase/app-types/private';
 import {
   FirebaseMessaging,
   FirebaseMessagingName

packages-exp/functions-exp/src/error.ts

@@ -15,9 +15,10 @@
  * limitations under the License.
  */
 
-import { HttpsError, FunctionsErrorCode } from '@firebase/functions-types-exp';
-import { Serializer } from './serializer';
+import { FunctionsErrorCode } from '@firebase/functions-types-exp';
+import { decode } from './serializer';
 import { HttpResponseBody } from './service';
+import { FirebaseError } from '@firebase/util';
 
 /**
  * Standard error codes for different ways a request can fail, as defined by:
@@ -50,28 +51,20 @@ const errorCodeMap: { [name: string]: FunctionsErrorCode } = {
  * An explicit error that can be thrown from a handler to send an error to the
  * client that called the function.
  */
-export class HttpsErrorImpl extends Error implements HttpsError {
-  /**
-   * A standard error code that will be returned to the client. This also
-   * determines the HTTP status code of the response, as defined in code.proto.
-   */
-  readonly code: FunctionsErrorCode;
-
-  /**
-   * Extra data to be converted to JSON and included in the error response.
-   */
-  readonly details?: unknown;
-
-  constructor(code: FunctionsErrorCode, message?: string, details?: unknown) {
-    super(message);
-
-    // This is a workaround for a bug in TypeScript when extending Error:
-    // tslint:disable-next-line
-    // https://github.com/Microsoft/TypeScript-wiki/blob/master/Breaking-Changes.md#extending-built-ins-like-error-array-and-map-may-no-longer-work
-    Object.setPrototypeOf(this, HttpsErrorImpl.prototype);
-
-    this.code = code;
-    this.details = details;
+export class FunctionsError extends FirebaseError {
+  constructor(
+    /**
+     * A standard error code that will be returned to the client. This also
+     * determines the HTTP status code of the response, as defined in code.proto.
+     */
+    readonly code: FunctionsErrorCode,
+    message?: string,
+    /**
+     * Extra data to be converted to JSON and included in the error response.
+     */
+    readonly details?: unknown
+  ) {
+    super(code, message || '');
   }
 }
 
@@ -124,8 +117,7 @@ function codeForHTTPStatus(status: number): FunctionsErrorCode {
  */
 export function _errorForResponse(
   status: number,
-  bodyJSON: HttpResponseBody | null,
-  serializer: Serializer
+  bodyJSON: HttpResponseBody | null
 ): Error | null {
   let code = codeForHTTPStatus(status);
 
@@ -142,7 +134,7 @@ export function _errorForResponse(
       if (typeof status === 'string') {
         if (!errorCodeMap[status]) {
           // They must've included an unknown error code in the body.
-          return new HttpsErrorImpl('internal', 'internal');
+          return new FunctionsError('internal', 'internal');
         }
         code = errorCodeMap[status];
 
@@ -158,7 +150,7 @@ export function _errorForResponse(
 
       details = errorJSON.details;
       if (details !== undefined) {
-        details = serializer.decode(details as {} | null);
+        details = decode(details as {} | null);
       }
     }
   } catch (e) {
@@ -172,5 +164,5 @@ export function _errorForResponse(
     return null;
   }
 
-  return new HttpsErrorImpl(code, description, details);
+  return new FunctionsError(code, description, details);
 }

packages-exp/functions-exp/src/serializer.test.ts

@@ -15,88 +15,82 @@
  * limitations under the License.
  */
 import { expect } from 'chai';
-import { Serializer } from './serializer';
+import { encode, decode } from './serializer';
 
 describe('Serializer', () => {
-  const serializer = new Serializer();
-
   it('encodes null', () => {
-    expect(serializer.encode(null)).to.be.null;
-    expect(serializer.encode(undefined)).to.be.null;
+    expect(encode(null)).to.be.null;
+    expect(encode(undefined)).to.be.null;
   });
 
   it('decodes null', () => {
-    expect(serializer.decode(null)).to.be.null;
+    expect(decode(null)).to.be.null;
   });
 
   it('encodes int', () => {
-    expect(serializer.encode(1)).to.equal(1);
+    expect(encode(1)).to.equal(1);
     // Number isn't allowed in our own codebase, but we need to test it, in case
     // a user passes one in. There's no reason not to support it, and we don't
     // want to unintentionally encode them as {}.
     // eslint-disable-next-line no-new-wrappers
-    expect(serializer.encode(new Number(1))).to.equal(1);
+    expect(encode(new Number(1))).to.equal(1);
   });
 
   it('decodes int', () => {
-    expect(serializer.decode(1)).to.equal(1);
+    expect(decode(1)).to.equal(1);
   });
 
   it('encodes long', () => {
-    expect(serializer.encode(-9223372036854775000)).to.equal(
-      -9223372036854775000
-    );
+    expect(encode(-9223372036854775000)).to.equal(-9223372036854775000);
   });
 
   it('decodes long', () => {
     expect(
-      serializer.decode({
+      decode({
         '@type': 'type.googleapis.com/google.protobuf.Int64Value',
         value: '-9223372036854775000'
       })
     ).to.equal(-9223372036854775000);
   });
 
   it('encodes unsigned long', () => {
-    expect(serializer.encode(9223372036854800000)).to.equal(
-      9223372036854800000
-    );
+    expect(encode(9223372036854800000)).to.equal(9223372036854800000);
   });
 
   it('decodes unsigned long', () => {
     expect(
-      serializer.decode({
+      decode({
         '@type': 'type.googleapis.com/google.protobuf.UInt64Value',
         value: '9223372036854800000'
       })
     ).to.equal(9223372036854800000);
   });
 
   it('encodes double', () => {
-    expect(serializer.encode(1.2)).to.equal(1.2);
+    expect(encode(1.2)).to.equal(1.2);
   });
 
   it('decodes double', () => {
-    expect(serializer.decode(1.2)).to.equal(1.2);
+    expect(decode(1.2)).to.equal(1.2);
   });
 
   it('encodes string', () => {
-    expect(serializer.encode('hello')).to.equal('hello');
+    expect(encode('hello')).to.equal('hello');
   });
 
   it('decodes string', () => {
-    expect(serializer.decode('hello')).to.equal('hello');
+    expect(decode('hello')).to.equal('hello');
   });
 
   // TODO(klimt): Make this test more interesting once we have a complex type
   // that can be created in JavaScript.
   it('encodes array', () => {
-    expect(serializer.encode([1, '2', [3, 4]])).to.deep.equal([1, '2', [3, 4]]);
+    expect(encode([1, '2', [3, 4]])).to.deep.equal([1, '2', [3, 4]]);
   });
 
   it('decodes array', () => {
     expect(
-      serializer.decode([
+      decode([
         1,
         '2',
         [
@@ -114,7 +108,7 @@ describe('Serializer', () => {
   // that can be created in JavaScript.
   it('encodes object', () => {
     expect(
-      serializer.encode({
+      encode({
         foo: 1,
         bar: 'hello',
         baz: [1, 2, 3]
@@ -128,7 +122,7 @@ describe('Serializer', () => {
 
   it('decodes object', () => {
     expect(
-      serializer.decode({
+      decode({
         foo: 1,
         bar: 'hello',
         baz: [
@@ -148,12 +142,12 @@ describe('Serializer', () => {
   });
 
   it('fails to encode NaN', () => {
-    expect(() => serializer.encode(NaN)).to.throw();
+    expect(() => encode(NaN)).to.throw();
   });
 
   it('fails to decode unknown type', () => {
     expect(() =>
-      serializer.decode({
+      decode({
         '@type': 'unknown',
         value: 'should be ignored'
       })

packages-exp/functions-exp/src/serializer.ts

@@ -33,71 +33,74 @@ function mapValues(
 }
 
 /**
+ * Takes data and encodes it in a JSON-friendly way, such that types such as
+ * Date are preserved.
  * @internal
+ * @param data - Data to encode.
  */
-export class Serializer {
-  // Takes data and encodes it in a JSON-friendly way, such that types such as
-  // Date are preserved.
-  encode(data: unknown): unknown {
-    if (data == null) {
-      return null;
-    }
-    if (data instanceof Number) {
-      data = data.valueOf();
-    }
-    if (typeof data === 'number' && isFinite(data)) {
-      // Any number in JS is safe to put directly in JSON and parse as a double
-      // without any loss of precision.
-      return data;
-    }
-    if (data === true || data === false) {
-      return data;
-    }
-    if (Object.prototype.toString.call(data) === '[object String]') {
-      return data;
-    }
-    if (Array.isArray(data)) {
-      return data.map(x => this.encode(x));
-    }
-    if (typeof data === 'function' || typeof data === 'object') {
-      return mapValues(data as object, x => this.encode(x));
-    }
-    // If we got this far, the data is not encodable.
-    throw new Error('Data cannot be encoded in JSON: ' + data);
+export function encode(data: unknown): unknown {
+  if (data == null) {
+    return null;
   }
+  if (data instanceof Number) {
+    data = data.valueOf();
+  }
+  if (typeof data === 'number' && isFinite(data)) {
+    // Any number in JS is safe to put directly in JSON and parse as a double
+    // without any loss of precision.
+    return data;
+  }
+  if (data === true || data === false) {
+    return data;
+  }
+  if (Object.prototype.toString.call(data) === '[object String]') {
+    return data;
+  }
+  if (Array.isArray(data)) {
+    return data.map(x => encode(x));
+  }
+  if (typeof data === 'function' || typeof data === 'object') {
+    return mapValues(data as object, x => encode(x));
+  }
+  // If we got this far, the data is not encodable.
+  throw new Error('Data cannot be encoded in JSON: ' + data);
+}
 
-  // Takes data that's been encoded in a JSON-friendly form and returns a form
-  // with richer datatypes, such as Dates, etc.
-  decode(json: unknown): unknown {
-    if (json == null) {
-      return json;
-    }
-    if ((json as { [key: string]: unknown })['@type']) {
-      switch ((json as { [key: string]: unknown })['@type']) {
-        case LONG_TYPE:
-        // Fall through and handle this the same as unsigned.
-        case UNSIGNED_LONG_TYPE: {
-          // Technically, this could work return a valid number for malformed
-          // data if there was a number followed by garbage. But it's just not
-          // worth all the extra code to detect that case.
-          const value = Number((json as { [key: string]: unknown })['value']);
-          if (isNaN(value)) {
-            throw new Error('Data cannot be decoded from JSON: ' + json);
-          }
-          return value;
-        }
-        default: {
+/**
+ * Takes data that's been encoded in a JSON-friendly form and returns a form
+ * with richer datatypes, such as Dates, etc.
+ * @internal
+ * @param json - JSON to convert.
+ */
+export function decode(json: unknown): unknown {
+  if (json == null) {
+    return json;
+  }
+  if ((json as { [key: string]: unknown })['@type']) {
+    switch ((json as { [key: string]: unknown })['@type']) {
+      case LONG_TYPE:
+      // Fall through and handle this the same as unsigned.
+      case UNSIGNED_LONG_TYPE: {
+        // Technically, this could work return a valid number for malformed
+        // data if there was a number followed by garbage. But it's just not
+        // worth all the extra code to detect that case.
+        const value = Number((json as { [key: string]: unknown })['value']);
+        if (isNaN(value)) {
           throw new Error('Data cannot be decoded from JSON: ' + json);
         }
+        return value;
+      }
+      default: {
+        throw new Error('Data cannot be decoded from JSON: ' + json);
       }
     }
-    if (Array.isArray(json)) {
-      return json.map(x => this.decode(x));
-    }
-    if (typeof json === 'function' || typeof json === 'object') {
-      return mapValues(json as object, x => this.decode(x as {} | null));
-    }
-    // Anything else is safe to return.
-    return json;
   }
+  if (Array.isArray(json)) {
+    return json.map(x => decode(x));
+  }
+  if (typeof json === 'function' || typeof json === 'object') {
+    return mapValues(json as object, x => decode(x as {} | null));
+  }
+  // Anything else is safe to return.
+  return json;
 }