[firestore][js] misc transactions internals changes

lib/modules/firestore/Transaction.js

@@ -8,7 +8,7 @@ import { buildNativeMap } from './utils/serialize';
 import type Firestore from './';
 import type { TransactionMeta } from './TransactionHandler';
 import type DocumentReference from './DocumentReference';
-import type DocumentSnapshot from './DocumentSnapshot';
+import DocumentSnapshot from './DocumentSnapshot';
 import { isObject, isString } from '../../utils';
 import FieldPath from './FieldPath';
 import { getNativeModule } from '../../utils/native';
@@ -24,6 +24,9 @@ type SetOptions = {
   merge: boolean,
 };
 
+// TODO docs state all get requests must be made FIRST before any modifications
+// TODO so need to validate that
+
 /**
  * @class Transaction
  */
@@ -72,10 +75,9 @@ export default class Transaction {
    */
   get(documentRef: DocumentReference): Promise<DocumentSnapshot> {
     // todo validate doc ref
-    return getNativeModule(this._firestore).transactionGetDocument(
-      this._meta.id,
-      documentRef.path
-    );
+    return getNativeModule(this._firestore)
+      .transactionGetDocument(this._meta.id, documentRef.path)
+      .then(result => new DocumentSnapshot(this._firestore, result));
   }
 
   /**
@@ -102,7 +104,7 @@ export default class Transaction {
       type: 'set',
       path: documentRef.path,
       data: buildNativeMap(data),
-      options,
+      options: options || {},
     });
 
     return this;

lib/modules/firestore/TransactionHandler.js

@@ -67,7 +67,10 @@ export default class TransactionHandler {
       reject: null,
       resolve: null,
       updateFunction,
-      stack: new Error().stack.slice(1),
+      stack: new Error().stack
+        .split('\n')
+        .slice(4)
+        .join('\n'),
     };
 
     meta.transaction = new Transaction(this._firestore, meta);
@@ -91,14 +94,11 @@ export default class TransactionHandler {
    * Destroys a local instance of a transaction meta
    *
    * @param id
-   * @param pendingAbort Notify native that there's still an transaction in
-   *        progress that needs aborting - this is to handle a JS side
-   *        exception
    * @private
    */
-  _remove(id, pendingAbort = false) {
+  _remove(id) {
     // todo confirm pending arg no longer needed
-    getNativeModule(this._firestore).transactionDispose(id, pendingAbort);
+    getNativeModule(this._firestore).transactionDispose(id);
     // TODO may need delaying to next event loop
     delete this._pending[id];
   }
@@ -168,11 +168,15 @@ export default class TransactionHandler {
         pendingResult = await possiblePromise;
       }
     } catch (exception) {
-      updateFailed = true; // in case the user rejects with nothing
+      // exception can still be falsey if user `Promise.reject();` 's with no args
+      // so we track the exception with a updateFailed boolean to ensure no fall-through
+      updateFailed = true;
       finalError = exception;
     }
 
     // reject the final promise and remove from native
+    // update is failed when either the users updateFunction
+    // throws an error or rejects a promise
     if (updateFailed) {
       return reject(finalError);
     }
@@ -183,7 +187,7 @@ export default class TransactionHandler {
     transaction._pendingResult = pendingResult;
 
     // send the buffered update/set/delete commands for native to process
-    return getNativeModule(this._firestore).transactionProcessUpdateResponse(
+    return getNativeModule(this._firestore).transactionApplyBuffer(
       id,
       transaction._commandBuffer
     );

lib/modules/firestore/index.js

@@ -154,6 +154,7 @@ export default class Firestore extends ModuleBase {
       )
     );
   }
+
   enableNetwork(): void {
     throw new Error(
       INTERNALS.STRINGS.ERROR_UNSUPPORTED_MODULE_METHOD(
@@ -162,6 +163,7 @@ export default class Firestore extends ModuleBase {
       )
     );
   }
+
   disableNetwork(): void {
     throw new Error(
       INTERNALS.STRINGS.ERROR_UNSUPPORTED_MODULE_METHOD(
@@ -180,6 +182,7 @@ export default class Firestore extends ModuleBase {
   enablePersistence(): Promise<void> {
     throw new Error('Persistence is enabled by default on the Firestore SDKs');
   }
+
   settings(): void {
     throw new Error('firebase.firestore().settings() coming soon');
   }