Addressed comments

common/api-review/data-connect.api.md

@@ -46,7 +46,7 @@ export interface ConnectorConfig {
 
 // @public (undocumented)
 export class DataConnect {
-    constructor(app: FirebaseApp, dataConnectOptions: DataConnectOptions, authProvider: Provider<FirebaseAuthInternalName>);
+    constructor(app: FirebaseApp, dataConnectOptions: DataConnectOptions, _authProvider: Provider<FirebaseAuthInternalName>);
     // (undocumented)
     readonly app: FirebaseApp;
     // (undocumented)
@@ -59,7 +59,7 @@ export class DataConnect {
     isEmulator: boolean;
     // (undocumented)
     setInitialized(): void;
-    }
+}
 
 // @public (undocumented)
 export interface DataConnectOptions extends ConnectorConfig {
@@ -115,7 +115,7 @@ export const FIREBASE_DATA_CONNECT_EMULATOR_HOST_VAR = "FIREBASE_DATA_CONNECT_EM
 
 // @public (undocumented)
 export class FirebaseAuthProvider implements AuthTokenProvider {
-    constructor(appName: string, options: FirebaseOptions, authProvider_: Provider<FirebaseAuthInternalName>);
+    constructor(_appName: string, _options: FirebaseOptions, _authProvider: Provider<FirebaseAuthInternalName>);
     // (undocumented)
     addTokenChangeListener(listener: AuthTokenListener): void;
     // (undocumented)
@@ -130,28 +130,31 @@ export function getDataConnect(options: ConnectorConfig): DataConnect;
 // @public (undocumented)
 export function getDataConnect(app: FirebaseApp, options: ConnectorConfig): DataConnect;
 
+// @public (undocumented)
+export const MUTATION_STR = "mutation";
+
 // @public (undocumented)
 export class MutationManager {
-    constructor(transport: DataConnectTransport);
+    constructor(_transport: DataConnectTransport);
     // (undocumented)
-    executeMutation<Response, Variables>(mutationRef: MutationRef<Response, Variables>): MutationPromise<Response, Variables>;
-    }
+    executeMutation<Data, Variables>(mutationRef: MutationRef<Data, Variables>): MutationPromise<Data, Variables>;
+}
 
 // @public (undocumented)
 export interface MutationPromise<Data, Variables> extends PromiseLike<MutationResult<Data, Variables>> {
 }
 
 // @public (undocumented)
-export interface MutationRef<Response, Variables> extends OperationRef<Response, Variables> {
+export interface MutationRef<Data, Variables> extends OperationRef<Data, Variables> {
     // (undocumented)
-    refType: typeof MutationStr;
+    refType: typeof MUTATION_STR;
 }
 
 // @public (undocumented)
-export function mutationRef<Response>(dcInstance: DataConnect, queryName: string): MutationRef<Response, undefined>;
+export function mutationRef<Data>(dcInstance: DataConnect, queryName: string): MutationRef<Data, undefined>;
 
 // @public (undocumented)
-export function mutationRef<Response, Variables>(dcInstance: DataConnect, queryName: string, variables: Variables): MutationRef<Response, Variables>;
+export function mutationRef<Data, Variables>(dcInstance: DataConnect, mutationName: string, variables: Variables): MutationRef<Data, Variables>;
 
 // @public (undocumented)
 export interface MutationResponse<T> extends CancellableOperation<T> {
@@ -163,9 +166,6 @@ export interface MutationResult<Data, Variables> extends DataConnectResult<Data,
     ref: MutationRef<Data, Variables>;
 }
 
-// @public (undocumented)
-export const MutationStr = "mutation";
-
 // @public (undocumented)
 export type OnCompleteSubscription = () => void;
 
@@ -198,16 +198,7 @@ export interface OpResult<Data> {
 }
 
 // @public (undocumented)
-export interface ProjectOptions {
-    // (undocumented)
-    connector: string;
-    // (undocumented)
-    location: string;
-    // (undocumented)
-    projectId: string;
-    // (undocumented)
-    service: string;
-}
+export const QUERY_STR = "query";
 
 // @public (undocumented)
 export interface QueryPromise<Data, Variables> extends PromiseLike<QueryResult<Data, Variables>> {
@@ -216,7 +207,7 @@ export interface QueryPromise<Data, Variables> extends PromiseLike<QueryResult<D
 // @public (undocumented)
 export interface QueryRef<Data, Variables> extends OperationRef<Data, Variables> {
     // (undocumented)
-    refType: typeof QueryStr;
+    refType: typeof QUERY_STR;
 }
 
 // @public (undocumented)
@@ -237,14 +228,11 @@ export interface QueryResult<Data, Variables> extends DataConnectResult<Data, Va
     toJSON: () => SerializedRef<Data, Variables>;
 }
 
-// @public (undocumented)
-export const QueryStr = "query";
-
 // @public (undocumented)
 export type QueryUnsubscribe = () => void;
 
 // @public (undocumented)
-export type ReferenceType = typeof QueryStr | typeof MutationStr;
+export type ReferenceType = typeof QUERY_STR | typeof MUTATION_STR;
 
 // @public (undocumented)
 export interface RefInfo<Variables> {
@@ -296,7 +284,7 @@ export interface SubscriptionOptions<Data, Variables> {
 }
 
 // @public (undocumented)
-export function terminate(dataConnect: DataConnect): void;
+export function terminate(dataConnect: DataConnect): Promise<void>;
 
 // @public (undocumented)
 export function toQueryRef<Data, Variables>(serializedRef: SerializedRef<Data, Variables>): QueryRef<unknown, Variables>;

packages/data-connect/package.json

@@ -33,7 +33,7 @@
     "prettier": "prettier --write '*.js' '*.ts' '@(src|test)/**/*.ts'",
     "build:deps": "lerna run --scope @firebase/'{app,data-connect}' --include-dependencies build",
     "dev": "rollup -c -w",
-    "test": "run-p --npm-path npm lint test:emulator",
+    "test": "run-p --npm-path npm test:emulator",
     "test:ci": "node ../../scripts/run_tests_in_ci.js -s test:emulator",
     "test:all": "npm run test:node",
     "test:browser": "karma start --single-run",

packages/data-connect/src/api.browser.ts

@@ -63,7 +63,7 @@ export function subscribe<Data, Variables>(
   let ref: QueryRef<Data, Variables>;
   let initialCache: OpResult<Data> | undefined;
   if ('refInfo' in queryRefOrSerializedResult) {
-    let serializedRef: SerializedRef<Data, Variables> =
+    const serializedRef: SerializedRef<Data, Variables> =
       queryRefOrSerializedResult;
     const { data, source, fetchTime } = serializedRef;
     initialCache = {

packages/data-connect/src/api/DataConnect.ts

@@ -24,24 +24,18 @@ import {
 import { FirebaseAuthInternalName } from '@firebase/auth-interop-types';
 import { Provider } from '@firebase/component';
 
+import { Code, DataConnectError } from '../core/error';
 import {
   AuthTokenProvider,
   FirebaseAuthProvider
 } from '../core/FirebaseAuthProvider';
 import { QueryManager } from '../core/QueryManager';
+import { logDebug, logError } from '../logger';
 import { DataConnectTransport, TransportClass } from '../network';
 import { RESTTransport } from '../network/transport/rest';
 
 import { MutationManager } from './Mutation';
-import { Code, DataConnectError } from '../core/error';
-import { logDebug, logError } from '../logger';
 
-export interface ProjectOptions {
-  location: string;
-  connector: string;
-  service: string;
-  projectId: string;
-}
 
 export interface ConnectorConfig {
   location: string;
@@ -78,27 +72,28 @@ export interface DataConnectOptions extends ConnectorConfig {
 export class DataConnect {
   _queryManager!: QueryManager;
   _mutationManager!: MutationManager;
-  public isEmulator = false;
+  isEmulator = false;
   initialized = false;
   private _transport!: DataConnectTransport;
-  private transportClass: TransportClass | undefined;
-  private transportOptions?: TransportOptions;
-  private authTokenProvider?: AuthTokenProvider;
+  private _transportClass: TransportClass | undefined;
+  private _transportOptions?: TransportOptions;
+  private _authTokenProvider?: AuthTokenProvider;
   constructor(
     public readonly app: FirebaseApp,
+    // TODO(mtewani): Replace with _dataConnectOptions in the future
     private readonly dataConnectOptions: DataConnectOptions,
-    private readonly authProvider: Provider<FirebaseAuthInternalName>
+    private readonly _authProvider: Provider<FirebaseAuthInternalName>
   ) {
     if (typeof process !== 'undefined' && process.env) {
       const host = process.env[FIREBASE_DATA_CONNECT_EMULATOR_HOST_VAR];
       if (host) {
         logDebug('Found custom host. Using emulator');
         this.isEmulator = true;
-        this.transportOptions = parseOptions(host);
+        this._transportOptions = parseOptions(host);
       }
     }
   }
-  _delete() {
+  _delete(): Promise<void> {
     _removeServiceInstance(
       this.app,
       'data-connect',
@@ -113,49 +108,49 @@ export class DataConnect {
     return copy;
   }
 
-  setInitialized() {
+  setInitialized(): void {
     if (this.initialized) {
       return;
     }
-    if (this.transportClass === undefined) {
+    if (this._transportClass === undefined) {
       logDebug('transportClass not provided. Defaulting to RESTTransport.');
-      this.transportClass = RESTTransport;
+      this._transportClass = RESTTransport;
     }
 
-    if (this.authProvider) {
-      this.authTokenProvider = new FirebaseAuthProvider(
+    if (this._authProvider) {
+      this._authTokenProvider = new FirebaseAuthProvider(
             this.app.name,
             this.app.options,
-            this.authProvider
+            this._authProvider
           );
     }
 
     this.initialized = true;
-    this._transport = new this.transportClass(
+    this._transport = new this._transportClass(
       this.dataConnectOptions,
       this.app.options.apiKey,
-      this.authTokenProvider
+      this._authTokenProvider
     );
-    if (this.transportOptions) {
+    if (this._transportOptions) {
       this._transport.useEmulator(
-        this.transportOptions.host,
-        this.transportOptions.port,
-        this.transportOptions.sslEnabled
+        this._transportOptions.host,
+        this._transportOptions.port,
+        this._transportOptions.sslEnabled
       );
     }
     this._queryManager = new QueryManager(this._transport);
     this._mutationManager = new MutationManager(this._transport);
   }
 
-  enableEmulator(transportOptions: TransportOptions) {
+  enableEmulator(transportOptions: TransportOptions): void {
     if (this.initialized) {
-      logError('enableEmulator called without initializing');
+      logError('enableEmulator called after initialization');
       throw new DataConnectError(
         Code.ALREADY_INITIALIZED,
         'DataConnect instance already initialized!'
       );
     }
-    this.transportOptions = transportOptions;
+    this._transportOptions = transportOptions;
     this.isEmulator = true;
   }
 }
@@ -165,7 +160,7 @@ export function connectDataConnectEmulator(
   host: string,
   port?: number,
   sslEnabled = false
-) {
+): void {
   dc.enableEmulator({ host, port, sslEnabled });
 }
 
@@ -213,7 +208,7 @@ export function getDataConnect(
   });
 }
 
-export function terminate(dataConnect: DataConnect) {
-  dataConnect._delete();
+export function terminate(dataConnect: DataConnect): Promise<void> {
+  return dataConnect._delete();
   // TODO(mtewani): Stop pending tasks
 }

packages/data-connect/src/api/Mutation.ts

@@ -20,52 +20,52 @@
 import { DataConnect } from './DataConnect';
 import {
   DataConnectResult,
-  MutationStr,
+  MUTATION_STR,
   OperationRef,
   SOURCE_SERVER
 } from './Reference';
 
-export interface MutationRef<Response, Variables>
-  extends OperationRef<Response, Variables> {
-  refType: typeof MutationStr;
+export interface MutationRef<Data, Variables>
+  extends OperationRef<Data, Variables> {
+  refType: typeof MUTATION_STR;
 }
 
-export function mutationRef<Response>(
+export function mutationRef<Data>(
   dcInstance: DataConnect,
   queryName: string
-): MutationRef<Response, undefined>;
-export function mutationRef<Response, Variables>(
+): MutationRef<Data, undefined>;
+export function mutationRef<Data, Variables>(
   dcInstance: DataConnect,
-  queryName: string,
+  mutationName: string,
   variables: Variables
-): MutationRef<Response, Variables>;
-export function mutationRef<Response, Variables>(
+): MutationRef<Data, Variables>;
+export function mutationRef<Data, Variables>(
   dcInstance: DataConnect,
   queryName: string,
   variables?: Variables
-): MutationRef<Response, Variables> {
+): MutationRef<Data, Variables> {
   dcInstance.setInitialized();
-  const ref: MutationRef<Response, Variables> = {
+  const ref: MutationRef<Data, Variables> = {
     dataConnect: dcInstance,
     name: queryName,
-    refType: MutationStr,
+    refType: MUTATION_STR,
     variables: variables as Variables
   };
   return ref;
 }
 
 export class MutationManager {
   private _inflight: Array<PromiseLike<unknown>> = [];
-  constructor(private transport: DataConnectTransport) {}
-  executeMutation<Response, Variables>(
-    mutationRef: MutationRef<Response, Variables>
-  ): MutationPromise<Response, Variables> {
-    const result = this.transport.invokeMutation<Response, Variables>(
+  constructor(private _transport: DataConnectTransport) {}
+  executeMutation<Data, Variables>(
+    mutationRef: MutationRef<Data, Variables>
+  ): MutationPromise<Data, Variables> {
+    const result = this._transport.invokeMutation<Data, Variables>(
       mutationRef.name,
       mutationRef.variables
     );
     const withRefPromise = result.then(res => {
-      const obj: MutationResult<Response, Variables> = {
+      const obj: MutationResult<Data, Variables> = {
         ...res, // Double check that the result is result.data, not just result
         source: SOURCE_SERVER,
         ref: mutationRef,
@@ -74,7 +74,7 @@
       return obj;
     });
     this._inflight.push(result);
     const removePromise = () =>
       (this._inflight = this._inflight.filter(promise => promise !== result));
     result.then(removePromise, removePromise);
     return withRefPromise;

packages/data-connect/src/api/Reference.ts

@@ -16,9 +16,9 @@
  */
 
 import { DataConnect, DataConnectOptions } from './DataConnect';
-export const QueryStr = 'query';
-export const MutationStr = 'mutation';
-export type ReferenceType = typeof QueryStr | typeof MutationStr;
+export const QUERY_STR = 'query';
+export const MUTATION_STR = 'mutation';
+export type ReferenceType = typeof QUERY_STR | typeof MUTATION_STR;
 
 export const SOURCE_SERVER = 'SERVER';
 export const SOURCE_CACHE = 'CACHE';