Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Framework: Add create-selector library for memoized selectors #3545

Merged
merged 3 commits into from
Feb 25, 2016
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions client/lib/create-selector/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
REPORTER ?= spec
NODE_BIN := $(shell npm bin)
MOCHA ?= $(NODE_BIN)/mocha
BASE_DIR := $(NODE_BIN)/../..
NODE_PATH := $(BASE_DIR)/client

test:
@NODE_ENV=test NODE_PATH=$(NODE_PATH) $(MOCHA) --compilers jsx:babel/register,js:babel/register --reporter $(REPORTER)

.PHONY: test
52 changes: 52 additions & 0 deletions client/lib/create-selector/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
`create-selector`
=================

This module exports a single function which creates a memoized state selector for use with the Redux global application state.

## Usage

`createSelector` accepts two arguments:

- A function which calculates the cached result given a state object and any number of variable arguments necessary to calculate the result
- A function which returns an array of dependent state paths

For example, we might consider that our state contains post objects, each of which are assigned to a particular site. Retrieving an array of posts for a specific site would require us to filter over all of the known posts. While this would normally be an expensive operation, we can use `createSelector` to create a memoized function:

```js
export const getSitePosts = createSelector(
( state, siteId ) => state.posts.filter( ( post ) => post.site_ID === siteId ),
( state ) => [ state.posts ]
);
```

In using the selector, we only need to consider the signature of the first function argument. In this case, we'll need to pass a state object and site ID.

```js
const sitePosts = getSitePosts( state, siteId );
```

This result would only be calculated once, so long as `state.posts` remains the same.

## FAQ

### What is a memoized selector?

We strive to keep redundant data out of our Redux state tree, but this can come at the cost of performance if our selectors are time-consuming in how they evaluate and filter over the minimal state.

A memoized selector can alleviate this concern by storing a cached result, skipping these expensive derivations when we know that the result has already been computed once before from the same state.

### How does a memoized selector know when to recalculate its result?

Because Redux discourages us from mutating state directly, we can be confident that state is only considered to be changed if the segments of the state tree we're concerned with are strictly unequal to a previous state.

When creating a memoized selector via `createSelector`, we pass a function which returns a value or array of values comprising of the parts of the tree we depend upon for our selector.

### Can I pass arguments to my memoized selector?

Yes! This is a very common pattern in our state selectors, and is a key differentiator from [`reselect`](https://github.com/reactjs/reselect), another popular tool which achieves a similar goal.

Do note that the internal memoized function calculates its cache key by a simple [`Array.prototype.join`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/join) call, so your arguments should not be complex objects.

### How can I access the internal cache?

While you should rarely need to do so, you can manage the internal Lodash `memoize.Cache` instance on the `memoizedSelector` property of the returned function.
71 changes: 71 additions & 0 deletions client/lib/create-selector/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/**
* External dependencies
*/
import memoize from 'lodash/memoize';
import shallowEqual from 'react-pure-render/shallowEqual';

/**
* Internal dependencies
*/
import config from 'config';

/**
* Constants
*/
const VALID_ARG_TYPES = [ 'number', 'boolean', 'string' ];

/**
* At runtime, assigns a function which returns a cache key for the memoized
* selector function, given a state object and a variable set of arguments. In
* development mode, this warns when the memoized selector is passed a complex
* object argument, as these cannot be depended upon as reliable cache keys.
*
* @type {Function} Function returning cache key for memoized selector
*/
const getCacheKey = ( () => {
if ( 'development' !== config( 'env' ) ) {
return ( state, ...args ) => args.join();
}

const warn = require( 'lib/warn' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh. does this actually keep these out of the prod bundle? I thought the analyzer was static...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh. does this actually keep these out of the prod bundle? I thought the analyzer was static...

In testing this theory, you're correct that it would be included in the bundle, with the caveat being if Webpack is able to determine assuredly that the require would never be reached, i.e. if I had a module defined as:

var m;
if ( true ) {
    m = require( './a' );
} else {
    m = require( './b' );
}

...then Webpack is smart enough not to include the ./b file in the bundle.

All the more reason I think we ought use DefinePlugin and specify a __DEV__ variable, to be replaced with true or false and such optimizations could take effect.

const includes = require( 'lodash/includes' );
return ( state, ...args ) => {
const hasInvalidArg = args.some( ( arg ) => {
return arg && ! includes( VALID_ARG_TYPES, typeof arg );
} );

if ( hasInvalidArg ) {
warn( 'Do not pass complex objects as arguments for a memoized selector' );
}

return args.join();
};
} )();

/**
* Returns a memoized state selector for use with the Redux global application state.
*
* @param {Function} selector Function calculating cached result
* @param {Function} getDependants Function describing dependent state
* @return {Function} Memoized selector
*/
export default function createSelector( selector, getDependants = ( state ) => state ) {
const memoizedSelector = memoize( selector, getCacheKey );
let lastDependants;

return Object.assign( function( state, ...args ) {
let currentDependants = getDependants( state );
if ( ! Array.isArray( currentDependants ) ) {
currentDependants = [ currentDependants ];
}

if ( lastDependants && ! shallowEqual( currentDependants, lastDependants ) ) {
memoizedSelector.cache.clear();
}

lastDependants = currentDependants;

return memoizedSelector( state, ...args );
}, { memoizedSelector } );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we exposing the memoizedSelector to be able to manipulate it's cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we exposing the memoizedSelector to be able to manipulate it's cache?

Yeah, particularly for testing so that the cache can be reset before each test.

}

171 changes: 171 additions & 0 deletions client/lib/create-selector/test/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
/**
* External dependencies
*/
import Chai, { expect } from 'chai';
import filter from 'lodash/filter';
import sinon from 'sinon';
import sinonChai from 'sinon-chai';

/**
* Internal dependencies
*/
import createSelector from '../';

describe( '#createSelector', () => {
let selector, getSitePosts;

before( () => {
Chai.use( sinonChai );

selector = sinon.spy( ( state, siteId ) => {
return filter( state.posts, { site_ID: siteId } );
} );
getSitePosts = createSelector( selector, ( state ) => state.posts );
sinon.stub( console, 'warn' );
} );

beforeEach( () => {
console.warn.reset();
selector.reset();
getSitePosts.memoizedSelector.cache.clear();
} );

after( () => {
console.warn.restore();
} );

it( 'should expose its memoized function', () => {
expect( getSitePosts.memoizedSelector ).to.be.a( 'function' );
} );

it( 'should create a function which returns the expected value when called', () => {
const state = {
posts: {
'3d097cb7c5473c169bba0eb8e3c6cb64': { ID: 841, site_ID: 2916284, global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64', title: 'Hello World' }
}
};

expect( getSitePosts( state, 2916284 ) ).to.eql( [
{ ID: 841, site_ID: 2916284, global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64', title: 'Hello World' }
] );
} );

it( 'should cache the result of a selector function', () => {
const state = {
posts: {
'3d097cb7c5473c169bba0eb8e3c6cb64': { ID: 841, site_ID: 2916284, global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64', title: 'Hello World' }
}
};

getSitePosts( state, 2916284 );
getSitePosts( state, 2916284 );

expect( selector ).to.have.been.calledOnce;
} );

it( 'should warn against complex arguments in development mode', () => {
const state = { posts: {} };

getSitePosts( state, 1 );
getSitePosts( state, '' );
getSitePosts( state, 'foo' );
getSitePosts( state, true );
getSitePosts( state, null );
getSitePosts( state, undefined );
getSitePosts( state, {} );
getSitePosts( state, [] );
getSitePosts( state, 1, [] );

expect( console.warn ).to.have.been.calledThrice;
} );

it( 'should return the expected value of differing arguments', () => {
const state = {
posts: {
'3d097cb7c5473c169bba0eb8e3c6cb64': { ID: 841, site_ID: 2916284, global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64', title: 'Hello World' },
'6c831c187ffef321eb43a67761a525a3': { ID: 413, site_ID: 38303081, global_ID: '6c831c187ffef321eb43a67761a525a3', title: 'Ribs & Chicken' }
}
};

getSitePosts( state, 2916284 );
const sitePosts = getSitePosts( state, 38303081 );
getSitePosts( state, 2916284 );

expect( sitePosts ).to.eql( [
{ ID: 413, site_ID: 38303081, global_ID: '6c831c187ffef321eb43a67761a525a3', title: 'Ribs & Chicken' }
] );
expect( selector ).to.have.been.calledTwice;
} );

it( 'should bust the cache when watched state changes', () => {
const currentState = {
posts: {
'3d097cb7c5473c169bba0eb8e3c6cb64': { ID: 841, site_ID: 2916284, global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64', title: 'Hello World' }
}
};

getSitePosts( currentState, 2916284 );

const nextState = {
posts: {
'3d097cb7c5473c169bba0eb8e3c6cb64': { ID: 841, site_ID: 2916284, global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64', title: 'Hello World' },
'6c831c187ffef321eb43a67761a525a3': { ID: 413, site_ID: 38303081, global_ID: '6c831c187ffef321eb43a67761a525a3', title: 'Ribs & Chicken' }
}
};

expect( getSitePosts( nextState, 2916284 ) ).to.eql( [
{ ID: 841, site_ID: 2916284, global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64', title: 'Hello World' }
] );
expect( selector ).to.have.been.calledTwice;
} );

it( 'should accept an array of dependent state values', () => {
const getSitePostsWithArrayDependants = createSelector( selector, ( state ) => [ state.posts ] );
const state = {
posts: {
'3d097cb7c5473c169bba0eb8e3c6cb64': { ID: 841, site_ID: 2916284, global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64', title: 'Hello World' }
}
};

getSitePostsWithArrayDependants( state, 2916284 );
getSitePostsWithArrayDependants( state, 2916284 );

expect( selector ).to.have.been.calledOnce;
} );

it( 'should default to watching entire state, returning cached result if no changes', () => {
const getSitePostsWithDefaultGetDependants = createSelector( selector );
const state = {
posts: {
'3d097cb7c5473c169bba0eb8e3c6cb64': { ID: 841, site_ID: 2916284, global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64', title: 'Hello World' }
}
};

getSitePostsWithDefaultGetDependants( state, 2916284 );
getSitePostsWithDefaultGetDependants( state, 2916284 );

expect( selector ).to.have.been.calledOnce;
} );

it( 'should default to watching entire state, busting if state has changed', () => {
const getSitePostsWithDefaultGetDependants = createSelector( selector );
const currentState = {
posts: {
'3d097cb7c5473c169bba0eb8e3c6cb64': { ID: 841, site_ID: 2916284, global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64', title: 'Hello World' }
}
};

getSitePostsWithDefaultGetDependants( currentState, 2916284 );

const nextState = {
posts: {
'3d097cb7c5473c169bba0eb8e3c6cb64': { ID: 841, site_ID: 2916284, global_ID: '3d097cb7c5473c169bba0eb8e3c6cb64', title: 'Hello World' },
'6c831c187ffef321eb43a67761a525a3': { ID: 413, site_ID: 38303081, global_ID: '6c831c187ffef321eb43a67761a525a3', title: 'Ribs & Chicken' }
}
};

getSitePostsWithDefaultGetDependants( nextState, 2916284 );

expect( selector ).to.have.been.calledTwice;
} );
} );