Simplify over-engineered Activity module

Summary: @​public
I've noticed that the logs can be sometimes misleading, as when an Activity ends it doesn't log immediatly. A sync `console.log` would log before it although the Acitivity should've finished before.

Turns out we wait before writing out the logs to the console. I don't see any reason for this. Looking at the `Activity` module it's over-engineered. This diff makes logging sync and simplfies the module.

Reviewed By: @martinbigio

Differential Revision: D2467922

packager/react-packager/src/Activity/__tests__/Activity-test.js

@@ -57,33 +57,15 @@ describe('Activity', () => {
     });
 
     it('throws when called with an invalid eventId', () => {
-      expect(() => Activity.endEvent(42)).toThrow(
-        'event(42) is not a valid event id!',
-      );
+      expect(() => Activity.endEvent(42)).toThrow();
     });
 
     it('throws when called with an expired eventId', () => {
       const eid = Activity.startEvent('', '');
       Activity.endEvent(eid);
-
       expect(() => {
         Activity.endEvent(eid);
-      }).toThrow('event(1) has already ended!');
-    });
-  });
-
-  describe('signal', () => {
-    it('writes a SIGNAL event out to the console', () => {
-      const EVENT_NAME = 'EVENT_NAME';
-      const DATA = {someData: 42};
-
-      Activity.signal(EVENT_NAME, DATA);
-      jest.runOnlyPendingTimers();
-
-      expect(console.log.mock.calls.length).toBe(1);
-      const consoleMsg = console.log.mock.calls[0][0];
-      expect(consoleMsg).toContain(EVENT_NAME);
-      expect(consoleMsg).toContain(JSON.stringify(DATA));
+      }).toThrow();
     });
   });
 });

packager/react-packager/src/Activity/index.js

@@ -11,60 +11,30 @@
 const chalk = require('chalk');
 const events = require('events');
 
-const COLLECTION_PERIOD = 1000;
-
-const _endedEvents = Object.create(null);
 const _eventStarts = Object.create(null);
-const _queuedActions = [];
 const _eventEmitter = new events.EventEmitter();
 
-let _scheduledCollectionTimer = null;
 let _uuid = 1;
 let _enabled = true;
 
 function endEvent(eventId) {
   const eventEndTime = Date.now();
-
   if (!_eventStarts[eventId]) {
-    _throw('event(' + eventId + ') is not a valid event id!');
+    throw new Error('event(' + eventId + ') either ended or never started');
   }
 
-  if (_endedEvents[eventId]) {
-    _throw('event(' + eventId + ') has already ended!');
-  }
-
-  _scheduleAction({
+  _writeAction({
     action: 'endEvent',
     eventId: eventId,
     tstamp: eventEndTime
   });
-  _endedEvents[eventId] = true;
-}
-
-function signal(eventName, data) {
-  const signalTime = Date.now();
-
-  if (eventName == null) {
-    _throw('No event name specified');
-  }
-
-  if (data == null) {
-    data = null;
-  }
-
-  _scheduleAction({
-    action: 'signal',
-    data: data,
-    eventName: eventName,
-    tstamp: signalTime
-  });
 }
 
 function startEvent(eventName, data) {
   const eventStartTime = Date.now();
 
   if (eventName == null) {
-    _throw('No event name specified');
+    throw new Error('No event name specified');
   }
 
   if (data == null) {
@@ -79,8 +49,8 @@ function startEvent(eventName, data) {
     eventName: eventName,
     tstamp: eventStartTime,
   };
-  _scheduleAction(action);
   _eventStarts[eventId] = action;
+  _writeAction(action);
 
   return eventId;
 }
@@ -89,46 +59,9 @@ function disable() {
   _enabled = false;
 }
 
-function _runCollection() {
-  /* jshint -W084 */
-  let action;
-  while ((action = _queuedActions.shift())) {
-    _writeAction(action);
-  }
-
-  _scheduledCollectionTimer = null;
-}
-
-function _scheduleAction(action) {
-  _queuedActions.push(action);
+function _writeAction(action) {
   _eventEmitter.emit(action.action, action);
 
-  if (_scheduledCollectionTimer === null) {
-    _scheduledCollectionTimer = setTimeout(_runCollection, COLLECTION_PERIOD);
-  }
-}
-
-/**
- * This a utility function that throws an error message.
- *
- * The only purpose of this utility is to make APIs like
- * startEvent/endEvent/signal inlineable in the JIT.
- *
- * (V8 can't inline functions that statically contain a `throw`, and probably
- *  won't be adding such a non-trivial optimization anytime soon)
- */
-function _throw(msg) {
-  const err = new Error(msg);
-
-  // Strip off the call to _throw()
-  const stack = err.stack.split('\n');
-  stack.splice(1, 1);
-  err.stack = stack.join('\n');
-
-  throw err;
-}
-
-function _writeAction(action) {
   if (!_enabled) {
     return;
   }
@@ -157,22 +90,13 @@ function _writeAction(action) {
       delete _eventStarts[action.eventId];
       break;
 
-    case 'signal':
-      console.log(
-        '[' + fmtTime + '] ' +
-        '        ' + action.eventName + '' +
-        data
-      );
-      break;
-
     default:
-      _throw('Unexpected scheduled action type: ' + action.action);
+      throw new Error('Unexpected scheduled action type: ' + action.action);
   }
 }
 
 
 exports.endEvent = endEvent;
-exports.signal = signal;
 exports.startEvent = startEvent;
 exports.disable = disable;
 exports.eventEmitter = _eventEmitter;

packager/react-packager/src/BundlesLayout/__tests__/BundlesLayout-test.js

@@ -8,8 +8,8 @@
  */
 'use strict';
 
-jest.dontMock('../index');
-jest.mock('fs');
+jest.dontMock('../index')
+    .mock('fs');
 
 var Promise = require('promise');
 var BundlesLayout = require('../index');

packager/react-packager/src/BundlesLayout/__tests__/BundlesLayoutIntegration-test.js

@@ -10,7 +10,8 @@
 
 jest
   .autoMockOff()
-  .mock('../../Cache');
+  .mock('../../Cache')
+  .mock('../../Activity');
 
 const Promise = require('promise');
 const path = require('path');

packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js

@@ -14,7 +14,8 @@ const Promise = require('promise');
 
 jest
   .mock('fs')
-  .mock('../../../Cache');
+  .mock('../../../Cache')
+  .mock('../../../Activity');
 
 var Cache = require('../../../Cache');
 var DependencyGraph = require('../index');