-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
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. |
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' ); | ||
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 } ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, particularly for testing so that the cache can be reset before each test. |
||
} | ||
|
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; | ||
} ); | ||
} ); |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:...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 withtrue
orfalse
and such optimizations could take effect.