feat: Add support for environment variable FIRESTORE_PREFER_REST

dev/src/index.ts

@@ -537,8 +537,10 @@ export class Firestore implements firestore.Firestore {
    * `DocumentReference<T>.onSnapshot()`, `CollectionReference<T>.onSnapshot()`, or
    * `Query<T>.onSnapshot()`. If specified, this setting value will take precedent over the
    * environment variable `FIRESTORE_PREFER_REST`. If not specified, the
-   * SDK will use the value specified in environment variable `FIRESTORE_PREFER_REST=true|false`. And if the
-   * environment variable is also not set, then the setting will default to `false`.
+   * SDK will use the value specified in the environment variable `FIRESTORE_PREFER_REST`.
+   * Valid values of `FIRESTORE_PREFER_REST` are `true` ('1') or `false` (`0`). Values are
+   * not case-sensitive. Any other value for the environment variable will be ignored and
+   * a warning will be logged to the console.
    */
   constructor(settings?: firestore.Settings) {
     const libraryHeader = {
@@ -669,9 +671,8 @@ export class Firestore implements firestore.Firestore {
 
     // If preferRest is not specified in settings, but is set as environment variable,
     // then use the environment variable value.
-    const [preferRestEnvIsSet, preferRestEnvValue] =
-      tryGetPreferRestEnvironmentVariable();
-    if (settings.preferRest === undefined && preferRestEnvIsSet) {
+    const preferRestEnvValue = tryGetPreferRestEnvironmentVariable();
+    if (settings.preferRest === undefined && preferRestEnvValue !== undefined) {
       settings = {
         ...settings,
         preferRest: preferRestEnvValue,

dev/src/util.ts

@@ -219,27 +219,28 @@ export function wrapError(err: Error, stack: string): Error {
 }
 
 /**
- * Returns a tuple indicating if FIRESTORE_PREFER_REST is set to a valid value,
- * and if so, whether the value enables preferRest or disables preferRest.
+ * Parses the value of the environment variable FIRESTORE_PREFER_REST, and returns
+ * a value indicating if the environment variable enables or disables preferRest.
  *
- * @return A tuple where the first value indicates if the environment variable is set to a valid value,
- * and the second value indicates if the environment variable enables preferRest or disables preferRest.
+ * This method will warn to the console if the environment variable is set to an unsupported value.
+ *
+ * @return {boolean | undefined} `true` if the environment variable enables `preferRest`,
+ * `false` if the environment variable disable `preferRest`, or `undefined` if the
+ * environment variable is not set or is set to an unsupported value.
  */
-export function tryGetPreferRestEnvironmentVariable():
-  | [false, undefined]
-  | [true, boolean] {
+export function tryGetPreferRestEnvironmentVariable(): boolean | undefined {
   const rawValue = process.env.FIRESTORE_PREFER_REST?.trim().toLowerCase();
   if (rawValue === undefined) {
-    return [false, undefined];
+    return undefined;
   } else if (rawValue === '1' || rawValue === 'true') {
-    return [true, true];
+    return true;
   } else if (rawValue === '0' || rawValue === 'false') {
-    return [true, false];
+    return false;
   } else {
     // eslint-disable-next-line no-console
     console.warn(
       `An unsupported value was specified for the environment variable FIRESTORE_PREFER_REST. Value ${rawValue} is unsupported.`
     );
-    return [false, undefined];
+    return undefined;
   }
 }

dev/test/index.ts

@@ -657,6 +657,10 @@ describe('instantiation', () => {
     it('preferRest is disabled (falsy) by default', async () => {
       delete process.env.FIRESTORE_PREFER_REST;
       const firestore = new Firestore.Firestore({});
+      // Convention with settings is to leave undefined settings as
+      // undefined. We could test for undefined here, but converting
+      // to boolean and testing for falsy-ness is consistent with the
+      // code that consumes settings.
       expect(!!firestore['_settings'].preferRest).to.be.false;
     });
 

dev/test/util.ts

@@ -58,51 +58,50 @@ describe('isPlainObject()', () => {
 
     it('reads true', async () => {
       process.env.FIRESTORE_PREFER_REST = 'true';
-      const [prevSet, prevValue] = tryGetPreferRestEnvironmentVariable();
-      expect(prevSet).to.be.true;
+      const prevValue = tryGetPreferRestEnvironmentVariable();
       expect(prevValue).to.be.true;
     });
 
     it('reads 1', async () => {
       process.env.FIRESTORE_PREFER_REST = '1';
-      const [prevSet, prevValue] = tryGetPreferRestEnvironmentVariable();
-      expect(prevSet).to.be.true;
+      const prevValue = tryGetPreferRestEnvironmentVariable();
       expect(prevValue).to.be.true;
     });
 
     it('reads false', async () => {
       process.env.FIRESTORE_PREFER_REST = 'false';
-      const [prevSet, prevValue] = tryGetPreferRestEnvironmentVariable();
-      expect(prevSet).to.be.true;
+      const prevValue = tryGetPreferRestEnvironmentVariable();
       expect(prevValue).to.be.false;
     });
 
     it('reads 0', async () => {
       process.env.FIRESTORE_PREFER_REST = '0';
-      const [prevSet, prevValue] = tryGetPreferRestEnvironmentVariable();
-      expect(prevSet).to.be.true;
+      const prevValue = tryGetPreferRestEnvironmentVariable();
       expect(prevValue).to.be.false;
     });
 
     it('ignores case', async () => {
       process.env.FIRESTORE_PREFER_REST = 'True';
-      const [prevSet, prevValue] = tryGetPreferRestEnvironmentVariable();
-      expect(prevSet).to.be.true;
+      const prevValue = tryGetPreferRestEnvironmentVariable();
       expect(prevValue).to.be.true;
     });
 
-    it('indicates when the environment variable is not set', async () => {
+    it('trims whitespace', async () => {
+      process.env.FIRESTORE_PREFER_REST = '  true  ';
+      const prevValue = tryGetPreferRestEnvironmentVariable();
+      expect(prevValue).to.be.true;
+    });
+
+    it('returns undefined when the environment variable is not set', async () => {
       delete process.env.FIRESTORE_PREFER_REST;
-      const [prevSet, prevValue] = tryGetPreferRestEnvironmentVariable();
-      expect(prevSet).to.be.false;
+      const prevValue = tryGetPreferRestEnvironmentVariable();
       expect(prevValue).to.be.undefined;
       expect(warnSpy.calledOnce).to.be.false;
     });
 
-    it('indicates when the environment variable is set to an unsupported value', async () => {
+    it('returns undefined and warns when the environment variable is set to an unsupported value', async () => {
       process.env.FIRESTORE_PREFER_REST = 'enable';
-      const [prevSet, prevValue] = tryGetPreferRestEnvironmentVariable();
-      expect(prevSet).to.be.false;
+      const prevValue = tryGetPreferRestEnvironmentVariable();
       expect(prevValue).to.be.undefined;
       expect(warnSpy.calledOnce).to.be.true;
       expect(warnSpy.getCall(0).args[0]).to.match(