Added more comments, and more tests for Node.

Author: wu-hui

packages/firestore/src/platform/platform.ts

@@ -50,6 +50,9 @@ export interface Platform {
    */
   randomBytes(nBytes: number): Uint8Array;
 
+  /**
+   * Builds a `ByteStreamReader` from a data source.
+   */
   toByteStreamReader(source: unknown): ByteStreamReader;
 
   /** The Platform's 'window' implementation or null if not available. */
@@ -62,16 +65,31 @@ export interface Platform {
   readonly base64Available: boolean;
 }
 
+/**
+ * An interface compatible with Web's ReadableStream.getReader() return type.
+ *
+ * This can be used as an abstraction to mimic `ReadableStream` where it is not
+ * available.
+ */
 export interface ByteStreamReader {
   read(): Promise<ByteStreamReadResult>;
   cancel(reason?: string): Promise<void>;
 }
 
+/**
+ * An interface compatible with ReadableStreamReadResult<UInt8Array>.
+ */
 export interface ByteStreamReadResult {
   done: boolean;
-  value: Uint8Array;
+  value?: Uint8Array;
 }
 
+/**
+ * Builds a `ByteStreamReader` from a UInt8Array.
+ * @param source The data source to use.
+ * @param bytesPerRead How many bytes each `read()` from the returned reader
+ *        will read.
+ */
 export function toByteStreamReader(
   source: Uint8Array,
   bytesPerRead = 10240
@@ -88,7 +106,7 @@ export function toByteStreamReader(
         return result;
       }
 
-      return { value: new Uint8Array(), done: true };
+      return { value: undefined, done: true };
     }
 
     async cancel(reason?: string): Promise<void> {}
@@ -109,6 +127,9 @@ export class PlatformSupport {
     PlatformSupport.platform = platform;
   }
 
+  /**
+   * Forcing to set the platform instance, testing only!
+   */
   private static _forceSetPlatform(platform: Platform): void {
     PlatformSupport.platform = platform;
   }

packages/firestore/src/platform_browser/browser_platform.ts

@@ -99,7 +99,12 @@ export class BrowserPlatform implements Platform {
     return bytes;
   }
 
-  toByteStreamReader(source: unknown): ByteStreamReader {
+  /**
+   * On web, a `ReadableStream` is wrapped around by a `ByteStreamReader`.
+   */
+  toByteStreamReader(
+    source: Uint8Array | ArrayBuffer | ReadableStream<Uint8Array>
+  ): ByteStreamReader {
     if (source instanceof Uint8Array) {
       return toByteStreamReader(source);
     }

packages/firestore/src/platform_node/node_platform.ts

@@ -88,6 +88,9 @@ export class NodePlatform implements Platform {
     return randomBytes(nBytes);
   }
 
+  /**
+   * On Node, only supported data source is a `Uint8Array` for now.
+   */
   toByteStreamReader(source: unknown): ByteStreamReader {
     if (source instanceof Uint8Array) {
       return toByteStreamReader(source);

packages/firestore/src/util/bundle_reader.ts

@@ -21,6 +21,7 @@ import {
 } from '../protos/firestore_bundle_proto';
 import { Deferred } from './promise';
 import { ByteStreamReader, PlatformSupport } from '../platform/platform';
+import { debugAssert } from './assert';
 
 /**
  * A complete element in the bundle stream, together with the byte length it
@@ -204,11 +205,12 @@ export class BundleReader {
   private async pullMoreDataToBuffer(): Promise<boolean> {
     const result = await this.reader.read();
     if (!result.done) {
+      debugAssert(!!result.value, 'Read undefined when "done" is false.');
       const newBuffer = new Uint8Array(
-        this.buffer.length + result.value.length
+        this.buffer.length + result.value!.length
       );
       newBuffer.set(this.buffer);
-      newBuffer.set(result.value, this.buffer.length);
+      newBuffer.set(result.value!, this.buffer.length);
       this.buffer = newBuffer;
     }
     return result.done;

packages/firestore/test/unit/platform/platform.test.ts

@@ -16,10 +16,34 @@
  */
 
 import { expect } from 'chai';
-import { PlatformSupport } from '../../../src/platform/platform';
+import {
+  PlatformSupport,
+  toByteStreamReader
+} from '../../../src/platform/platform';
 
 describe('Platform', () => {
   it('can load the platform at runtime', () => {
     expect(PlatformSupport.getPlatform()).to.exist;
   });
+
+  it('toByteStreamReader() steps underlying data', async () => {
+    const encoder = new TextEncoder();
+    const r = toByteStreamReader(encoder.encode('0123456789'), 4);
+
+    let result = await r.read();
+    expect(result.value).to.deep.equal(encoder.encode('0123'));
+    expect(result.done).to.be.false;
+
+    result = await r.read();
+    expect(result.value).to.deep.equal(encoder.encode('4567'));
+    expect(result.done).to.be.false;
+
+    result = await r.read();
+    expect(result.value).to.deep.equal(encoder.encode('89'));
+    expect(result.done).to.be.false;
+
+    result = await r.read();
+    expect(result.value).to.be.undefined;
+    expect(result.done).to.be.true;
+  });
 });

packages/firestore/test/unit/util/bundle.test.ts

@@ -57,11 +57,14 @@ function lengthPrefixedString(o: {}): string {
   return `${l}${str}`;
 }
 
-describe('readableStreamFromString()', () => {
+// Testing readableStreamFromString() is working as expected.
+// eslint-disable-next-line no-restricted-properties
+(isNode() ? describe.skip : describe)('readableStreamFromString()', () => {
   it('returns stepping readable stream', async () => {
     const encoder = new TextEncoder();
-    const s = readableStreamFromString('0123456789', 4);
-    const r = s.getReader();
+    const r = PlatformSupport.getPlatform().toByteStreamReader(
+      readableStreamFromString('0123456789', 4)
+    );
 
     let result = await r.read();
     expect(result.value).to.deep.equal(encoder.encode('0123'));
@@ -81,14 +84,15 @@ describe('readableStreamFromString()', () => {
   });
 });
 
-describe.only('Bundle ', () => {
+describe('Bundle ', () => {
   genericBundleReadingTests(1);
   genericBundleReadingTests(4);
   genericBundleReadingTests(64);
   genericBundleReadingTests(1024);
 });
 
 function genericBundleReadingTests(bytesPerRead: number): void {
+  // On Node, we need to override `bytesPerRead` from it's platform's `toByteStreamReader` call.
   if (isNode()) {
     const platform = PlatformSupport.getPlatform();
     platform.toByteStreamReader = source =>