[fix] NetInfo event listeners and types

* Fix 'addEventListener' handler registration.
* Fix event object provided to handlers.
* Fix event object type - always include 'type' and 'effectiveType'.
* Fix unit test semantics.
* Fix documented NetInfo types.

Close #724

docs/storybook/2-apis/NetInfo/NetInfoScreen.js

@@ -32,8 +32,9 @@ const NetInfoScreen = () => (
       <DocItem
         description={
           <AppText>
-            One of <Code>slow-2g</Code>, <Code>2g</Code>, <Code>3g</Code>, <Code>4g</Code>,{' '}
+            One of <Code>bluebooth</Code>, <Code>cellular</Code>, <Code>ethernet</Code>,{' '}
-            <Code>unknown</Code>.
+            <Code>mixed</Code>, <Code>mixed</Code>, <Code>none</Code>, <Code>other</Code>,{' '}
+            <Code>unknown</Code>, <Code>wifi</Code>, <Code>wimax</Code>
           </AppText>
         }
         name="ConnectionType"
@@ -41,9 +42,8 @@ const NetInfoScreen = () => (
       <DocItem
         description={
           <AppText>
-            One of <Code>bluebooth</Code>, <Code>cellular</Code>, <Code>ethernet</Code>,{' '}
+            One of <Code>slow-2g</Code>, <Code>2g</Code>, <Code>3g</Code>, <Code>4g</Code>,{' '}
-            <Code>mixed</Code>, <Code>mixed</Code>, <Code>none</Code>, <Code>other</Code>,{' '}
+            <Code>unknown</Code>.
-            <Code>unknown</Code>, <Code>wifi</Code>, <Code>wimax</Code>
           </AppText>
         }
         name="EffectiveConnectionType"

src/apis/NetInfo/__tests__/index-test.js

@@ -2,6 +2,8 @@
 
 import NetInfo from '..';
 
+const handler = () => {};
+
 describe('apis/NetInfo', () => {
   describe('getConnectionInfo', () => {
     test('fills out basic fields', done => {
@@ -13,9 +15,22 @@ describe('apis/NetInfo', () => {
     });
   });
 
-  describe('isConnected', () => {
+  describe('addEventListener', () => {
-    const handler = () => {};
+    test('throws if the provided "eventType" is not supported', () => {
+      expect(() => NetInfo.addEventListener('foo', handler)).toThrow();
+    });
+  });
 
+  describe('removeEventListener', () => {
+    test('throws if the provided "eventType" is not supported', () => {
+      expect(() => NetInfo.removeEventListener('foo', handler)).toThrow();
+    });
+    test('throws if the handler is not registered', () => {
+      expect(() => NetInfo.removeEventListener('connectionChange', handler)).toThrow();
+    });
+  });
+
+  describe('isConnected', () => {
     afterEach(() => {
       try {
         NetInfo.isConnected.removeEventListener('connectionChange', handler);
@@ -25,22 +40,18 @@ describe('apis/NetInfo', () => {
     describe('addEventListener', () => {
       test('throws if the provided "eventType" is not supported', () => {
         expect(() => NetInfo.isConnected.addEventListener('foo', handler)).toThrow();
-        expect(() =>
-          NetInfo.isConnected.addEventListener('connectionChange', handler)
-        ).not.toThrow();
       });
     });
 
     describe('removeEventListener', () => {
-      test('throws if the handler is not registered', () => {
-        expect(() => NetInfo.isConnected.removeEventListener('connectionChange', handler)).toThrow;
-      });
-
       test('throws if the provided "eventType" is not supported', () => {
         NetInfo.isConnected.addEventListener('connectionChange', handler);
-        expect(() => NetInfo.isConnected.removeEventListener('foo', handler)).toThrow;
+        expect(() => NetInfo.isConnected.removeEventListener('foo', handler)).toThrow();
-        expect(() => NetInfo.isConnected.removeEventListener('connectionChange', handler)).not
+      });
-          .toThrow;
+      test('throws if the handler is not registered', () => {
+        expect(() =>
+          NetInfo.isConnected.removeEventListener('connectionChange', handler)
+        ).toThrow();
       });
     });
   });

src/apis/NetInfo/index.js

@@ -23,13 +23,17 @@ const connection =
 // Prevent the underlying event handlers from leaking and include additional
 // properties available in browsers
 const getConnectionInfoObject = () => {
-  const result = {};
+  const result = {
+    effectiveType: 'unknown',
+    type: 'unknown'
+  };
   if (!connection) {
     return result;
   }
   for (const prop in connection) {
-    if (typeof connection[prop] !== 'function') {
+    const value = connection[prop];
-      result[prop] = connection[prop];
+    if (typeof value !== 'function' && value != null) {
+      result[prop] = value;
     }
   }
   return result;
@@ -43,6 +47,7 @@ const eventTypesMap = {
 const eventTypes = Object.keys(eventTypesMap);
 
 const connectionListeners = [];
+const netInfoListeners = [];
 
 /**
  * Navigator online: https://developer.mozilla.org/en-US/docs/Web/API/NavigatorOnLine/onLine
@@ -63,21 +68,29 @@ const NetInfo = {
       };
     }
 
-    connection.addEventListener(eventTypesMap[type], handler);
+    const wrappedHandler = () => handler(getConnectionInfoObject());
+    netInfoListeners.push([handler, wrappedHandler]);
+    connection.addEventListener(eventTypesMap[type], wrappedHandler);
     return {
       remove: () => NetInfo.removeEventListener(eventTypesMap[type], handler)
     };
   },
 
   removeEventListener(type: string, handler: Function): void {
-    invariant(eventTypes.indexOf(type) !== -1, 'Trying to subscribe to unknown event: "%s"', type);
+    invariant(
+      eventTypes.indexOf(type) !== -1,
+      'Trying to unsubscribe from unknown event: "%s"',
+      type
+    );
     if (type === 'change') {
       console.warn('Listening to event `change` is deprecated. Use `connectionChange` instead.');
     }
-    if (!connection) {
+
-      return;
+    const listenerIndex = findIndex(netInfoListeners, pair => pair[0] === handler);
-    }
+    invariant(listenerIndex !== -1, 'Trying to remove NetInfo listener for unregistered handler');
-    connection.removeEventListener(eventTypesMap[type], handler);
+    const [, wrappedHandler] = netInfoListeners[listenerIndex];
+    connection.removeEventListener(eventTypesMap[type], wrappedHandler);
+    netInfoListeners.splice(listenerIndex, 1);
   },
 
   fetch(): Promise<any> {
@@ -93,11 +106,7 @@ const NetInfo = {
 
   getConnectionInfo(): Promise<Object> {
     return new Promise((resolve, reject) => {
-      resolve({
+      resolve(getConnectionInfoObject());
-        effectiveType: 'unknown',
-        type: 'unknown',
-        ...getConnectionInfoObject()
-      });
     });
   },