From 77ece4dbed4bb8d2cca24c22904115e86324fee2 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 1 Nov 2017 16:13:11 -0400 Subject: [PATCH 1/5] State: Refactor post dirty state as middleware side effect --- editor/reducer.js | 2 + editor/selectors.js | 48 +--- editor/state/save-state.js | 72 +++++ editor/state/test/save-state.js | 105 ++++++++ editor/store.js | 3 +- editor/test/selectors.js | 449 ++++++-------------------------- 6 files changed, 260 insertions(+), 419 deletions(-) create mode 100644 editor/state/save-state.js create mode 100644 editor/state/test/save-state.js diff --git a/editor/reducer.js b/editor/reducer.js index 90111e7003c3c7..85ba5d6139b871 100644 --- a/editor/reducer.js +++ b/editor/reducer.js @@ -28,6 +28,7 @@ import { getBlockTypes, getBlockType } from '@wordpress/blocks'; */ import { combineUndoableReducers } from './utils/undoable-reducer'; import { STORE_DEFAULTS } from './store-defaults'; +import saveState from './state/save-state'; /*** * Module constants @@ -658,4 +659,5 @@ export default optimist( combineReducers( { saving, notices, metaBoxes, + saveState, } ) ); diff --git a/editor/selectors.js b/editor/selectors.js index f1cfcbe5a19540..46d909ca0d81ca 100644 --- a/editor/selectors.js +++ b/editor/selectors.js @@ -6,10 +6,8 @@ import { first, get, has, - isEqual, last, reduce, - some, keys, without, compact, @@ -181,49 +179,9 @@ export function isEditedPostNew( state ) { * @param {Object} state Global application state * @return {Boolean} Whether unsaved values exist */ -export const isEditedPostDirty = createSelector( - ( state ) => { - const edits = getPostEdits( state ); - const currentPost = getCurrentPost( state ); - const hasEditedAttributes = some( edits, ( value, key ) => { - return ! isEqual( value, currentPost[ key ] ); - } ); - - if ( hasEditedAttributes ) { - return true; - } - - if ( isMetaBoxStateDirty( state ) ) { - return true; - } - - // This is a cheaper operation that still must occur after checking - // attributes, because a post initialized with attributes different - // from its saved copy should be considered dirty. - if ( ! hasEditorUndo( state ) ) { - return false; - } - - // Check whether there are differences between editor from its original - // state (when history was last reset) and currently. Any difference in - // attributes, block type, order should consistute needing save. - const { history } = state.editor; - const originalEditor = history.past[ 0 ]; - const currentEditor = history.present; - return some( [ - 'blocksByUid', - 'blockOrder', - ], ( key ) => ! isEqual( - originalEditor[ key ], - currentEditor[ key ] - ) ); - }, - ( state ) => [ - state.editor, - state.currentPost, - state.metaBoxes, - ] -); +export function isEditedPostDirty( state ) { + return state.saveState.isDirty || isMetaBoxStateDirty( state ); +} /** * Returns true if there are no unsaved values for the current edit session and if diff --git a/editor/state/save-state.js b/editor/state/save-state.js new file mode 100644 index 00000000000000..7f349224b61012 --- /dev/null +++ b/editor/state/save-state.js @@ -0,0 +1,72 @@ +/** + * External dependencies + */ +import { combineReducers } from 'redux'; + +/** + * Internal dependencies + */ +import { getCurrentPost, hasEditorUndo } from '../selectors'; + +/** + * Action creator returning an action object used in signalling that the post's + * dirty saved state should be toggled. + * + * @param {Boolean} isDirty Whether post is dirty + * @return {Object} Action object + */ +export function toggleDirty( isDirty ) { + return { + type: 'TOGGLE_DIRTY', + isDirty, + }; +} + +/** + * Redux middleware for save state dirtying tracking. + * + * @param {Function} store Redux store object + * @return {Function} Middleware function + */ +export const middleware = ( store ) => ( next ) => ( action ) => { + const state = store.getState(); + const result = next( action ); + const nextState = store.getState(); + + let isDirty; + if ( getCurrentPost( nextState ) !== getCurrentPost( state ) ) { + // Current post reflects last known post save, so if these references + // differ, we can assume that the post has been saved + isDirty = false; + } else if ( state.editor !== nextState.editor && hasEditorUndo( nextState ) ) { + // Editor state tracks post attribute edits, block order, block + // attributes. If any of these change, assume the post is dirtied. + // Without undo indicates an editor history reset (initial load). + isDirty = true; + } + + // Avoid dispatch except when we know that the state is changing + if ( undefined !== isDirty && isDirty !== nextState.saveState.isDirty ) { + store.dispatch( toggleDirty( isDirty ) ); + } + + return result; +}; + +export default combineReducers( { + /** + * Reducer reflecting whether the post is considered dirty (needing save). + * + * @param {Boolean} state Original state + * @param {Object} action Action object + * @return {Boolean} Next state + */ + isDirty( state = false, action ) { + switch ( action.type ) { + case 'TOGGLE_DIRTY': + return action.isDirty; + } + + return state; + }, +} ); diff --git a/editor/state/test/save-state.js b/editor/state/test/save-state.js new file mode 100644 index 00000000000000..d814a97c1a432a --- /dev/null +++ b/editor/state/test/save-state.js @@ -0,0 +1,105 @@ +/** + * External dependencies + */ +import { noop } from 'lodash'; +import deepFreeze from 'deep-freeze'; + +/** + * Internal dependencies + */ +import reducer, { toggleDirty, middleware } from '../save-state'; +import * as selectors from '../../selectors'; + +jest.mock( '../../selectors' ); + +describe( 'saveState', () => { + beforeEach( () => jest.resetAllMocks() ); + + describe( '.middleware()', () => { + const dispatch = jest.fn(); + + let isDirty, editor; + const store = { + getState() { + return { + saveState: { + isDirty, + }, + editor, + }; + }, + dispatch, + }; + + it( 'should dispatch toggle dirty false when post reset', () => { + isDirty = true; + // Return new object reference for each call, strictly unequal + selectors.getCurrentPost.mockImplementation( () => ( {} ) ); + + middleware( store )( noop )( {} ); + + expect( store.dispatch ).toHaveBeenCalledWith( toggleDirty( false ) ); + } ); + + it( 'should dispatch toggle dirty true when editor changes', () => { + isDirty = false; + const currentPost = {}; + selectors.getCurrentPost.mockReturnValue( currentPost ); + selectors.hasEditorUndo.mockReturnValue( true ); + + editor = {}; + middleware( store )( () => { + // Set editor as new reference, strictly unequal to old + editor = {}; + } )( {} ); + + expect( store.dispatch ).toHaveBeenCalledWith( toggleDirty( true ) ); + } ); + + it( 'should not dispatch toggle dirty when post has no undo history', () => { + isDirty = false; + const currentPost = {}; + selectors.getCurrentPost.mockReturnValue( currentPost ); + selectors.hasEditorUndo.mockReturnValue( false ); + + middleware( store )( noop )( {} ); + + expect( store.dispatch ).not.toHaveBeenCalled(); + } ); + + it( 'should not dispatch toggle if already the same', () => { + isDirty = true; + const currentPost = {}; + selectors.getCurrentPost.mockReturnValue( currentPost ); + selectors.hasEditorUndo.mockReturnValue( true ); + + editor = {}; + middleware( store )( () => { + // Set editor as new reference, strictly unequal to old + editor = {}; + } )( {} ); + + expect( store.dispatch ).not.toHaveBeenCalled(); + } ); + } ); + + describe( 'reducer', () => { + describe( '.isDirty()', () => { + it( 'should default to false', () => { + const state = reducer( undefined, {} ); + + expect( state.isDirty ).toBe( false ); + } ); + + it( 'should return value by toggle property', () => { + const originalState = deepFreeze( reducer( undefined, {} ) ); + const state = reducer( originalState, { + type: 'TOGGLE_DIRTY', + isDirty: true, + } ); + + expect( state.isDirty ).toBe( true ); + } ); + } ); + } ); +} ); diff --git a/editor/store.js b/editor/store.js index fa2e7db7dfc7c4..d56c40bb9c9033 100644 --- a/editor/store.js +++ b/editor/store.js @@ -12,6 +12,7 @@ import { flowRight } from 'lodash'; import effects from './effects'; import reducer from './reducer'; import storePersist from './store-persist'; +import { middleware as saveStateMiddleware } from './state/save-state'; /** * Module constants @@ -25,7 +26,7 @@ const GUTENBERG_PREFERENCES_KEY = `GUTENBERG_PREFERENCES_${ window.userSettings. */ function createReduxStore() { const enhancers = [ - applyMiddleware( multi, refx( effects ) ), + applyMiddleware( multi, refx( effects ), saveStateMiddleware ), storePersist( 'preferences', GUTENBERG_PREFERENCES_KEY ), ]; diff --git a/editor/test/selectors.js b/editor/test/selectors.js index a1252667f4ee15..b396a18bf8f99b 100644 --- a/editor/test/selectors.js +++ b/editor/test/selectors.js @@ -76,16 +76,6 @@ import { } from '../selectors'; describe( 'selectors', () => { - function getEditorState( states ) { - const past = [ ...states ]; - const present = past.pop(); - - return { - ...present, - history: { past, present }, - }; - } - beforeAll( () => { registerBlockType( 'core/test-block', { save: ( props ) => props.attributes.text, @@ -96,7 +86,6 @@ describe( 'selectors', () => { beforeEach( () => { getDirtyMetaBoxes.clear(); - isEditedPostDirty.clear(); getBlock.clear(); getBlocks.clear(); getEditedPostContent.clear(); @@ -478,312 +467,33 @@ describe( 'selectors', () => { }, }; - it( 'should return true when the post has edited attributes', () => { + it( 'should return true when post saved state dirty', () => { const state = { - currentPost: { - title: '', + saveState: { + isDirty: true, }, - editor: getEditorState( [ - { - edits: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - blocksByUid: {}, - blockOrder: [], - }, - ] ), metaBoxes, }; expect( isEditedPostDirty( state ) ).toBe( true ); } ); - it( 'should return false when the post has no edited attributes and no past', () => { + it( 'should return false when post saved state not dirty', () => { const state = { - currentPost: { - title: 'The Meat Eater\'s Guide to Delicious Meats', + saveState: { + isDirty: false, }, - editor: getEditorState( [ - { - edits: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - blocksByUid: {}, - blockOrder: [], - }, - ] ), metaBoxes, }; expect( isEditedPostDirty( state ) ).toBe( false ); } ); - it( 'should return false when the post has no edited attributes', () => { + it( 'should return true when post saved state not dirty, but meta box state has changed.', () => { const state = { - currentPost: { - title: 'The Meat Eater\'s Guide to Delicious Meats', + saveState: { + isDirty: false, }, - editor: getEditorState( [ - { - edits: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - blocksByUid: {}, - blockOrder: [], - }, - { - edits: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - blocksByUid: { - 123: { - name: 'core/food', - attributes: { name: 'Chicken', delicious: false }, - }, - }, - blockOrder: [ - 123, - ], - }, - { - edits: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - blocksByUid: {}, - blockOrder: [], - }, - ] ), - metaBoxes, - }; - - expect( isEditedPostDirty( state ) ).toBe( false ); - } ); - - it( 'should return true when the post has edited block attributes', () => { - const state = { - currentPost: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - editor: getEditorState( [ - { - edits: {}, - blocksByUid: { - 123: { - name: 'core/food', - attributes: { name: 'Chicken', delicious: false }, - }, - }, - blockOrder: [ - 123, - ], - }, - { - edits: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - blocksByUid: { - 123: { - name: 'core/food', - attributes: { name: 'Chicken', delicious: true }, - }, - }, - blockOrder: [ - 123, - ], - }, - ] ), - metaBoxes, - }; - - expect( isEditedPostDirty( state ) ).toBe( true ); - } ); - - it( 'should return true when the post has new blocks', () => { - const state = { - currentPost: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - editor: getEditorState( [ - { - edits: {}, - blocksByUid: { - 123: { - name: 'core/food', - attributes: { name: 'Chicken', delicious: true }, - }, - }, - blockOrder: [ - 123, - 456, - ], - }, - { - edits: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - blocksByUid: { - 123: { - name: 'core/food', - attributes: { name: 'Chicken', delicious: true }, - }, - 456: { - name: 'core/food', - attributes: { name: 'Ribs', delicious: true }, - }, - }, - blockOrder: [ - 123, - 456, - ], - }, - ] ), - metaBoxes, - }; - - expect( isEditedPostDirty( state ) ).toBe( true ); - } ); - - it( 'should return true when the post has changed block order', () => { - const state = { - currentPost: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - editor: getEditorState( [ - { - edits: {}, - blocksByUid: { - 123: { - name: 'core/food', - attributes: { name: 'Chicken', delicious: true }, - }, - 456: { - name: 'core/food', - attributes: { name: 'Ribs', delicious: true }, - }, - }, - blockOrder: [ - 123, - 456, - ], - }, - { - edits: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - blocksByUid: { - 123: { - name: 'core/food', - attributes: { name: 'Chicken', delicious: true }, - }, - 456: { - name: 'core/food', - attributes: { name: 'Ribs', delicious: true }, - }, - }, - blockOrder: [ - 456, - 123, - ], - }, - ] ), - metaBoxes, - }; - - expect( isEditedPostDirty( state ) ).toBe( true ); - } ); - - it( 'should return false when no edits, no changed block attributes, no changed order', () => { - const state = { - currentPost: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - editor: getEditorState( [ - { - edits: {}, - blocksByUid: { - 123: { - name: 'core/food', - attributes: { name: 'Chicken', delicious: true }, - }, - 456: { - name: 'core/food', - attributes: { name: 'Ribs', delicious: true }, - }, - }, - blockOrder: [ - 456, - 123, - ], - }, - { - edits: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - blocksByUid: { - 123: { - name: 'core/food', - attributes: { name: 'Chicken', delicious: true }, - }, - 456: { - name: 'core/food', - attributes: { name: 'Ribs', delicious: true }, - }, - }, - blockOrder: [ - 456, - 123, - ], - }, - ] ), - metaBoxes, - }; - - expect( isEditedPostDirty( state ) ).toBe( false ); - } ); - - it( 'should return true when no edits, no changed block attributes, no changed order, but meta box state has changed.', () => { - const state = { - currentPost: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - editor: getEditorState( [ - { - edits: {}, - blocksByUid: { - 123: { - name: 'core/food', - attributes: { name: 'Chicken', delicious: true }, - }, - 456: { - name: 'core/food', - attributes: { name: 'Ribs', delicious: true }, - }, - }, - blockOrder: [ - 456, - 123, - ], - }, - { - edits: { - title: 'The Meat Eater\'s Guide to Delicious Meats', - }, - blocksByUid: { - 123: { - name: 'core/food', - attributes: { name: 'Chicken', delicious: true }, - }, - 456: { - name: 'core/food', - attributes: { name: 'Ribs', delicious: true }, - }, - }, - blockOrder: [ - 456, - 123, - ], - }, - ] ), metaBoxes: dirtyMetaBoxes, }; @@ -796,11 +506,9 @@ describe( 'selectors', () => { it( 'should return true when the post is not dirty and has not been saved before', () => { const state = { - editor: getEditorState( [ - { - edits: {}, - }, - ] ), + saveState: { + isDirty: false, + }, currentPost: { id: 1, status: 'auto-draft', @@ -813,11 +521,9 @@ describe( 'selectors', () => { it( 'should return false when the post is not dirty but the post has been saved', () => { const state = { - editor: getEditorState( [ - { - edits: {}, - }, - ] ), + saveState: { + isDirty: false, + }, currentPost: { id: 1, status: 'draft', @@ -830,14 +536,9 @@ describe( 'selectors', () => { it( 'should return false when the post is dirty but the post has not been saved', () => { const state = { - editor: getEditorState( [ - { - edits: {}, - }, - { - edits: { title: 'Dirty' }, - }, - ] ), + saveState: { + isDirty: true, + }, currentPost: { id: 1, status: 'auto-draft', @@ -977,15 +678,16 @@ describe( 'selectors', () => { const metaBoxes = { isDirty: false, isUpdating: false }; it( 'should return current title unedited existing post', () => { const state = { + saveState: { + isDirty: false, + }, currentPost: { id: 123, title: 'The Title', }, - editor: getEditorState( [ - { - edits: {}, - }, - ] ), + editor: { + edits: {}, + }, metaBoxes, }; @@ -998,14 +700,11 @@ describe( 'selectors', () => { id: 123, title: 'The Title', }, - editor: getEditorState( [ - { - edits: {}, - }, - { - edits: { title: 'Modified Title' }, + editor: { + edits: { + title: 'Modified Title', }, - ] ), + }, metaBoxes, }; @@ -1014,16 +713,17 @@ describe( 'selectors', () => { it( 'should return new post title when new post is clean', () => { const state = { + saveState: { + isDirty: false, + }, currentPost: { id: 1, status: 'auto-draft', title: '', }, - editor: getEditorState( [ - { - edits: {}, - }, - ] ), + editor: { + edits: {}, + }, metaBoxes, }; @@ -1032,16 +732,17 @@ describe( 'selectors', () => { it( 'should return untitled title', () => { const state = { + saveState: { + isDirty: true, + }, currentPost: { id: 123, status: 'draft', title: '', }, - editor: getEditorState( [ - { - edits: {}, - }, - ] ), + editor: { + edits: {}, + }, metaBoxes, }; @@ -1184,14 +885,12 @@ describe( 'selectors', () => { it( 'should return true for pending posts', () => { const state = { + saveState: { + isDirty: false, + }, currentPost: { status: 'pending', }, - editor: getEditorState( [ - { - edits: {}, - }, - ] ), metaBoxes, }; @@ -1200,14 +899,12 @@ describe( 'selectors', () => { it( 'should return true for draft posts', () => { const state = { + saveState: { + isDirty: false, + }, currentPost: { status: 'draft', }, - editor: getEditorState( [ - { - edits: {}, - }, - ] ), metaBoxes, }; @@ -1216,30 +913,40 @@ describe( 'selectors', () => { it( 'should return false for published posts', () => { const state = { + saveState: { + isDirty: false, + }, currentPost: { status: 'publish', }, - editor: getEditorState( [ - { - edits: {}, - }, - ] ), metaBoxes, }; expect( isEditedPostPublishable( state ) ).toBe( false ); } ); + it( 'should return true for published, dirty posts', () => { + const state = { + saveState: { + isDirty: true, + }, + currentPost: { + status: 'publish', + }, + metaBoxes, + }; + + expect( isEditedPostPublishable( state ) ).toBe( true ); + } ); + it( 'should return false for private posts', () => { const state = { + saveState: { + isDirty: false, + }, currentPost: { status: 'private', }, - editor: getEditorState( [ - { - edits: {}, - }, - ] ), metaBoxes, }; @@ -1248,14 +955,12 @@ describe( 'selectors', () => { it( 'should return false for scheduled posts', () => { const state = { + saveState: { + isDirty: false, + }, currentPost: { - status: 'private', + status: 'future', }, - editor: getEditorState( [ - { - edits: {}, - }, - ] ), metaBoxes, }; @@ -1264,17 +969,15 @@ describe( 'selectors', () => { it( 'should return true for dirty posts with usable title', () => { const state = { + saveState: { + isDirty: true, + }, currentPost: { status: 'private', }, - editor: getEditorState( [ - { - edits: {}, - }, - { - edits: { title: 'Dirty' }, - }, - ] ), + editor: { + edits: { title: 'Dirty' }, + }, metaBoxes, }; From 36655bdea210532e88ea8001b8fc86e5ecaf1508 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 3 Nov 2017 10:12:20 -0400 Subject: [PATCH 2/5] State: Implement dirtyingReducer higher-order reducer --- editor/utils/dirtying-reducer/README.md | 41 +++++++++ editor/utils/dirtying-reducer/index.js | 50 +++++++++++ editor/utils/dirtying-reducer/test/index.js | 99 +++++++++++++++++++++ 3 files changed, 190 insertions(+) create mode 100644 editor/utils/dirtying-reducer/README.md create mode 100644 editor/utils/dirtying-reducer/index.js create mode 100644 editor/utils/dirtying-reducer/test/index.js diff --git a/editor/utils/dirtying-reducer/README.md b/editor/utils/dirtying-reducer/README.md new file mode 100644 index 00000000000000..1e547b50bb78e9 --- /dev/null +++ b/editor/utils/dirtying-reducer/README.md @@ -0,0 +1,41 @@ +Dirtying Reducer +================ + +`dirtyingReducer` is a [Redux higher-order reducer](http://redux.js.org/docs/recipes/reducers/ReusingReducerLogic.html#customizing-behavior-with-higher-order-reducers) for tracking changes to reducer state over time. + +It does this based on the following assumptions: + +- The original reducer returns an object +- The original reducer returns a new reference only if a change has in-fact occurred + +Using these assumptions, the enhanced reducer returned from `dirtyingReducer` will include a new property on the object `isDirty` corresponding to whether the original reference of the reducer has ever changed. + +Leveraging a `resetTypes` option, this can be used to mark intervals at which a state is considered to be clean (without changes) and dirty (with changes). + +## Example + +Considering a simple count reducer, we can enhance it with `dirtyingReducer` to reflect whether changes have occurred: + +```js +function counter( state = { count: 0 }, action ) { + switch ( action.type ) { + case 'INCREMENT': + return { ...state, count: state.count + 1 }; + } + + return state; +} + +const enhancedCounter = dirtyingReducer( counter, { resetTypes: [ 'RESET' ] } ); + +let state; + +state = enhancedCounter( undefined, {} ); +// { count: 0, isDirty: false } + +state = enhancedCounter( state, { type: 'INCREMENT' } ); +// { count: 1, isDirty: true } + +state = enhancedCounter( state, { type: 'RESET' } ); +// { count: 1, isDirty: false } +``` diff --git a/editor/utils/dirtying-reducer/index.js b/editor/utils/dirtying-reducer/index.js new file mode 100644 index 00000000000000..dcb29b33147983 --- /dev/null +++ b/editor/utils/dirtying-reducer/index.js @@ -0,0 +1,50 @@ +/** + * External dependencies + */ +import { includes } from 'lodash'; + +/** + * Reducer enhancer for tracking changes to reducer state over time. The + * returned reducer will include a new `isDirty` property on the object + * reflecting whether the original reference of the reducer has changed. + * + * @param {Function} reducer Original reducer + * @param {?Object} options Optional options + * @param {?Array} options.resetTypes Action types upon which to reset dirty + * @return {Function} Enhanced reducer + */ +export default function dirtyingReducer( reducer, options = {} ) { + let originalState; + + return ( state, action ) => { + let nextState = reducer( state, action ); + + // Reset at: + // - Initial state + // - Reset types + const isReset = ( + state === undefined || + includes( options.resetTypes, action.type ) + ); + + const nextIsDirty = ! isReset && originalState !== nextState; + + // Only revise state if changing. + if ( nextIsDirty !== nextState.isDirty ) { + // In case the original reducer returned the same reference and we + // intend to mutate, create a shallow clone. + if ( state === nextState ) { + nextState = { ...nextState }; + } + + nextState.isDirty = nextIsDirty; + } + + // Track original state against which dirty test compares reference + if ( isReset ) { + originalState = nextState; + } + + return nextState; + }; +} diff --git a/editor/utils/dirtying-reducer/test/index.js b/editor/utils/dirtying-reducer/test/index.js new file mode 100644 index 00000000000000..f01e04dae0d8ca --- /dev/null +++ b/editor/utils/dirtying-reducer/test/index.js @@ -0,0 +1,99 @@ +/** + * External dependencies + */ +import deepFreeze from 'deep-freeze'; + +/** + * Internal dependencies + */ +import dirtyingReducer from '../'; + +describe( 'dirtyingReducer()', () => { + const initialState = { count: 0 }; + + function originalReducer( state = initialState, action ) { + switch ( action.type ) { + case 'INCREMENT': + return { ...state, count: state.count + 1 }; + + case 'RESET_AND_CHANGE_REFERENCE': + return { ...state }; + + case 'SET_STATE': + return action.state; + } + + return state; + } + + it( 'should respect original reducer behavior', () => { + const reducer = dirtyingReducer( originalReducer ); + + const state = reducer( undefined, {} ); + expect( state ).toEqual( { count: 0, isDirty: false } ); + + const nextState = reducer( deepFreeze( state ), { type: 'INCREMENT' } ); + expect( nextState ).not.toBe( state ); + expect( nextState ).toEqual( { count: 1, isDirty: true } ); + } ); + + it( 'should allow reset types as option', () => { + const reducer = dirtyingReducer( originalReducer, { resetTypes: [ 'RESET' ] } ); + + let state; + + state = reducer( undefined, {} ); + expect( state ).toEqual( { count: 0, isDirty: false } ); + + state = reducer( deepFreeze( state ), { type: 'INCREMENT' } ); + expect( state ).toEqual( { count: 1, isDirty: true } ); + + state = reducer( deepFreeze( state ), { type: 'RESET' } ); + expect( state ).toEqual( { count: 1, isDirty: false } ); + } ); + + it( 'should preserve isDirty into non-resetting non-reference-changing types', () => { + const reducer = dirtyingReducer( originalReducer, { resetTypes: [ 'RESET' ] } ); + + let state; + + state = reducer( undefined, {} ); + expect( state ).toEqual( { count: 0, isDirty: false } ); + + state = reducer( deepFreeze( state ), { type: 'INCREMENT' } ); + expect( state ).toEqual( { count: 1, isDirty: true } ); + + state = reducer( deepFreeze( state ), {} ); + expect( state ).toEqual( { count: 1, isDirty: true } ); + } ); + + it( 'should reset if state reverts to its original reference', () => { + const reducer = dirtyingReducer( originalReducer, { resetTypes: [ 'RESET' ] } ); + + let state; + + const originalState = state = reducer( undefined, {} ); + expect( state ).toEqual( { count: 0, isDirty: false } ); + + state = reducer( deepFreeze( state ), { type: 'INCREMENT' } ); + expect( state ).toEqual( { count: 1, isDirty: true } ); + + state = reducer( deepFreeze( state ), { type: 'SET_STATE', state: originalState } ); + expect( state ).toEqual( { count: 0, isDirty: false } ); + } ); + + it( 'should flag as not dirty even if reset type causes reference change', () => { + const reducer = dirtyingReducer( originalReducer, { resetTypes: [ 'RESET_AND_CHANGE_REFERENCE' ] } ); + + let state; + + state = reducer( undefined, {} ); + expect( state ).toEqual( { count: 0, isDirty: false } ); + + state = reducer( deepFreeze( state ), { type: 'INCREMENT' } ); + expect( state ).toEqual( { count: 1, isDirty: true } ); + + state = reducer( deepFreeze( state ), { type: 'RESET_AND_CHANGE_REFERENCE' } ); + expect( state ).toEqual( { count: 1, isDirty: false } ); + } ); +} ); From cdad65a3913bce2792a04af4bd7b7d48ca33fd00 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 3 Nov 2017 10:30:58 -0400 Subject: [PATCH 3/5] Utils: Remove getters handling from undoable reducer Adds complexity (preserving getters/setters through cloning), obscurity (magic proxying of keys to history subkey), and is not respecting of desired plain object structure of state. --- editor/reducer.js | 13 +- editor/selectors.js | 62 ++-- editor/test/reducer.js | 74 ++--- editor/test/selectors.js | 390 +++++++++++++++++--------- editor/utils/test/undoable-reducer.js | 195 ++++++------- editor/utils/undoable-reducer.js | 46 +-- 6 files changed, 418 insertions(+), 362 deletions(-) diff --git a/editor/reducer.js b/editor/reducer.js index 85ba5d6139b871..d5f5680dd6a72a 100644 --- a/editor/reducer.js +++ b/editor/reducer.js @@ -4,6 +4,8 @@ import optimist from 'redux-optimist'; import { combineReducers } from 'redux'; import { + flow, + partialRight, difference, get, reduce, @@ -26,7 +28,7 @@ import { getBlockTypes, getBlockType } from '@wordpress/blocks'; /** * Internal dependencies */ -import { combineUndoableReducers } from './utils/undoable-reducer'; +import undoableReducer from './utils/undoable-reducer'; import { STORE_DEFAULTS } from './store-defaults'; import saveState from './state/save-state'; @@ -64,7 +66,12 @@ export function getPostRawValue( value ) { * @param {Object} action Dispatched action * @return {Object} Updated state */ -export const editor = combineUndoableReducers( { +export const editor = flow( [ + combineReducers, + + // Track undo history, starting at editor initialization. + partialRight( undoableReducer, { resetTypes: [ 'SETUP_EDITOR' ] } ), +] )( { edits( state = {}, action ) { switch ( action.type ) { case 'EDIT_POST': @@ -262,7 +269,7 @@ export const editor = combineUndoableReducers( { return state; }, -}, { resetTypes: [ 'SETUP_EDITOR' ] } ); +} ); /** * Reducer returning the last-known state of the current post, in the format diff --git a/editor/selectors.js b/editor/selectors.js index 46d909ca0d81ca..f358eebfa4da8a 100644 --- a/editor/selectors.js +++ b/editor/selectors.js @@ -147,7 +147,7 @@ export function isEditorSidebarPanelOpened( state, panel ) { * @return {Boolean} Whether undo history exists */ export function hasEditorUndo( state ) { - return state.editor.history.past.length > 0; + return state.editor.past.length > 0; } /** @@ -158,7 +158,7 @@ export function hasEditorUndo( state ) { * @return {Boolean} Whether redo history exists */ export function hasEditorRedo( state ) { - return state.editor.history.future.length > 0; + return state.editor.future.length > 0; } /** @@ -256,7 +256,7 @@ export function getCurrentPostLastRevisionId( state ) { * @return {Object} Object of key value pairs comprising unsaved edits */ export function getPostEdits( state ) { - return state.editor.edits; + return state.editor.present.edits; } /** @@ -269,9 +269,9 @@ export function getPostEdits( state ) { * @return {*} Post attribute value */ export function getEditedPostAttribute( state, attributeName ) { - return state.editor.edits[ attributeName ] === undefined ? + return state.editor.present.edits[ attributeName ] === undefined ? state.currentPost[ attributeName ] : - state.editor.edits[ attributeName ]; + state.editor.present.edits[ attributeName ]; } /** @@ -390,9 +390,9 @@ export function getDocumentTitle( state ) { * @return {String} Raw post excerpt */ export function getEditedPostExcerpt( state ) { - return state.editor.edits.excerpt === undefined ? + return state.editor.present.edits.excerpt === undefined ? state.currentPost.excerpt : - state.editor.edits.excerpt; + state.editor.present.edits.excerpt; } /** @@ -422,7 +422,7 @@ export function getEditedPostPreviewLink( state ) { */ export const getBlock = createSelector( ( state, uid ) => { - const block = state.editor.blocksByUid[ uid ]; + const block = state.editor.present.blocksByUid[ uid ]; if ( ! block ) { return null; } @@ -475,11 +475,11 @@ function getPostMeta( state, key ) { */ export const getBlocks = createSelector( ( state ) => { - return state.editor.blockOrder.map( ( uid ) => getBlock( state, uid ) ); + return state.editor.present.blockOrder.map( ( uid ) => getBlock( state, uid ) ); }, ( state ) => [ - state.editor.blockOrder, - state.editor.blocksByUid, + state.editor.present.blockOrder, + state.editor.present.blocksByUid, ] ); @@ -533,7 +533,7 @@ export function getSelectedBlock( state ) { */ export const getMultiSelectedBlockUids = createSelector( ( state ) => { - const { blockOrder } = state.editor; + const { blockOrder } = state.editor.present; const { start, end } = state.blockSelection; if ( start === end ) { return []; @@ -549,7 +549,7 @@ export const getMultiSelectedBlockUids = createSelector( return blockOrder.slice( startIndex, endIndex + 1 ); }, ( state ) => [ - state.editor.blockOrder, + state.editor.present.blockOrder, state.blockSelection.start, state.blockSelection.end, ], @@ -565,11 +565,11 @@ export const getMultiSelectedBlockUids = createSelector( export const getMultiSelectedBlocks = createSelector( ( state ) => getMultiSelectedBlockUids( state ).map( ( uid ) => getBlock( state, uid ) ), ( state ) => [ - state.editor.blockOrder, + state.editor.present.blockOrder, state.blockSelection.start, state.blockSelection.end, - state.editor.blocksByUid, - state.editor.edits.meta, + state.editor.present.blocksByUid, + state.editor.present.edits.meta, state.currentPost.meta, ] ); @@ -665,7 +665,7 @@ export function getMultiSelectedBlocksEndUid( state ) { * @return {Array} Ordered unique IDs of post blocks */ export function getBlockUids( state ) { - return state.editor.blockOrder; + return state.editor.present.blockOrder; } /** @@ -677,7 +677,7 @@ export function getBlockUids( state ) { * @return {Number} Index at which block exists in order */ export function getBlockIndex( state, uid ) { - return state.editor.blockOrder.indexOf( uid ); + return state.editor.present.blockOrder.indexOf( uid ); } /** @@ -689,7 +689,7 @@ export function getBlockIndex( state, uid ) { * @return {Boolean} Whether block is first in post */ export function isFirstBlock( state, uid ) { - return first( state.editor.blockOrder ) === uid; + return first( state.editor.present.blockOrder ) === uid; } /** @@ -701,7 +701,7 @@ export function isFirstBlock( state, uid ) { * @return {Boolean} Whether block is last in post */ export function isLastBlock( state, uid ) { - return last( state.editor.blockOrder ) === uid; + return last( state.editor.present.blockOrder ) === uid; } /** @@ -714,7 +714,7 @@ export function isLastBlock( state, uid ) { */ export function getPreviousBlock( state, uid ) { const order = getBlockIndex( state, uid ); - return state.editor.blocksByUid[ state.editor.blockOrder[ order - 1 ] ] || null; + return state.editor.present.blocksByUid[ state.editor.present.blockOrder[ order - 1 ] ] || null; } /** @@ -727,7 +727,7 @@ export function getPreviousBlock( state, uid ) { */ export function getNextBlock( state, uid ) { const order = getBlockIndex( state, uid ); - return state.editor.blocksByUid[ state.editor.blockOrder[ order + 1 ] ] || null; + return state.editor.present.blocksByUid[ state.editor.present.blockOrder[ order + 1 ] ] || null; } /** @@ -818,7 +818,7 @@ export function isTyping( state ) { */ export function getBlockInsertionPoint( state ) { if ( getEditorMode( state ) !== 'visual' ) { - return state.editor.blockOrder.length; + return state.editor.present.blockOrder.length; } const position = getBlockSiblingInserterPosition( state ); @@ -836,7 +836,7 @@ export function getBlockInsertionPoint( state ) { return getBlockIndex( state, selectedBlock.uid ) + 1; } - return state.editor.blockOrder.length; + return state.editor.present.blockOrder.length; } /** @@ -906,20 +906,20 @@ export function didPostSaveRequestFail( state ) { * @return {?String} Suggested post format */ export function getSuggestedPostFormat( state ) { - const blocks = state.editor.blockOrder; + const blocks = state.editor.present.blockOrder; let name; // If there is only one block in the content of the post grab its name // so we can derive a suitable post format from it. if ( blocks.length === 1 ) { - name = state.editor.blocksByUid[ blocks[ 0 ] ].name; + name = state.editor.present.blocksByUid[ blocks[ 0 ] ].name; } // If there are two blocks in the content and the last one is a text blocks // grab the name of the first one to also suggest a post format from it. if ( blocks.length === 2 ) { - if ( state.editor.blocksByUid[ blocks[ 1 ] ].name === 'core/paragraph' ) { - name = state.editor.blocksByUid[ blocks[ 0 ] ].name; + if ( state.editor.present.blocksByUid[ blocks[ 1 ] ].name === 'core/paragraph' ) { + name = state.editor.present.blocksByUid[ blocks[ 0 ] ].name; } } @@ -962,9 +962,9 @@ export const getEditedPostContent = createSelector( return serialize( getBlocks( state ) ); }, ( state ) => [ - state.editor.edits.content, - state.editor.blocksByUid, - state.editor.blockOrder, + state.editor.present.edits.content, + state.editor.present.blocksByUid, + state.editor.present.blockOrder, ], ); diff --git a/editor/test/reducer.js b/editor/test/reducer.js index be49a7badf7d9a..4315f87c258764 100644 --- a/editor/test/reducer.js +++ b/editor/test/reducer.js @@ -56,12 +56,12 @@ describe( 'state', () => { unregisterBlockType( 'core/test-block' ); } ); - it( 'should return empty blocksByUid, blockOrder, history by default', () => { + it( 'should return empty edits, blocksByUid, blockOrder by default', () => { const state = editor( undefined, {} ); - expect( state.blocksByUid ).toEqual( {} ); - expect( state.blockOrder ).toEqual( [] ); - expect( state ).toHaveProperty( 'history' ); + expect( state.present.edits ).toEqual( {} ); + expect( state.present.blocksByUid ).toEqual( {} ); + expect( state.present.blockOrder ).toEqual( [] ); } ); it( 'should key by replaced blocks uid', () => { @@ -71,9 +71,9 @@ describe( 'state', () => { blocks: [ { uid: 'bananas' } ], } ); - expect( Object.keys( state.blocksByUid ) ).toHaveLength( 1 ); - expect( values( state.blocksByUid )[ 0 ].uid ).toBe( 'bananas' ); - expect( state.blockOrder ).toEqual( [ 'bananas' ] ); + expect( Object.keys( state.present.blocksByUid ) ).toHaveLength( 1 ); + expect( values( state.present.blocksByUid )[ 0 ].uid ).toBe( 'bananas' ); + expect( state.present.blockOrder ).toEqual( [ 'bananas' ] ); } ); it( 'should insert block', () => { @@ -93,9 +93,9 @@ describe( 'state', () => { } ], } ); - expect( Object.keys( state.blocksByUid ) ).toHaveLength( 2 ); - expect( values( state.blocksByUid )[ 1 ].uid ).toBe( 'ribs' ); - expect( state.blockOrder ).toEqual( [ 'chicken', 'ribs' ] ); + expect( Object.keys( state.present.blocksByUid ) ).toHaveLength( 2 ); + expect( values( state.present.blocksByUid )[ 1 ].uid ).toBe( 'ribs' ); + expect( state.present.blockOrder ).toEqual( [ 'chicken', 'ribs' ] ); } ); it( 'should replace the block', () => { @@ -116,10 +116,10 @@ describe( 'state', () => { } ], } ); - expect( Object.keys( state.blocksByUid ) ).toHaveLength( 1 ); - expect( values( state.blocksByUid )[ 0 ].name ).toBe( 'core/freeform' ); - expect( values( state.blocksByUid )[ 0 ].uid ).toBe( 'wings' ); - expect( state.blockOrder ).toEqual( [ 'wings' ] ); + expect( Object.keys( state.present.blocksByUid ) ).toHaveLength( 1 ); + expect( values( state.present.blocksByUid )[ 0 ].name ).toBe( 'core/freeform' ); + expect( values( state.present.blocksByUid )[ 0 ].uid ).toBe( 'wings' ); + expect( state.present.blockOrder ).toEqual( [ 'wings' ] ); } ); it( 'should update the block', () => { @@ -141,7 +141,7 @@ describe( 'state', () => { }, } ); - expect( state.blocksByUid.chicken ).toEqual( { + expect( state.present.blocksByUid.chicken ).toEqual( { uid: 'chicken', name: 'core/test-block', attributes: { content: 'ribs' }, @@ -167,7 +167,7 @@ describe( 'state', () => { uids: [ 'ribs' ], } ); - expect( state.blockOrder ).toEqual( [ 'ribs', 'chicken' ] ); + expect( state.present.blockOrder ).toEqual( [ 'ribs', 'chicken' ] ); } ); it( 'should move multiple blocks up', () => { @@ -192,7 +192,7 @@ describe( 'state', () => { uids: [ 'ribs', 'veggies' ], } ); - expect( state.blockOrder ).toEqual( [ 'ribs', 'veggies', 'chicken' ] ); + expect( state.present.blockOrder ).toEqual( [ 'ribs', 'veggies', 'chicken' ] ); } ); it( 'should not move the first block up', () => { @@ -213,7 +213,7 @@ describe( 'state', () => { uids: [ 'chicken' ], } ); - expect( state.blockOrder ).toBe( original.blockOrder ); + expect( state.present.blockOrder ).toBe( original.present.blockOrder ); } ); it( 'should move the block down', () => { @@ -234,7 +234,7 @@ describe( 'state', () => { uids: [ 'chicken' ], } ); - expect( state.blockOrder ).toEqual( [ 'ribs', 'chicken' ] ); + expect( state.present.blockOrder ).toEqual( [ 'ribs', 'chicken' ] ); } ); it( 'should move multiple blocks down', () => { @@ -259,7 +259,7 @@ describe( 'state', () => { uids: [ 'chicken', 'ribs' ], } ); - expect( state.blockOrder ).toEqual( [ 'veggies', 'chicken', 'ribs' ] ); + expect( state.present.blockOrder ).toEqual( [ 'veggies', 'chicken', 'ribs' ] ); } ); it( 'should not move the last block down', () => { @@ -280,7 +280,7 @@ describe( 'state', () => { uids: [ 'ribs' ], } ); - expect( state.blockOrder ).toBe( original.blockOrder ); + expect( state.present.blockOrder ).toBe( original.present.blockOrder ); } ); it( 'should remove the block', () => { @@ -301,8 +301,8 @@ describe( 'state', () => { uids: [ 'chicken' ], } ); - expect( state.blockOrder ).toEqual( [ 'ribs' ] ); - expect( state.blocksByUid ).toEqual( { + expect( state.present.blockOrder ).toEqual( [ 'ribs' ] ); + expect( state.present.blocksByUid ).toEqual( { ribs: { uid: 'ribs', name: 'core/test-block', @@ -333,8 +333,8 @@ describe( 'state', () => { uids: [ 'chicken', 'veggies' ], } ); - expect( state.blockOrder ).toEqual( [ 'ribs' ] ); - expect( state.blocksByUid ).toEqual( { + expect( state.present.blockOrder ).toEqual( [ 'ribs' ] ); + expect( state.present.blocksByUid ).toEqual( { ribs: { uid: 'ribs', name: 'core/test-block', @@ -366,8 +366,8 @@ describe( 'state', () => { } ], } ); - expect( Object.keys( state.blocksByUid ) ).toHaveLength( 3 ); - expect( state.blockOrder ).toEqual( [ 'kumquat', 'persimmon', 'loquat' ] ); + expect( Object.keys( state.present.blocksByUid ) ).toHaveLength( 3 ); + expect( state.present.blockOrder ).toEqual( [ 'kumquat', 'persimmon', 'loquat' ] ); } ); describe( 'edits()', () => { @@ -387,7 +387,7 @@ describe( 'state', () => { }, } ); - expect( state.edits ).toEqual( { + expect( state.present.edits ).toEqual( { status: 'draft', title: 'post title', tags: [ 1 ], @@ -410,7 +410,7 @@ describe( 'state', () => { }, } ); - expect( state.edits ).toBe( original.edits ); + expect( state.present.edits ).toBe( original.present.edits ); } ); it( 'should save modified properties', () => { @@ -431,7 +431,7 @@ describe( 'state', () => { }, } ); - expect( state.edits ).toEqual( { + expect( state.present.edits ).toEqual( { status: 'draft', title: 'modified title', tags: [ 2 ], @@ -447,7 +447,7 @@ describe( 'state', () => { }, } ); - expect( state.edits ).toEqual( { + expect( state.present.edits ).toEqual( { status: 'draft', title: 'post title', } ); @@ -465,7 +465,7 @@ describe( 'state', () => { }, } ); - expect( state.edits ).toHaveProperty( 'content' ); + expect( state.present.edits ).toHaveProperty( 'content' ); state = editor( original, { type: 'RESET_BLOCKS', @@ -480,7 +480,7 @@ describe( 'state', () => { } ], } ); - expect( state.edits ).not.toHaveProperty( 'content' ); + expect( state.present.edits ).not.toHaveProperty( 'content' ); } ); } ); @@ -501,7 +501,7 @@ describe( 'state', () => { }, } ); - expect( state.blocksByUid.kumquat.attributes.updated ).toBe( true ); + expect( state.present.blocksByUid.kumquat.attributes.updated ).toBe( true ); } ); it( 'should accumulate attribute block updates', () => { @@ -522,7 +522,7 @@ describe( 'state', () => { }, } ); - expect( state.blocksByUid.kumquat.attributes ).toEqual( { + expect( state.present.blocksByUid.kumquat.attributes ).toEqual( { updated: true, moreUpdated: true, } ); @@ -541,7 +541,7 @@ describe( 'state', () => { }, } ); - expect( state.blocksByUid ).toBe( original.blocksByUid ); + expect( state.present.blocksByUid ).toBe( original.present.blocksByUid ); } ); it( 'should return with same reference if no changes in updates', () => { @@ -562,7 +562,7 @@ describe( 'state', () => { }, } ); - expect( state.blocksByUid ).toBe( state.blocksByUid ); + expect( state.present.blocksByUid ).toBe( state.present.blocksByUid ); } ); } ); } ); diff --git a/editor/test/selectors.js b/editor/test/selectors.js index b396a18bf8f99b..8553efcd1db36b 100644 --- a/editor/test/selectors.js +++ b/editor/test/selectors.js @@ -360,11 +360,9 @@ describe( 'selectors', () => { it( 'should return true when the past history is not empty', () => { const state = { editor: { - history: { - past: [ - {}, - ], - }, + past: [ + {}, + ], }, }; @@ -374,9 +372,7 @@ describe( 'selectors', () => { it( 'should return false when the past history is empty', () => { const state = { editor: { - history: { - past: [], - }, + past: [], }, }; @@ -388,11 +384,9 @@ describe( 'selectors', () => { it( 'should return true when the future history is not empty', () => { const state = { editor: { - history: { - future: [ - {}, - ], - }, + future: [ + {}, + ], }, }; @@ -402,9 +396,7 @@ describe( 'selectors', () => { it( 'should return false when the future history is empty', () => { const state = { editor: { - history: { - future: [], - }, + future: [], }, }; @@ -419,7 +411,9 @@ describe( 'selectors', () => { status: 'auto-draft', }, editor: { - edits: {}, + present: { + edits: {}, + }, }, }; @@ -432,7 +426,9 @@ describe( 'selectors', () => { status: 'draft', }, editor: { - edits: {}, + present: { + edits: {}, + }, }, }; @@ -638,7 +634,9 @@ describe( 'selectors', () => { it( 'should return the post edits', () => { const state = { editor: { - edits: { title: 'terga' }, + present: { + edits: { title: 'terga' }, + }, }, }; @@ -653,7 +651,9 @@ describe( 'selectors', () => { title: 'sassel', }, editor: { - edits: { status: 'private' }, + present: { + edits: { status: 'private' }, + }, }, }; @@ -666,7 +666,9 @@ describe( 'selectors', () => { title: 'sassel', }, editor: { - edits: { title: 'youcha' }, + present: { + edits: { title: 'youcha' }, + }, }, }; @@ -686,7 +688,11 @@ describe( 'selectors', () => { title: 'The Title', }, editor: { - edits: {}, + present: { + edits: {}, + blocksByUid: {}, + blockOrder: [], + }, }, metaBoxes, }; @@ -701,8 +707,10 @@ describe( 'selectors', () => { title: 'The Title', }, editor: { - edits: { - title: 'Modified Title', + present: { + edits: { + title: 'Modified Title', + }, }, }, metaBoxes, @@ -722,7 +730,11 @@ describe( 'selectors', () => { title: '', }, editor: { - edits: {}, + present: { + edits: {}, + blocksByUid: {}, + blockOrder: [], + }, }, metaBoxes, }; @@ -741,7 +753,11 @@ describe( 'selectors', () => { title: '', }, editor: { - edits: {}, + present: { + edits: {}, + blocksByUid: {}, + blockOrder: [], + }, }, metaBoxes, }; @@ -757,7 +773,9 @@ describe( 'selectors', () => { excerpt: 'sassel', }, editor: { - edits: { status: 'private' }, + present: { + edits: { status: 'private' }, + }, }, }; @@ -770,7 +788,9 @@ describe( 'selectors', () => { excerpt: 'sassel', }, editor: { - edits: { excerpt: 'youcha' }, + present: { + edits: { excerpt: 'youcha' }, + }, }, }; @@ -785,7 +805,9 @@ describe( 'selectors', () => { status: 'draft', }, editor: { - edits: {}, + present: { + edits: {}, + }, }, }; @@ -798,7 +820,9 @@ describe( 'selectors', () => { status: 'private', }, editor: { - edits: {}, + present: { + edits: {}, + }, }, }; @@ -812,7 +836,9 @@ describe( 'selectors', () => { password: 'chicken', }, editor: { - edits: {}, + present: { + edits: {}, + }, }, }; @@ -826,9 +852,11 @@ describe( 'selectors', () => { password: 'chicken', }, editor: { - edits: { - status: 'private', - password: null, + present: { + edits: { + status: 'private', + password: null, + }, }, }, }; @@ -989,9 +1017,11 @@ describe( 'selectors', () => { it( 'should return false if the post has no title, excerpt, content', () => { const state = { editor: { - blocksByUid: {}, - blockOrder: [], - edits: {}, + present: { + blocksByUid: {}, + blockOrder: [], + edits: {}, + }, }, currentPost: {}, }; @@ -1002,9 +1032,11 @@ describe( 'selectors', () => { it( 'should return true if the post has a title', () => { const state = { editor: { - blocksByUid: {}, - blockOrder: [], - edits: {}, + present: { + blocksByUid: {}, + blockOrder: [], + edits: {}, + }, }, currentPost: { title: 'sassel', @@ -1017,9 +1049,11 @@ describe( 'selectors', () => { it( 'should return true if the post has an excerpt', () => { const state = { editor: { - blocksByUid: {}, - blockOrder: [], - edits: {}, + present: { + blocksByUid: {}, + blockOrder: [], + edits: {}, + }, }, currentPost: { excerpt: 'sassel', @@ -1032,17 +1066,19 @@ describe( 'selectors', () => { it( 'should return true if the post has content', () => { const state = { editor: { - blocksByUid: { - 123: { - uid: 123, - name: 'core/test-block', - attributes: { - text: '', + present: { + blocksByUid: { + 123: { + uid: 123, + name: 'core/test-block', + attributes: { + text: '', + }, }, }, + blockOrder: [ 123 ], + edits: {}, }, - blockOrder: [ 123 ], - edits: {}, }, currentPost: {}, }; @@ -1055,7 +1091,9 @@ describe( 'selectors', () => { it( 'should return true for posts with a future date', () => { const state = { editor: { - edits: { date: moment().add( 7, 'days' ).format( '' ) }, + present: { + edits: { date: moment().add( 7, 'days' ).format( '' ) }, + }, }, }; @@ -1065,7 +1103,9 @@ describe( 'selectors', () => { it( 'should return false for posts with an old date', () => { const state = { editor: { - edits: { date: '2016-05-30T17:21:39' }, + present: { + edits: { date: '2016-05-30T17:21:39' }, + }, }, }; @@ -1098,10 +1138,12 @@ describe( 'selectors', () => { const state = { currentPost: {}, editor: { - blocksByUid: { - 123: { uid: 123, name: 'core/paragraph' }, + present: { + blocksByUid: { + 123: { uid: 123, name: 'core/paragraph' }, + }, + edits: {}, }, - edits: {}, }, }; @@ -1112,8 +1154,10 @@ describe( 'selectors', () => { const state = { currentPost: {}, editor: { - blocksByUid: {}, - edits: {}, + present: { + blocksByUid: {}, + edits: {}, + }, }, }; @@ -1140,10 +1184,12 @@ describe( 'selectors', () => { }, }, editor: { - blocksByUid: { - 123: { uid: 123, name: 'core/meta-block' }, + present: { + blocksByUid: { + 123: { uid: 123, name: 'core/meta-block' }, + }, + edits: {}, }, - edits: {}, }, }; @@ -1162,12 +1208,14 @@ describe( 'selectors', () => { const state = { currentPost: {}, editor: { - blocksByUid: { - 23: { uid: 23, name: 'core/heading' }, - 123: { uid: 123, name: 'core/paragraph' }, + present: { + blocksByUid: { + 23: { uid: 23, name: 'core/heading' }, + 123: { uid: 123, name: 'core/paragraph' }, + }, + blockOrder: [ 123, 23 ], + edits: {}, }, - blockOrder: [ 123, 23 ], - edits: {}, }, }; @@ -1182,11 +1230,13 @@ describe( 'selectors', () => { it( 'should return the number of blocks in the post', () => { const state = { editor: { - blocksByUid: { - 23: { uid: 23, name: 'core/heading' }, - 123: { uid: 123, name: 'core/paragraph' }, + present: { + blocksByUid: { + 23: { uid: 23, name: 'core/heading' }, + 123: { uid: 123, name: 'core/paragraph' }, + }, + blockOrder: [ 123, 23 ], }, - blockOrder: [ 123, 23 ], }, }; @@ -1199,11 +1249,13 @@ describe( 'selectors', () => { const state = { currentPost: {}, editor: { - blocksByUid: { - 23: { uid: 23, name: 'core/heading' }, - 123: { uid: 123, name: 'core/paragraph' }, + present: { + blocksByUid: { + 23: { uid: 23, name: 'core/heading' }, + 123: { uid: 123, name: 'core/paragraph' }, + }, + edits: {}, }, - edits: {}, }, blockSelection: { start: null, end: null }, }; @@ -1214,9 +1266,11 @@ describe( 'selectors', () => { it( 'should return null if there is multi selection', () => { const state = { editor: { - blocksByUid: { - 23: { uid: 23, name: 'core/heading' }, - 123: { uid: 123, name: 'core/paragraph' }, + present: { + blocksByUid: { + 23: { uid: 23, name: 'core/heading' }, + 123: { uid: 123, name: 'core/paragraph' }, + }, }, }, blockSelection: { start: 23, end: 123 }, @@ -1228,15 +1282,17 @@ describe( 'selectors', () => { it( 'should return the selected block', () => { const state = { editor: { - blocksByUid: { - 23: { uid: 23, name: 'core/heading' }, - 123: { uid: 123, name: 'core/paragraph' }, + present: { + blocksByUid: { + 23: { uid: 23, name: 'core/heading' }, + 123: { uid: 123, name: 'core/paragraph' }, + }, }, }, blockSelection: { start: 23, end: 23 }, }; - expect( getSelectedBlock( state ) ).toBe( state.editor.blocksByUid[ 23 ] ); + expect( getSelectedBlock( state ) ).toBe( state.editor.present.blocksByUid[ 23 ] ); } ); } ); @@ -1244,7 +1300,9 @@ describe( 'selectors', () => { it( 'should return empty if there is no multi selection', () => { const state = { editor: { - blockOrder: [ 123, 23 ], + present: { + blockOrder: [ 123, 23 ], + }, }, blockSelection: { start: null, end: null }, }; @@ -1255,7 +1313,9 @@ describe( 'selectors', () => { it( 'should return selected block uids if there is multi selection', () => { const state = { editor: { - blockOrder: [ 5, 4, 3, 2, 1 ], + present: { + blockOrder: [ 5, 4, 3, 2, 1 ], + }, }, blockSelection: { start: 2, end: 4 }, }; @@ -1268,7 +1328,9 @@ describe( 'selectors', () => { it( 'returns null if there is no multi selection', () => { const state = { editor: { - blockOrder: [ 123, 23 ], + present: { + blockOrder: [ 123, 23 ], + }, }, blockSelection: { start: null, end: null }, }; @@ -1279,7 +1341,9 @@ describe( 'selectors', () => { it( 'returns multi selection start', () => { const state = { editor: { - blockOrder: [ 5, 4, 3, 2, 1 ], + present: { + blockOrder: [ 5, 4, 3, 2, 1 ], + }, }, blockSelection: { start: 2, end: 4 }, }; @@ -1292,7 +1356,9 @@ describe( 'selectors', () => { it( 'returns null if there is no multi selection', () => { const state = { editor: { - blockOrder: [ 123, 23 ], + present: { + blockOrder: [ 123, 23 ], + }, }, blockSelection: { start: null, end: null }, }; @@ -1303,7 +1369,9 @@ describe( 'selectors', () => { it( 'returns multi selection end', () => { const state = { editor: { - blockOrder: [ 5, 4, 3, 2, 1 ], + present: { + blockOrder: [ 5, 4, 3, 2, 1 ], + }, }, blockSelection: { start: 2, end: 4 }, }; @@ -1316,7 +1384,9 @@ describe( 'selectors', () => { it( 'should return the ordered block UIDs', () => { const state = { editor: { - blockOrder: [ 123, 23 ], + present: { + blockOrder: [ 123, 23 ], + }, }, }; @@ -1328,7 +1398,9 @@ describe( 'selectors', () => { it( 'should return the block order', () => { const state = { editor: { - blockOrder: [ 123, 23 ], + present: { + blockOrder: [ 123, 23 ], + }, }, }; @@ -1340,7 +1412,9 @@ describe( 'selectors', () => { it( 'should return true when the block is first', () => { const state = { editor: { - blockOrder: [ 123, 23 ], + present: { + blockOrder: [ 123, 23 ], + }, }, }; @@ -1350,7 +1424,9 @@ describe( 'selectors', () => { it( 'should return false when the block is not first', () => { const state = { editor: { - blockOrder: [ 123, 23 ], + present: { + blockOrder: [ 123, 23 ], + }, }, }; @@ -1362,7 +1438,9 @@ describe( 'selectors', () => { it( 'should return true when the block is last', () => { const state = { editor: { - blockOrder: [ 123, 23 ], + present: { + blockOrder: [ 123, 23 ], + }, }, }; @@ -1372,7 +1450,9 @@ describe( 'selectors', () => { it( 'should return false when the block is not last', () => { const state = { editor: { - blockOrder: [ 123, 23 ], + present: { + blockOrder: [ 123, 23 ], + }, }, }; @@ -1384,11 +1464,13 @@ describe( 'selectors', () => { it( 'should return the previous block', () => { const state = { editor: { - blocksByUid: { - 23: { uid: 23, name: 'core/heading' }, - 123: { uid: 123, name: 'core/paragraph' }, + present: { + blocksByUid: { + 23: { uid: 23, name: 'core/heading' }, + 123: { uid: 123, name: 'core/paragraph' }, + }, + blockOrder: [ 123, 23 ], }, - blockOrder: [ 123, 23 ], }, }; @@ -1398,11 +1480,13 @@ describe( 'selectors', () => { it( 'should return null for the first block', () => { const state = { editor: { - blocksByUid: { - 23: { uid: 23, name: 'core/heading' }, - 123: { uid: 123, name: 'core/paragraph' }, + present: { + blocksByUid: { + 23: { uid: 23, name: 'core/heading' }, + 123: { uid: 123, name: 'core/paragraph' }, + }, + blockOrder: [ 123, 23 ], }, - blockOrder: [ 123, 23 ], }, }; @@ -1414,11 +1498,13 @@ describe( 'selectors', () => { it( 'should return the following block', () => { const state = { editor: { - blocksByUid: { - 23: { uid: 23, name: 'core/heading' }, - 123: { uid: 123, name: 'core/paragraph' }, + present: { + blocksByUid: { + 23: { uid: 23, name: 'core/heading' }, + 123: { uid: 123, name: 'core/paragraph' }, + }, + blockOrder: [ 123, 23 ], }, - blockOrder: [ 123, 23 ], }, }; @@ -1428,11 +1514,13 @@ describe( 'selectors', () => { it( 'should return null for the last block', () => { const state = { editor: { - blocksByUid: { - 23: { uid: 23, name: 'core/heading' }, - 123: { uid: 123, name: 'core/paragraph' }, + present: { + blocksByUid: { + 23: { uid: 23, name: 'core/heading' }, + 123: { uid: 123, name: 'core/paragraph' }, + }, + blockOrder: [ 123, 23 ], }, - blockOrder: [ 123, 23 ], }, }; @@ -1469,7 +1557,9 @@ describe( 'selectors', () => { describe( 'isBlockMultiSelected', () => { const state = { editor: { - blockOrder: [ 5, 4, 3, 2, 1 ], + present: { + blockOrder: [ 5, 4, 3, 2, 1 ], + }, }, blockSelection: { start: 2, end: 4 }, }; @@ -1486,7 +1576,9 @@ describe( 'selectors', () => { describe( 'isFirstMultiSelectedBlock', () => { const state = { editor: { - blockOrder: [ 5, 4, 3, 2, 1 ], + present: { + blockOrder: [ 5, 4, 3, 2, 1 ], + }, }, blockSelection: { start: 2, end: 4 }, }; @@ -1616,11 +1708,13 @@ describe( 'selectors', () => { end: 2, }, editor: { - blocksByUid: { - 2: { uid: 2 }, + present: { + blocksByUid: { + 2: { uid: 2 }, + }, + blockOrder: [ 1, 2, 3 ], + edits: {}, }, - blockOrder: [ 1, 2, 3 ], - edits: {}, }, blockInsertionPoint: {}, }; @@ -1633,7 +1727,9 @@ describe( 'selectors', () => { preferences: { mode: 'visual' }, blockSelection: {}, editor: { - blockOrder: [ 1, 2, 3 ], + present: { + blockOrder: [ 1, 2, 3 ], + }, }, blockInsertionPoint: { position: 2, @@ -1651,7 +1747,9 @@ describe( 'selectors', () => { end: 2, }, editor: { - blockOrder: [ 1, 2, 3 ], + present: { + blockOrder: [ 1, 2, 3 ], + }, }, blockInsertionPoint: {}, }; @@ -1664,7 +1762,9 @@ describe( 'selectors', () => { preferences: { mode: 'visual' }, blockSelection: { start: null, end: null }, editor: { - blockOrder: [ 1, 2, 3 ], + present: { + blockOrder: [ 1, 2, 3 ], + }, }, blockInsertionPoint: {}, }; @@ -1677,7 +1777,9 @@ describe( 'selectors', () => { preferences: { mode: 'text' }, blockSelection: { start: 2, end: 2 }, editor: { - blockOrder: [ 1, 2, 3 ], + present: { + blockOrder: [ 1, 2, 3 ], + }, }, blockInsertionPoint: {}, }; @@ -1788,8 +1890,10 @@ describe( 'selectors', () => { it( 'returns null if cannot be determined', () => { const state = { editor: { - blockOrder: [], - blocksByUid: {}, + present: { + blockOrder: [], + blocksByUid: {}, + }, }, }; @@ -1799,10 +1903,12 @@ describe( 'selectors', () => { it( 'returns null if there is more than one block in the post', () => { const state = { editor: { - blockOrder: [ 123, 456 ], - blocksByUid: { - 123: { uid: 123, name: 'core/image' }, - 456: { uid: 456, name: 'core/quote' }, + present: { + blockOrder: [ 123, 456 ], + blocksByUid: { + 123: { uid: 123, name: 'core/image' }, + 456: { uid: 456, name: 'core/quote' }, + }, }, }, }; @@ -1813,9 +1919,11 @@ describe( 'selectors', () => { it( 'returns Image if the first block is of type `core/image`', () => { const state = { editor: { - blockOrder: [ 123 ], - blocksByUid: { - 123: { uid: 123, name: 'core/image' }, + present: { + blockOrder: [ 123 ], + blocksByUid: { + 123: { uid: 123, name: 'core/image' }, + }, }, }, }; @@ -1826,9 +1934,11 @@ describe( 'selectors', () => { it( 'returns Quote if the first block is of type `core/quote`', () => { const state = { editor: { - blockOrder: [ 456 ], - blocksByUid: { - 456: { uid: 456, name: 'core/quote' }, + present: { + blockOrder: [ 456 ], + blocksByUid: { + 456: { uid: 456, name: 'core/quote' }, + }, }, }, }; @@ -1839,9 +1949,11 @@ describe( 'selectors', () => { it( 'returns Video if the first block is of type `core-embed/youtube`', () => { const state = { editor: { - blockOrder: [ 567 ], - blocksByUid: { - 567: { uid: 567, name: 'core-embed/youtube' }, + present: { + blockOrder: [ 567 ], + blocksByUid: { + 567: { uid: 567, name: 'core-embed/youtube' }, + }, }, }, }; @@ -1852,10 +1964,12 @@ describe( 'selectors', () => { it( 'returns Quote if the first block is of type `core/quote` and second is of type `core/paragraph`', () => { const state = { editor: { - blockOrder: [ 456, 789 ], - blocksByUid: { - 456: { uid: 456, name: 'core/quote' }, - 789: { uid: 789, name: 'core/paragraph' }, + present: { + blockOrder: [ 456, 789 ], + blocksByUid: { + 456: { uid: 456, name: 'core/quote' }, + 789: { uid: 789, name: 'core/paragraph' }, + }, }, }, }; diff --git a/editor/utils/test/undoable-reducer.js b/editor/utils/test/undoable-reducer.js index 9423cd8fe4f3ba..7c21ca39649bb4 100644 --- a/editor/utils/test/undoable-reducer.js +++ b/editor/utils/test/undoable-reducer.js @@ -1,142 +1,121 @@ /** * Internal dependencies */ -import { undoable, combineUndoableReducers } from '../undoable-reducer'; +import undoableReducer from '../undoable-reducer'; describe( 'undoableReducer', () => { - describe( 'undoable()', () => { - const counter = ( state = 0, { type } ) => ( - type === 'INCREMENT' ? state + 1 : state - ); - - it( 'should return a new reducer', () => { - const reducer = undoable( counter ); - - expect( typeof reducer ).toBe( 'function' ); - expect( reducer( undefined, {} ) ).toEqual( { - past: [], - present: 0, - future: [], - } ); + const counter = ( state = 0, { type } ) => ( + type === 'INCREMENT' ? state + 1 : state + ); + + it( 'should return a new reducer', () => { + const reducer = undoableReducer( counter ); + + expect( typeof reducer ).toBe( 'function' ); + expect( reducer( undefined, {} ) ).toEqual( { + past: [], + present: 0, + future: [], } ); + } ); - it( 'should track history', () => { - const reducer = undoable( counter ); + it( 'should track history', () => { + const reducer = undoableReducer( counter ); - let state; - state = reducer( undefined, {} ); - state = reducer( state, { type: 'INCREMENT' } ); + let state; + state = reducer( undefined, {} ); + state = reducer( state, { type: 'INCREMENT' } ); - expect( state ).toEqual( { - past: [ 0 ], - present: 1, - future: [], - } ); + expect( state ).toEqual( { + past: [ 0 ], + present: 1, + future: [], } ); + } ); - it( 'should perform undo', () => { - const reducer = undoable( counter ); + it( 'should perform undo', () => { + const reducer = undoableReducer( counter ); - let state; - state = reducer( undefined, {} ); - state = reducer( state, { type: 'INCREMENT' } ); - state = reducer( state, { type: 'UNDO' } ); + let state; + state = reducer( undefined, {} ); + state = reducer( state, { type: 'INCREMENT' } ); + state = reducer( state, { type: 'UNDO' } ); - expect( state ).toEqual( { - past: [], - present: 0, - future: [ 1 ], - } ); + expect( state ).toEqual( { + past: [], + present: 0, + future: [ 1 ], } ); + } ); - it( 'should not perform undo on empty past', () => { - const reducer = undoable( counter ); + it( 'should not perform undo on empty past', () => { + const reducer = undoableReducer( counter ); - let state; - state = reducer( undefined, {} ); - state = reducer( state, { type: 'INCREMENT' } ); - state = reducer( state, { type: 'UNDO' } ); + let state; + state = reducer( undefined, {} ); + state = reducer( state, { type: 'INCREMENT' } ); + state = reducer( state, { type: 'UNDO' } ); - expect( state ).toEqual( { - past: [], - present: 0, - future: [ 1 ], - } ); + expect( state ).toEqual( { + past: [], + present: 0, + future: [ 1 ], } ); + } ); - it( 'should perform redo', () => { - const reducer = undoable( counter ); + it( 'should perform redo', () => { + const reducer = undoableReducer( counter ); - let state; - state = reducer( undefined, {} ); - state = reducer( state, { type: 'INCREMENT' } ); - state = reducer( state, { type: 'UNDO' } ); - state = reducer( state, { type: 'UNDO' } ); + let state; + state = reducer( undefined, {} ); + state = reducer( state, { type: 'INCREMENT' } ); + state = reducer( state, { type: 'UNDO' } ); + state = reducer( state, { type: 'UNDO' } ); - expect( state ).toEqual( { - past: [], - present: 0, - future: [ 1 ], - } ); + expect( state ).toEqual( { + past: [], + present: 0, + future: [ 1 ], } ); + } ); - it( 'should not perform redo on empty future', () => { - const reducer = undoable( counter ); + it( 'should not perform redo on empty future', () => { + const reducer = undoableReducer( counter ); - let state; - state = reducer( undefined, {} ); - state = reducer( state, { type: 'INCREMENT' } ); - state = reducer( state, { type: 'REDO' } ); + let state; + state = reducer( undefined, {} ); + state = reducer( state, { type: 'INCREMENT' } ); + state = reducer( state, { type: 'REDO' } ); - expect( state ).toEqual( { - past: [ 0 ], - present: 1, - future: [], - } ); - } ); - - it( 'should reset history by options.resetTypes', () => { - const reducer = undoable( counter, { resetTypes: [ 'RESET_HISTORY' ] } ); - - let state; - state = reducer( undefined, {} ); - state = reducer( state, { type: 'INCREMENT' } ); - state = reducer( state, { type: 'RESET_HISTORY' } ); - state = reducer( state, { type: 'INCREMENT' } ); - state = reducer( state, { type: 'INCREMENT' } ); - - expect( state ).toEqual( { - past: [ 1, 2 ], - present: 3, - future: [], - } ); + expect( state ).toEqual( { + past: [ 0 ], + present: 1, + future: [], } ); } ); - describe( 'combineUndoableReducers()', () => { - const reducer = combineUndoableReducers( { - count: ( state = 0 ) => state, - } ); + it( 'should reset history by options.resetTypes', () => { + const reducer = undoableReducer( counter, { resetTypes: [ 'RESET_HISTORY' ] } ); + + let state; + state = reducer( undefined, {} ); + state = reducer( state, { type: 'INCREMENT' } ); + state = reducer( state, { type: 'RESET_HISTORY' } ); + state = reducer( state, { type: 'INCREMENT' } ); + state = reducer( state, { type: 'INCREMENT' } ); - it( 'should return a combined reducer with getters', () => { - const state = reducer( undefined, {} ); - - expect( typeof reducer ).toBe( 'function' ); - expect( state.count ).toBe( 0 ); - expect( state.history ).toEqual( { - past: [], - present: { - count: 0, - }, - future: [], - } ); + expect( state ).toEqual( { + past: [ 1, 2 ], + present: 3, + future: [], } ); + } ); - it( 'should return same reference if state has not changed', () => { - const original = reducer( undefined, {} ); - const state = reducer( original, {} ); + it( 'should return same reference if state has not changed', () => { + const reducer = undoableReducer( counter ); + const original = reducer( undefined, {} ); + const state = reducer( original, {} ); - expect( state ).toBe( original ); - } ); + expect( state ).toBe( original ); } ); } ); diff --git a/editor/utils/undoable-reducer.js b/editor/utils/undoable-reducer.js index a7d2e97681df9b..e4c9e0368503a5 100644 --- a/editor/utils/undoable-reducer.js +++ b/editor/utils/undoable-reducer.js @@ -1,7 +1,6 @@ /** * External dependencies */ -import { combineReducers } from 'redux'; import { includes } from 'lodash'; /** @@ -13,7 +12,7 @@ import { includes } from 'lodash'; * @param {?Array} options.resetTypes Action types upon which to clear past * @return {Function} Enhanced reducer */ -export function undoable( reducer, options = {} ) { +export default function undoableReducer( reducer, options = {} ) { const initialState = { past: [], present: reducer( undefined, {} ), @@ -70,46 +69,3 @@ export function undoable( reducer, options = {} ) { }; }; } - -/** - * A wrapper for combineReducers which applies an undo history to the combined - * reducer. As a convenience, properties of the reducers object are accessible - * via object getters, with history assigned to a nested history property. - * - * @see undoable - * - * @param {Object} reducers Object of reducers - * @param {?Object} options Optional options - * @return {Function} Combined reducer - */ -export function combineUndoableReducers( reducers, options ) { - const reducer = undoable( combineReducers( reducers ), options ); - - function withGetters( history ) { - const state = { history }; - const keys = Object.getOwnPropertyNames( history.present ); - const getters = keys.reduce( ( memo, key ) => { - memo[ key ] = { - get: function() { - return this.history.present[ key ]; - }, - }; - - return memo; - }, {} ); - Object.defineProperties( state, getters ); - - return state; - } - - const initialState = withGetters( reducer( undefined, {} ) ); - - return ( state = initialState, action ) => { - const nextState = reducer( state.history, action ); - if ( nextState === state.history ) { - return state; - } - - return withGetters( nextState ); - }; -} From 2c0ada2801a8f232467207645f335115ed54b42a Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 3 Nov 2017 10:31:24 -0400 Subject: [PATCH 4/5] State: Convert editor dirtying behaviors to higher-order reducer --- editor/reducer.js | 7 ++- editor/selectors.js | 10 +-- editor/state/save-state.js | 72 ---------------------- editor/state/test/save-state.js | 105 -------------------------------- editor/store.js | 3 +- editor/test/reducer.js | 5 +- editor/test/selectors.js | 41 +++++-------- 7 files changed, 31 insertions(+), 212 deletions(-) delete mode 100644 editor/state/save-state.js delete mode 100644 editor/state/test/save-state.js diff --git a/editor/reducer.js b/editor/reducer.js index d5f5680dd6a72a..4725de88b2376f 100644 --- a/editor/reducer.js +++ b/editor/reducer.js @@ -29,8 +29,8 @@ import { getBlockTypes, getBlockType } from '@wordpress/blocks'; * Internal dependencies */ import undoableReducer from './utils/undoable-reducer'; +import dirtyingReducer from './utils/dirtying-reducer'; import { STORE_DEFAULTS } from './store-defaults'; -import saveState from './state/save-state'; /*** * Module constants @@ -71,6 +71,10 @@ export const editor = flow( [ // Track undo history, starting at editor initialization. partialRight( undoableReducer, { resetTypes: [ 'SETUP_EDITOR' ] } ), + + // Track whether changes exist, starting at editor initialization and + // resetting at each post save. + partialRight( dirtyingReducer, { resetTypes: [ 'SETUP_EDITOR', 'RESET_POST' ] } ), ] )( { edits( state = {}, action ) { switch ( action.type ) { @@ -666,5 +670,4 @@ export default optimist( combineReducers( { saving, notices, metaBoxes, - saveState, } ) ); diff --git a/editor/selectors.js b/editor/selectors.js index f358eebfa4da8a..02288e7e68dc29 100644 --- a/editor/selectors.js +++ b/editor/selectors.js @@ -180,7 +180,7 @@ export function isEditedPostNew( state ) { * @return {Boolean} Whether unsaved values exist */ export function isEditedPostDirty( state ) { - return state.saveState.isDirty || isMetaBoxStateDirty( state ); + return state.editor.isDirty || isMetaBoxStateDirty( state ); } /** @@ -453,15 +453,15 @@ export const getBlock = createSelector( }; }, ( state, uid ) => [ - get( state, [ 'editor', 'blocksByUid', uid ] ), - get( state, 'editor.edits.meta' ), + get( state, [ 'editor', 'present', 'blocksByUid', uid ] ), + get( state, [ 'editor', 'present', 'edits', 'meta' ] ), get( state, 'currentPost.meta' ), ] ); function getPostMeta( state, key ) { - return has( state, [ 'editor', 'edits', 'meta', key ] ) ? - get( state, [ 'editor', 'edits', 'meta', key ] ) : + return has( state, [ 'editor', 'edits', 'present', 'meta', key ] ) ? + get( state, [ 'editor', 'edits', 'present', 'meta', key ] ) : get( state, [ 'currentPost', 'meta', key ] ); } diff --git a/editor/state/save-state.js b/editor/state/save-state.js deleted file mode 100644 index 7f349224b61012..00000000000000 --- a/editor/state/save-state.js +++ /dev/null @@ -1,72 +0,0 @@ -/** - * External dependencies - */ -import { combineReducers } from 'redux'; - -/** - * Internal dependencies - */ -import { getCurrentPost, hasEditorUndo } from '../selectors'; - -/** - * Action creator returning an action object used in signalling that the post's - * dirty saved state should be toggled. - * - * @param {Boolean} isDirty Whether post is dirty - * @return {Object} Action object - */ -export function toggleDirty( isDirty ) { - return { - type: 'TOGGLE_DIRTY', - isDirty, - }; -} - -/** - * Redux middleware for save state dirtying tracking. - * - * @param {Function} store Redux store object - * @return {Function} Middleware function - */ -export const middleware = ( store ) => ( next ) => ( action ) => { - const state = store.getState(); - const result = next( action ); - const nextState = store.getState(); - - let isDirty; - if ( getCurrentPost( nextState ) !== getCurrentPost( state ) ) { - // Current post reflects last known post save, so if these references - // differ, we can assume that the post has been saved - isDirty = false; - } else if ( state.editor !== nextState.editor && hasEditorUndo( nextState ) ) { - // Editor state tracks post attribute edits, block order, block - // attributes. If any of these change, assume the post is dirtied. - // Without undo indicates an editor history reset (initial load). - isDirty = true; - } - - // Avoid dispatch except when we know that the state is changing - if ( undefined !== isDirty && isDirty !== nextState.saveState.isDirty ) { - store.dispatch( toggleDirty( isDirty ) ); - } - - return result; -}; - -export default combineReducers( { - /** - * Reducer reflecting whether the post is considered dirty (needing save). - * - * @param {Boolean} state Original state - * @param {Object} action Action object - * @return {Boolean} Next state - */ - isDirty( state = false, action ) { - switch ( action.type ) { - case 'TOGGLE_DIRTY': - return action.isDirty; - } - - return state; - }, -} ); diff --git a/editor/state/test/save-state.js b/editor/state/test/save-state.js deleted file mode 100644 index d814a97c1a432a..00000000000000 --- a/editor/state/test/save-state.js +++ /dev/null @@ -1,105 +0,0 @@ -/** - * External dependencies - */ -import { noop } from 'lodash'; -import deepFreeze from 'deep-freeze'; - -/** - * Internal dependencies - */ -import reducer, { toggleDirty, middleware } from '../save-state'; -import * as selectors from '../../selectors'; - -jest.mock( '../../selectors' ); - -describe( 'saveState', () => { - beforeEach( () => jest.resetAllMocks() ); - - describe( '.middleware()', () => { - const dispatch = jest.fn(); - - let isDirty, editor; - const store = { - getState() { - return { - saveState: { - isDirty, - }, - editor, - }; - }, - dispatch, - }; - - it( 'should dispatch toggle dirty false when post reset', () => { - isDirty = true; - // Return new object reference for each call, strictly unequal - selectors.getCurrentPost.mockImplementation( () => ( {} ) ); - - middleware( store )( noop )( {} ); - - expect( store.dispatch ).toHaveBeenCalledWith( toggleDirty( false ) ); - } ); - - it( 'should dispatch toggle dirty true when editor changes', () => { - isDirty = false; - const currentPost = {}; - selectors.getCurrentPost.mockReturnValue( currentPost ); - selectors.hasEditorUndo.mockReturnValue( true ); - - editor = {}; - middleware( store )( () => { - // Set editor as new reference, strictly unequal to old - editor = {}; - } )( {} ); - - expect( store.dispatch ).toHaveBeenCalledWith( toggleDirty( true ) ); - } ); - - it( 'should not dispatch toggle dirty when post has no undo history', () => { - isDirty = false; - const currentPost = {}; - selectors.getCurrentPost.mockReturnValue( currentPost ); - selectors.hasEditorUndo.mockReturnValue( false ); - - middleware( store )( noop )( {} ); - - expect( store.dispatch ).not.toHaveBeenCalled(); - } ); - - it( 'should not dispatch toggle if already the same', () => { - isDirty = true; - const currentPost = {}; - selectors.getCurrentPost.mockReturnValue( currentPost ); - selectors.hasEditorUndo.mockReturnValue( true ); - - editor = {}; - middleware( store )( () => { - // Set editor as new reference, strictly unequal to old - editor = {}; - } )( {} ); - - expect( store.dispatch ).not.toHaveBeenCalled(); - } ); - } ); - - describe( 'reducer', () => { - describe( '.isDirty()', () => { - it( 'should default to false', () => { - const state = reducer( undefined, {} ); - - expect( state.isDirty ).toBe( false ); - } ); - - it( 'should return value by toggle property', () => { - const originalState = deepFreeze( reducer( undefined, {} ) ); - const state = reducer( originalState, { - type: 'TOGGLE_DIRTY', - isDirty: true, - } ); - - expect( state.isDirty ).toBe( true ); - } ); - } ); - } ); -} ); diff --git a/editor/store.js b/editor/store.js index d56c40bb9c9033..fa2e7db7dfc7c4 100644 --- a/editor/store.js +++ b/editor/store.js @@ -12,7 +12,6 @@ import { flowRight } from 'lodash'; import effects from './effects'; import reducer from './reducer'; import storePersist from './store-persist'; -import { middleware as saveStateMiddleware } from './state/save-state'; /** * Module constants @@ -26,7 +25,7 @@ const GUTENBERG_PREFERENCES_KEY = `GUTENBERG_PREFERENCES_${ window.userSettings. */ function createReduxStore() { const enhancers = [ - applyMiddleware( multi, refx( effects ), saveStateMiddleware ), + applyMiddleware( multi, refx( effects ) ), storePersist( 'preferences', GUTENBERG_PREFERENCES_KEY ), ]; diff --git a/editor/test/reducer.js b/editor/test/reducer.js index 4315f87c258764..c82de4fe25ce28 100644 --- a/editor/test/reducer.js +++ b/editor/test/reducer.js @@ -56,12 +56,15 @@ describe( 'state', () => { unregisterBlockType( 'core/test-block' ); } ); - it( 'should return empty edits, blocksByUid, blockOrder by default', () => { + it( 'should return history (empty edits, blocksByUid, blockOrder), dirty flag by default', () => { const state = editor( undefined, {} ); + expect( state.past ).toEqual( [] ); + expect( state.future ).toEqual( [] ); expect( state.present.edits ).toEqual( {} ); expect( state.present.blocksByUid ).toEqual( {} ); expect( state.present.blockOrder ).toEqual( [] ); + expect( state.isDirty ).toBe( false ); } ); it( 'should key by replaced blocks uid', () => { diff --git a/editor/test/selectors.js b/editor/test/selectors.js index 8553efcd1db36b..abd0839d0b706b 100644 --- a/editor/test/selectors.js +++ b/editor/test/selectors.js @@ -465,7 +465,7 @@ describe( 'selectors', () => { it( 'should return true when post saved state dirty', () => { const state = { - saveState: { + editor: { isDirty: true, }, metaBoxes, @@ -476,7 +476,7 @@ describe( 'selectors', () => { it( 'should return false when post saved state not dirty', () => { const state = { - saveState: { + editor: { isDirty: false, }, metaBoxes, @@ -487,7 +487,7 @@ describe( 'selectors', () => { it( 'should return true when post saved state not dirty, but meta box state has changed.', () => { const state = { - saveState: { + editor: { isDirty: false, }, metaBoxes: dirtyMetaBoxes, @@ -502,7 +502,7 @@ describe( 'selectors', () => { it( 'should return true when the post is not dirty and has not been saved before', () => { const state = { - saveState: { + editor: { isDirty: false, }, currentPost: { @@ -517,7 +517,7 @@ describe( 'selectors', () => { it( 'should return false when the post is not dirty but the post has been saved', () => { const state = { - saveState: { + editor: { isDirty: false, }, currentPost: { @@ -532,7 +532,7 @@ describe( 'selectors', () => { it( 'should return false when the post is dirty but the post has not been saved', () => { const state = { - saveState: { + editor: { isDirty: true, }, currentPost: { @@ -680,9 +680,6 @@ describe( 'selectors', () => { const metaBoxes = { isDirty: false, isUpdating: false }; it( 'should return current title unedited existing post', () => { const state = { - saveState: { - isDirty: false, - }, currentPost: { id: 123, title: 'The Title', @@ -693,6 +690,7 @@ describe( 'selectors', () => { blocksByUid: {}, blockOrder: [], }, + isDirty: false, }, metaBoxes, }; @@ -721,9 +719,6 @@ describe( 'selectors', () => { it( 'should return new post title when new post is clean', () => { const state = { - saveState: { - isDirty: false, - }, currentPost: { id: 1, status: 'auto-draft', @@ -735,6 +730,7 @@ describe( 'selectors', () => { blocksByUid: {}, blockOrder: [], }, + isDirty: false, }, metaBoxes, }; @@ -744,9 +740,6 @@ describe( 'selectors', () => { it( 'should return untitled title', () => { const state = { - saveState: { - isDirty: true, - }, currentPost: { id: 123, status: 'draft', @@ -758,6 +751,7 @@ describe( 'selectors', () => { blocksByUid: {}, blockOrder: [], }, + isDirty: true, }, metaBoxes, }; @@ -913,7 +907,7 @@ describe( 'selectors', () => { it( 'should return true for pending posts', () => { const state = { - saveState: { + editor: { isDirty: false, }, currentPost: { @@ -927,7 +921,7 @@ describe( 'selectors', () => { it( 'should return true for draft posts', () => { const state = { - saveState: { + editor: { isDirty: false, }, currentPost: { @@ -941,7 +935,7 @@ describe( 'selectors', () => { it( 'should return false for published posts', () => { const state = { - saveState: { + editor: { isDirty: false, }, currentPost: { @@ -955,7 +949,7 @@ describe( 'selectors', () => { it( 'should return true for published, dirty posts', () => { const state = { - saveState: { + editor: { isDirty: true, }, currentPost: { @@ -969,7 +963,7 @@ describe( 'selectors', () => { it( 'should return false for private posts', () => { const state = { - saveState: { + editor: { isDirty: false, }, currentPost: { @@ -983,7 +977,7 @@ describe( 'selectors', () => { it( 'should return false for scheduled posts', () => { const state = { - saveState: { + editor: { isDirty: false, }, currentPost: { @@ -997,14 +991,11 @@ describe( 'selectors', () => { it( 'should return true for dirty posts with usable title', () => { const state = { - saveState: { - isDirty: true, - }, currentPost: { status: 'private', }, editor: { - edits: { title: 'Dirty' }, + isDirty: true, }, metaBoxes, }; From 0304b605b239c00e13735680c0873104075b9893 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 6 Nov 2017 10:54:05 -0500 Subject: [PATCH 5/5] State: Rename higher-order reducers using "with" prefix Better reflecting the fact that undoableReducer is not itself a reducer, but a function generating an enhanced reducer. --- editor/reducer.js | 8 ++-- .../README.md | 12 +++--- .../index.js | 2 +- .../test/index.js | 14 +++---- editor/utils/with-history/README.md | 42 +++++++++++++++++++ .../index.js} | 2 +- .../test/index.js} | 20 ++++----- 7 files changed, 71 insertions(+), 29 deletions(-) rename editor/utils/{dirtying-reducer => with-change-detection}/README.md (53%) rename editor/utils/{dirtying-reducer => with-change-detection}/index.js (94%) rename editor/utils/{dirtying-reducer => with-change-detection}/test/index.js (83%) create mode 100644 editor/utils/with-history/README.md rename editor/utils/{undoable-reducer.js => with-history/index.js} (95%) rename editor/utils/{test/undoable-reducer.js => with-history/test/index.js} (82%) diff --git a/editor/reducer.js b/editor/reducer.js index 4725de88b2376f..fa6e6c9d858ef0 100644 --- a/editor/reducer.js +++ b/editor/reducer.js @@ -28,8 +28,8 @@ import { getBlockTypes, getBlockType } from '@wordpress/blocks'; /** * Internal dependencies */ -import undoableReducer from './utils/undoable-reducer'; -import dirtyingReducer from './utils/dirtying-reducer'; +import withHistory from './utils/with-history'; +import withChangeDetection from './utils/with-change-detection'; import { STORE_DEFAULTS } from './store-defaults'; /*** @@ -70,11 +70,11 @@ export const editor = flow( [ combineReducers, // Track undo history, starting at editor initialization. - partialRight( undoableReducer, { resetTypes: [ 'SETUP_EDITOR' ] } ), + partialRight( withHistory, { resetTypes: [ 'SETUP_EDITOR' ] } ), // Track whether changes exist, starting at editor initialization and // resetting at each post save. - partialRight( dirtyingReducer, { resetTypes: [ 'SETUP_EDITOR', 'RESET_POST' ] } ), + partialRight( withChangeDetection, { resetTypes: [ 'SETUP_EDITOR', 'RESET_POST' ] } ), ] )( { edits( state = {}, action ) { switch ( action.type ) { diff --git a/editor/utils/dirtying-reducer/README.md b/editor/utils/with-change-detection/README.md similarity index 53% rename from editor/utils/dirtying-reducer/README.md rename to editor/utils/with-change-detection/README.md index 1e547b50bb78e9..75f6061483f832 100644 --- a/editor/utils/dirtying-reducer/README.md +++ b/editor/utils/with-change-detection/README.md @@ -1,20 +1,20 @@ -Dirtying Reducer -================ +withChangeDetection +=================== -`dirtyingReducer` is a [Redux higher-order reducer](http://redux.js.org/docs/recipes/reducers/ReusingReducerLogic.html#customizing-behavior-with-higher-order-reducers) for tracking changes to reducer state over time. +`withChangeDetection` is a [Redux higher-order reducer](http://redux.js.org/docs/recipes/reducers/ReusingReducerLogic.html#customizing-behavior-with-higher-order-reducers) for tracking changes to reducer state over time. It does this based on the following assumptions: - The original reducer returns an object - The original reducer returns a new reference only if a change has in-fact occurred -Using these assumptions, the enhanced reducer returned from `dirtyingReducer` will include a new property on the object `isDirty` corresponding to whether the original reference of the reducer has ever changed. +Using these assumptions, the enhanced reducer returned from `withChangeDetection` will include a new property on the object `isDirty` corresponding to whether the original reference of the reducer has ever changed. Leveraging a `resetTypes` option, this can be used to mark intervals at which a state is considered to be clean (without changes) and dirty (with changes). ## Example -Considering a simple count reducer, we can enhance it with `dirtyingReducer` to reflect whether changes have occurred: +Considering a simple count reducer, we can enhance it with `withChangeDetection` to reflect whether changes have occurred: ```js function counter( state = { count: 0 }, action ) { @@ -26,7 +26,7 @@ function counter( state = { count: 0 }, action ) { return state; } -const enhancedCounter = dirtyingReducer( counter, { resetTypes: [ 'RESET' ] } ); +const enhancedCounter = withChangeDetection( counter, { resetTypes: [ 'RESET' ] } ); let state; diff --git a/editor/utils/dirtying-reducer/index.js b/editor/utils/with-change-detection/index.js similarity index 94% rename from editor/utils/dirtying-reducer/index.js rename to editor/utils/with-change-detection/index.js index dcb29b33147983..1a1fd842fe2f55 100644 --- a/editor/utils/dirtying-reducer/index.js +++ b/editor/utils/with-change-detection/index.js @@ -13,7 +13,7 @@ import { includes } from 'lodash'; * @param {?Array} options.resetTypes Action types upon which to reset dirty * @return {Function} Enhanced reducer */ -export default function dirtyingReducer( reducer, options = {} ) { +export default function withChangeDetection( reducer, options = {} ) { let originalState; return ( state, action ) => { diff --git a/editor/utils/dirtying-reducer/test/index.js b/editor/utils/with-change-detection/test/index.js similarity index 83% rename from editor/utils/dirtying-reducer/test/index.js rename to editor/utils/with-change-detection/test/index.js index f01e04dae0d8ca..d2fede2528978a 100644 --- a/editor/utils/dirtying-reducer/test/index.js +++ b/editor/utils/with-change-detection/test/index.js @@ -6,9 +6,9 @@ import deepFreeze from 'deep-freeze'; /** * Internal dependencies */ -import dirtyingReducer from '../'; +import withChangeDetection from '../'; -describe( 'dirtyingReducer()', () => { +describe( 'withChangeDetection()', () => { const initialState = { count: 0 }; function originalReducer( state = initialState, action ) { @@ -27,7 +27,7 @@ describe( 'dirtyingReducer()', () => { } it( 'should respect original reducer behavior', () => { - const reducer = dirtyingReducer( originalReducer ); + const reducer = withChangeDetection( originalReducer ); const state = reducer( undefined, {} ); expect( state ).toEqual( { count: 0, isDirty: false } ); @@ -38,7 +38,7 @@ describe( 'dirtyingReducer()', () => { } ); it( 'should allow reset types as option', () => { - const reducer = dirtyingReducer( originalReducer, { resetTypes: [ 'RESET' ] } ); + const reducer = withChangeDetection( originalReducer, { resetTypes: [ 'RESET' ] } ); let state; @@ -53,7 +53,7 @@ describe( 'dirtyingReducer()', () => { } ); it( 'should preserve isDirty into non-resetting non-reference-changing types', () => { - const reducer = dirtyingReducer( originalReducer, { resetTypes: [ 'RESET' ] } ); + const reducer = withChangeDetection( originalReducer, { resetTypes: [ 'RESET' ] } ); let state; @@ -68,7 +68,7 @@ describe( 'dirtyingReducer()', () => { } ); it( 'should reset if state reverts to its original reference', () => { - const reducer = dirtyingReducer( originalReducer, { resetTypes: [ 'RESET' ] } ); + const reducer = withChangeDetection( originalReducer, { resetTypes: [ 'RESET' ] } ); let state; @@ -83,7 +83,7 @@ describe( 'dirtyingReducer()', () => { } ); it( 'should flag as not dirty even if reset type causes reference change', () => { - const reducer = dirtyingReducer( originalReducer, { resetTypes: [ 'RESET_AND_CHANGE_REFERENCE' ] } ); + const reducer = withChangeDetection( originalReducer, { resetTypes: [ 'RESET_AND_CHANGE_REFERENCE' ] } ); let state; diff --git a/editor/utils/with-history/README.md b/editor/utils/with-history/README.md new file mode 100644 index 00000000000000..52b515d1159cdf --- /dev/null +++ b/editor/utils/with-history/README.md @@ -0,0 +1,42 @@ +withHistory +=========== + +`withHistory` is a [Redux higher-order reducer](http://redux.js.org/docs/recipes/reducers/ReusingReducerLogic.html#customizing-behavior-with-higher-order-reducers) for tracking the history of a reducer state over time. The enhanced reducer returned from `withHistory` will return an object shape with properties `past`, `present`, and `future`. The `present` value maintains the current value of state returned from the original reducer. Past and future are respectively maintained as arrays of state values occuring previously and future (if history undone). + +Leveraging a `resetTypes` option, this can be used to mark intervals at which a state history should be reset, emptying the values of the `past` and `future` arrays. + +History can be adjusted by dispatching actions with type `UNDO` (reset to the previous state) and `REDO` (reset to the next state). + +## Example + +Considering a simple count reducer, we can enhance it with `withHistory` to track value over time: + +```js +function counter( state = { count: 0 }, action ) { + switch ( action.type ) { + case 'INCREMENT': + return { ...state, count: state.count + 1 }; + } + + return state; +} + +const enhancedCounter = withHistory( counter, { resetTypes: [ 'RESET' ] } ); + +let state; + +state = enhancedCounter( undefined, {} ); +// { past: [], present: 0, future: [] } + +state = enhancedCounter( state, { type: 'INCREMENT' } ); +// { past: [ 0 ], present: 1, future: [] } + +state = enhancedCounter( state, { type: 'UNDO' } ); +// { past: [], present: 0, future: [ 1 ] } + +state = enhancedCounter( state, { type: 'REDO' } ); +// { past: [ 0 ], present: 1, future: [] } + +state = enhancedCounter( state, { type: 'RESET' } ); +// { past: [], present: 1, future: [] } +``` diff --git a/editor/utils/undoable-reducer.js b/editor/utils/with-history/index.js similarity index 95% rename from editor/utils/undoable-reducer.js rename to editor/utils/with-history/index.js index e4c9e0368503a5..7fa250d1319e79 100644 --- a/editor/utils/undoable-reducer.js +++ b/editor/utils/with-history/index.js @@ -12,7 +12,7 @@ import { includes } from 'lodash'; * @param {?Array} options.resetTypes Action types upon which to clear past * @return {Function} Enhanced reducer */ -export default function undoableReducer( reducer, options = {} ) { +export default function withHistory( reducer, options = {} ) { const initialState = { past: [], present: reducer( undefined, {} ), diff --git a/editor/utils/test/undoable-reducer.js b/editor/utils/with-history/test/index.js similarity index 82% rename from editor/utils/test/undoable-reducer.js rename to editor/utils/with-history/test/index.js index 7c21ca39649bb4..d59dcfc39847f2 100644 --- a/editor/utils/test/undoable-reducer.js +++ b/editor/utils/with-history/test/index.js @@ -1,15 +1,15 @@ /** * Internal dependencies */ -import undoableReducer from '../undoable-reducer'; +import withHistory from '../'; -describe( 'undoableReducer', () => { +describe( 'withHistory', () => { const counter = ( state = 0, { type } ) => ( type === 'INCREMENT' ? state + 1 : state ); it( 'should return a new reducer', () => { - const reducer = undoableReducer( counter ); + const reducer = withHistory( counter ); expect( typeof reducer ).toBe( 'function' ); expect( reducer( undefined, {} ) ).toEqual( { @@ -20,7 +20,7 @@ describe( 'undoableReducer', () => { } ); it( 'should track history', () => { - const reducer = undoableReducer( counter ); + const reducer = withHistory( counter ); let state; state = reducer( undefined, {} ); @@ -34,7 +34,7 @@ describe( 'undoableReducer', () => { } ); it( 'should perform undo', () => { - const reducer = undoableReducer( counter ); + const reducer = withHistory( counter ); let state; state = reducer( undefined, {} ); @@ -49,7 +49,7 @@ describe( 'undoableReducer', () => { } ); it( 'should not perform undo on empty past', () => { - const reducer = undoableReducer( counter ); + const reducer = withHistory( counter ); let state; state = reducer( undefined, {} ); @@ -64,7 +64,7 @@ describe( 'undoableReducer', () => { } ); it( 'should perform redo', () => { - const reducer = undoableReducer( counter ); + const reducer = withHistory( counter ); let state; state = reducer( undefined, {} ); @@ -80,7 +80,7 @@ describe( 'undoableReducer', () => { } ); it( 'should not perform redo on empty future', () => { - const reducer = undoableReducer( counter ); + const reducer = withHistory( counter ); let state; state = reducer( undefined, {} ); @@ -95,7 +95,7 @@ describe( 'undoableReducer', () => { } ); it( 'should reset history by options.resetTypes', () => { - const reducer = undoableReducer( counter, { resetTypes: [ 'RESET_HISTORY' ] } ); + const reducer = withHistory( counter, { resetTypes: [ 'RESET_HISTORY' ] } ); let state; state = reducer( undefined, {} ); @@ -112,7 +112,7 @@ describe( 'undoableReducer', () => { } ); it( 'should return same reference if state has not changed', () => { - const reducer = undoableReducer( counter ); + const reducer = withHistory( counter ); const original = reducer( undefined, {} ); const state = reducer( original, {} );