Expose selector dependencies for testing purposes

[jimbol]

This PR allows us to test which dependencies are passed into a selector.

Original conversation: #76 (comment)

It matters which dependencies a selector uses. For example, if we have two selectors that use the same resultFunc...

const lookUp = (hash, ids) => ids.map((id) => hash[id]);

const getMyValues = createSelector(
  getMyHash,
  getMyIds,
  lookUp,
);

const getYourValues = createSelector(
  getYourHash,
  getYourIds,
  lookUp,
);

...testing the resultFunc isn't enough. We actually need to test that the first two functions are what we expect them to be. We can do this with an integration test (passing in the entire state into the whole selector) but, it forces us to essentially re-test getMyHash, getMyIds.

Desired API

We've been running into this problem. The direction I'm leaning is to expose the dependencies so that we can test them with a simple equality check.

it('uses the dependencies we expect', () => {
  // We can expose `dependencies`
  const dependencies = getMyValues.dependencies;

  expect(dependencies).to.deep.equal([
    getMyHash,
    getMyIds,
  ]);
});

[coveralls]

Coverage remained the same at 100.0% when pulling fd3238c on jimbol:expose-dependencies into 0c9150e on reactjs:master.

[codecov-io]

Codecov Report

Merging #251 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #251   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          15     15           
=====================================
  Hits           15     15

Impacted Files	Coverage Δ
src/index.js	100% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c9150e...fd3238c. Read the comment docs.

[neurosnap]

Any updates on this PR? Are the maintainers absent?

[stephanschubert]

Would love to have that feature too.

[jimbol]

What does everyone think of this?

[elisechant]

merge! 🚢

[sandersiim]

Just ran into the same issue myself, so this would be super helpful. Merge!

[hally9k]

Merge! 👍

[skortchmark9]

Wooohoooo! I can't believe I didn't see this. This will allow me to remove a step from installing the reselect developer tools

[neurosnap]

Did this get merged?

[ghost]

@ellbee @alex3165 @markerikson can y'all take a look at this to see if we can merge it?

[skortchmark9]

@ellbee could you take a look at this? It would be really handy to have in

[ellbee]

Ok, this is not something that has been a pain point for me but seems to be for others so lets merge it in!

[akinnee]

Has this change been published? I see it in master but not in my node_modules.

[Aksana-Tsishchanka]

I do not see that there is a necessity to mock the dependencies of the selector and test the only resultFunction.
The selector is a pure function that receives several params. From the unit testing perspective, we have to test the entire function and test behavior, not implementation.
This function should be considered as a black box. There are input and output that's all. In that proposal, dependencies are considered everything inside the selector. But let's take a look at the following example?
If I have selector

   export const getProjectAssignmentsTimeRange = createSelector(
      (store, projectId) => getProjectAssignmentsByProjectId(store, projected),
      store => store[ns.calendarRangeExtension].extraCount,
      getPlanningAssignmentTabZoomLevel,
      (projectGantt, extraCount, zoomLevel) => {
         const yearAddition = extraCount || 0;
        ......
      }
);

Are getPlanningAssignmentTabZoomLevel and getProjectAssignmentsByProjectId real dependencies for the main selector? I do not think so, since it's a pure function and does not have side effects, basically, you can copy the code of this function and put inside the selector.

However, if you mock getPlanningAssignmentTabZoomLevel you rely on the code inside of the selector,
which will require the test update if the code/implementation of this pseudo-dependencies changes (store change, param change) even though the "business" requirement of the selector is still the same.

I do not think that it's a good approach to test the technical implementation rather than test what the selector should do.

As an example of the correct test approach, please find my test. I used the BDD (Behavior-Driven Development) approach.

describe('calendar range for calendar on Assignments Tab for Week view', () => {
    beforeAll(() => {
      store = {
        [ns.calendarRangeExtension]: {
          extraCount: 0,
        },
        [ns.projectGantts]: {
          byProjectId: {

          },
        },
        [ns.planningSettings]: {
          projectAssignmentsZoomLevel: ZoomLevel.WEEK,
        },
      };
      spyToday = jest.spyOn(global, 'Date').mockImplementation(() => new Date(2020, 8, 24));
    });

    test('has to calculate range when there is no project allocation', () => {
      spyToday.mockRestore();
      expect(getProjectAssignmentsTimeRange(store, 1)).toEqual({
        startDate: new Date(getFakedUTCDate('2019-01-01')),
        endDate: new Date(getFakedUTCDate('2021-12-31')),
      });
    });

Instead of mock function getPlanningAssignmentTabZoomLevel I mock the param which is store.