Add optional handleChange param get data from listeners for profiling

[brentvatne]

This started off from a discussion in reduxjs/redux#980

As far as I can tell there is no way to get the information that I need from listeners using the approach suggested by @gaearon, but if we have the handleChange return some data for us then it's straightforward.

The approach I took here is to have handleChange take an optional parameter which is a function that should extract the data we need, given the wrapped component's name, the nextState, and to leave the door open to other requirements, the instance of the connect component itself.

If this or some variation of this is accepted, it would be great to also do a point release for 3.x in order to support React Native right away (which is the context where I am using this).

An example of how you might use this:

let currentActionType;
let measurements = {};

function trackReducer(reducer) {
  return (state, action) => {
    let startTime = new Date();

    /* Do the work */
    let result = reducer(state, action);

    let duration = (new Date()) - startTime;

    if (action && action.type) {
      measurements[action.type] = {
        ...measurements[action.type],
        reducerTime: duration,
      }
    }

    return result;
  };
}

function extractListenerData(component, state) {
  return { component, state };
}

function trackListener(listener) {
  return function() {
    let startTime = new Date();
    let result;

    /* Do the work */
    let { component, state } = listener(extractListenerData);
    = result;

    let duration = (new Date()) - startTime;
    let currentMeasurements = measurements[currentActionType];

    let data = `${component}: ${duration}ms`;

    /* Alternatively, show all of the data */
    // let data = {
    //   component,
    //   duration,
    //   state,
    // }

    measurements[currentActionType].listeners.push(data);

    measurements[currentActionType] = {
      ...currentMeasurements,
      listenerTime: currentMeasurements.listenerTime + duration,
    };
  };
}

function trackDispatch(action, dispatch) {
  /* It is possible for an action to be dispatched by another action, so
   * here we track the parent to surface that. Could also easily support
   * tracking multiple parents.. But probably don't want to encourage that.. */
  let parentActionType = currentActionType;
  currentActionType = action.type;

  /* Initialize some clean data for this action */
  measurements[currentActionType] = {
    type: currentActionType,
    parentType: parentActionType,
    listeners: [],
    listenerTime: 0,
    reducerTime: 0,
  }

  /* Actually do the action */
  let result = dispatch();

  /* Log the results */
  console.log(measurements[currentActionType]);

  /* Clean up */
  currentActionType = parentActionType;
  return result;
}

function hook(wrapReducer, wrapListener, wrapDispatch) {
  return (next) => (reducer, initialState) => {
    const store = next(wrapReducer(reducer), initialState);

    return {
      ...store,
      dispatch: (action) => {
        return wrapDispatch(action, () => { store.dispatch(action) });
      },
      subscribe: (listener) => {
        return store.subscribe(wrapListener(listener));
      }
    };
  };
}

export default function instrumentCreateStore(createStoreFn) {
  return hook(trackReducer, trackListener, trackDispatch)(createStoreFn);
}

The result:

{ type: 'update-image-gallery-lifecycle',
  parentType: undefined,
  listeners:
   [ 'App: 1ms',
     'HomeScreen: 1ms',
     'ImageGallery: 21ms',
     'MainScreen: 1ms',
     'StoryScreen: 0ms',
     'StoryItem: 1ms',
     'StoryItem: 0ms',
     'StoryItem: 1ms',
     'StoryItem: 0ms',
     'StoryItem: 0ms',
     'Story: 1ms',
     'StoryItem: 0ms',
     'StoryItem: 0ms',
     'StoryItem: 0ms',
     'StoryItem: 1ms',
     'StoryItem: 0ms',
     'StoryItem: 0ms',
     'StoryItem: 0ms',
     'ImageGalleryItem: 16ms',
     'ImageGalleryItem: 85ms',
     'ImageGalleryItem: 9ms',
     'ImageGalleryItem: 8ms',
     'ImageGalleryItem: 8ms',
     'ImageGalleryItem: 8ms',
     'ImageGalleryItem: 8ms',
     'ImageGalleryItem: 7ms',
     'ImageGalleryItem: 8ms',
     'ImageGalleryItem: 8ms',
     'ImageGalleryItem: 8ms' ],
  listenerTime: 201,
  reducerTime: 0 }

[brentvatne]

Lint error and failing test unrelated to this branch, happening to me when I run against master as well

[brentvatne]

@epeli @gaearon - any feedback?

[gaearon]

While the solution is neat I would like to avoid extending meaning of existing APIs. Adding an argument to the listener seems problematic to me because other Redux extensions may try to do the same, and thus be incompatible.

How about we put this information on listener itself as properties? Even without a profiling tool, it would be useful when debugging Redux in console because you'd be see much more information about the listeners. This also means no changes to core API.

[gaearon]

I don't want to complicate connect() with this but I'm up for extracting the heavy lifting from connect() into a separate function that would make it much easier to build your custom connect() that can support this. If you're interested in discussing an API, let's do that in #269 (comment).