how to implement multiple undoable states

[aeneasr]

I have multiple documents on one page in my redux store:

store = {
  docs: [
    { doc1 }, { doc2 }, { docN }
  ]
}

I want to undo each document independently of one another. If user focus is on doc1 and he presses ctrl+z, only doc1 should be undone, not all changes of all docs.

How can I achieve this with redux undo?

[aeneasr]

It looks like this could be possible by dispatching unique undo/redo actions for each document and use { undoType: UNDO_COUNTER, redoType: REDO_COUNTER }

[aeneasr]

confirmed working

[benjasHu]

Hi @arekkas @omnidan !

Could you please post an example of this? How do you manage the action and reducer of each object independently?

I have not found any examples of this anywhere and I can't figure out how to do it.

Thanks!

[aeneasr]

Sorry, I don't have one at hand

[omnidan]

@benjasHu You can have multiple instances of redux-undo by making separate subtrees undoable, then you can also define custom actions for each of the subtrees.

If you still cannot get it to work, please provide us an example repo so we can reproduce your issue :wink:

[benjasHu]

Hi! Thanks for the tips, have helped me a lot.

I've finally move the undoable reducer to the root of the state, and in this way I can encapsulate its history in this subtree.

But, one more question. when I assign a custom undoType, everything works but the action is not dispatched inside the undoable reducers (in all the others do).

Is this the expected behaviour or it's expected that the action were dispatched in all of reducers (including the undoable)?

[benjasHu]

Ok! I finally get it to work. In my case, I'm managing game cards, so:

// reducer
function cards( state={}, action ) {

  switch(action.type) {

    case UNDO_CARDS:
      return { ... something to reduce ... }

    default:
      return state
  }
}

undoable(cards, {
  undoType: HISTORY_UNDO_CARDS
})

//action
export function undoCards() {
  return dispatch => {

    dispatch({ type: HISTORY_UNDO_CARDS })

    return dispatch({ type: UNDO_CARDS })
  }
}

Is this the way you were talking about?

[omnidan]

@benjasHu It's hard to tell from your example - but if you use multiple undoable instances and each of them has a different undoType, then yes, that's the way to have two independent instances of redux-undo. Without specifying undoType, all your undoable instances will undo at the same time.

[benjasHu]

@omnidan Ok, I'm working in this way and it works perfectly ;)

Thanks for your time!

[omnidan]

you're welcome! I'm glad you got it to work :grin:

[mindjuice]

I also want to do multiple document undo/redo. I'm planning to dynamically add a reducer for each document and wrap each one in undoable(). This link describes adding dynamic reducers: http://stackoverflow.com/a/33044701/332048

The problem is I also want to use TypeScript for this project. When using fixed action strings, TS is able to guarantee type safety for the actions, and can do automatic type narrowing in the reducers so that you can access action-specific payload arguments in a type-safe way. Some info on this here: https://spin.atomicobject.com/2016/09/27/typed-redux-reducers-typescript-2-0/

Using a dynamically-generated action type for undo, redo, etc. as described in the previous comments does not allow for this type safety, so I'd like to stay away from that (although I don't think there are any type definitions for this lib yet anyway -- if I end up using redux-undo with TS, I'll try working on that).

I've been bouncing around a few ideas on how to handle this in a type-safe way. One way would be the ability to pass a shouldApplyAction function to undoable() (similar to how filter is passed). This function would be called with the action as an argument, and could be bound to other state via closure or partial application (e.g., to pass your store to it).

In the case of a multiple document app, the shouldApplyAction function could then check to see if this reducer was for the current document and if not, ignore the action (e.g., compare the state's currentDocumentId to the documentId in the slice of state managed by the reducer). This way, only one of the reducers would apply the action and the others would ignore it.

Another way might be a context parameter that could be passed into the redux-undo action creators.

One other thing I'm still not sure the right way to handle are actions that are for the app as a whole, not a specific document. This includes:

• Changing between documents
• Changing view layouts
• Changing global app settings

It's tricky because the user might do something in the document, then do something in the main app, then do more things in the main document. Should those things be undone in the same order? What if the user switches to another document? Has anyone else has thought about these things and found a clean way to handle them?

[samlasserson]

@mindjuice I've got multiple undo/redo working with a variation of the currentDocumentId method you mentioned, which presented itself when I normalized my state shape: my solution is to call undoable() 'behind' my document entity object, which is keyed by id, for example:

// Given this normalized state object (using Immutable.js)
state: {
  activeDocumentID: 1,
  documents: Map({
    entities: Map({
      1: { DocumentObject1 }, // in my case these are Immutable Records
      2: { DocumentObject2 },
      3: { DocumentObject3 }
    }),
    result: List([1,2,3])
  })
}

In documentsReducer.js, which is called from your root reducer with the documents state slice:

import { Map, List } from 'immutable'
import { combineReducers } from 'redux-immutable'
import documentReducer from './documentReducer'

const entitiesReducer = (state = new Map(), action) => {
  const { documentID } = action.payload
  if( action.type === 'DELETE_DOCUMENT' ){ 
    return state.remove(documentID)
  }
  return state.set(documentID, documentReducer(state.get(documentID), action))
}

const resultReducer = (state = new List(), action) => {
  const { documentID } = action.payload
  // Respond to document creation, removal, reordering here
  switch( action.type ){
    case 'CREATE_DOCUMENT':
      return state.push(documentID)
    case 'DELETE_DOCUMENT':
      return state.filter( id => id !== documentID )
    // etc...
    default: return state
  }
}

const INITIAL_STATE = new Map({
  entities: new Map(),
  result: new List()
})

export default (state = INITIAL_STATE, action) => {
  const { documentID } = action.payload
  if( documentID ){
    // Only respond if there is a specified document ID
    return combineReducers({
      entities: entitiesReducer(state.get('entities'), action),
      result: resultReducer(state.get('result'), action)
    })
  }
  return state
}

In documentReducer.js:

import undoable, { excludeAction } from 'redux-undo'

const documentReducer = (state, action) => {
  // Any action types you respond to here will only apply to the
  // document instance specified by documentID, which is the
  // state parameter passed in above
  switch(action.type){
    case 'CREATE_DOCUMENT':
      return action.payload.document
    // All document edit actions go here...
    default: return state
  }
}

const config = {
  // Your specific redux-undo config. You may need to tweak initTypes or
  // ignoreInitialState here depending on how you hydrate your store to ensure undoable
  // starts with a valid state
  undoType: 'DOCUMENT_UNDO',
  redoType: 'DOCUMENT_REDO',
  // define any other types you need.
  filter: excludeAction(someArrayOfUIActions.concat(anythingElseYouWantToIgnore))
}
export default undoable(documentReducer, config)

This way you get consistent action and undo typing with independent undo/redo. In my application, document editing is so integral that I have a simple middleware which sets action.payload.documentID to the value of getState().activeDocumentID if it hasn't been defined before the action hits the store, and I filter any UI or other actions which I don't want reflected in the document undo history in the config object passed to undoable. I then simply fire a SET_ACTIVE_DOCUMENT action with the new documentID in the payload on routing between different documents, and any actions performed on the new document have the new documentID injected by the middleware and flow through the appropriate undo history. Dispatching DOCUMENT_UNDO or DOCUMENT_REDO along with payload.documentID defined or injected only hits the undoable reducer behind entities[documentID].

Hope the code and explanation are clear. As for app state, a separate undoable instance with namespaced undoTypes could hold your ui subtree or whatever, and then adding those app-namespaced undoTypes to the filter parameter in the document reducer undoable config would avoid unrelated actions polluting the document undo history. In terms of undoing settings and such whilst editing documents, it would be fairly implementation-specific - you could have a popup undo button after changing things in the main app which triggers the app undo and then removes itself on the next action or times out without user interaction, separate from a toolbar with document undo/redo buttons for example.

[mindjuice]

Thanks Sam...I'll be getting around to this in the next week or so.

Everything was clear except that I don't follow how you are preventing the DOCUMENT_UNDO and DOCUMENT_REDO actions from being processed by every undoable() reducer.

You said

Dispatching DOCUMENT_UNDO or DOCUMENT_REDO along with payload.documentID defined or injected only hits the undoable reducer behind entities[documentID].

Where is that done?

[samlasserson]

With this state / reducer shape, only one undoable reducer at a time is ever called - the one which corresponds to documentID (see entitiesReducer in the code block above). The reducers for documents with different IDs will not be called, and so will not result in a distinct state or touch the undo history.

Dispatching the action is likely to be from a UI component, if you're using React then react-redux's connect makes it easy to grab the state.activeDocumentID (or rely on middleware so you don't have to keep selecting it and passing it around) and fire the undo/redo types.

[mindjuice]

I see. I missed what was happening on this line:

return state.set(documentID, documentReducer(state.get(documentID), action))

I'm confused by your combineReducers() code though. Why is it calling the reducers there? That doesn't make sense.

return combineReducers({
  entities: entitiesReducer(state.get('entities'), action),  // <== wut?
  result: resultReducer(state.get('result'), action)
})

[samlasserson]

Where else would you call it?

state.documents => documentsReducer state.documents.entities => entitiesReducer state.documents.result => resultReducer

Given documents: { entities: {}, result: [] }, the documents reducer should pass the state and action along to the reducers which handle the next slice, i.e documents.entities and documents.result. combineReducers is just a utility function to call multiple reducers on nested state slices, a functional equivalent in this case would be:

return state.set('entities', entitiesReducer(state.get('entities'), action))
           .set('result', resultReducer(state.get('result'), action))

UPDATE: Been a while since I used combineReducers (my bad), thought it would make the code look more familiar but neglected to check the function signature - that's not how it works, you're quite right. The code above should make sense.

[mindjuice]

Well normally combineReducers() takes an object mapping keys to functions and returns a function. You're calling combineReducers() with an object mapping keys to state values which makes no sense in normal redux.

I guess this redux-immutable does something quite different. Poor choice to give it the same name as the redux function.

[samlasserson]

No, you're right - I just hadn't written anything with it for a while and used it wrongly - see update to my earlier comment.

[mindjuice]

OK...phew! Thanks for the clarification.

[dotellie]

I almost have something like this working (well, I thought I had it working) but now I'm starting to get an error about redux-undo not being able to read the length of history.past because it's treating my state, which is only meant to represent the present, as the full undo history.

Basically, I have an action called REPLACE which takes in a full state which I read from indexedDB and then replaces the entire redux state with it. When it comes to the undoable reducer a second time, it gives the error. I have kind of narrowed it down to initialState at line 556 already being populated when it shouldn't be. I'm not too familiar with the code of redux-undo, but isn't this breaking the whole immutable paradigm to have redux-undo keep track of the initial state outside of the store? Any ideas on how to fix it maybe?

Stack trace from Chrome:

Uncaught (in promise) TypeError: Cannot read property 'length' of undefined
    at lengthWithoutFuture (redux-undo.js:453)
    at insert (redux-undo.js:461)
    at module.exports (redux-undo.js:658)

[samlasserson]

Are you passing a config object as the second argument to undoable()? You need to specify your custom rehydrate action as an initType, so redux-undo knows to construct a history around the state passed to the reducer when this action is fired. Otherwise, it will treat the incoming state as a history object when it isn't, and fail loud and fast as in your stack trace - that's my best guess, hard to be sure without seeing any of your code.

Try this:

/* Assuming we're in the file where you export your undoable instances from,
 * your reducer is imaginatively named 'reducer', and you've imported * as ACTIONS...
 */

const undoConfig = {
  initTypes: [ACTIONS.REPLACE]
}

export default undoable( reducer, undoConfig )

It sounds like you may also need to specify that the initialState should be ignored the first time. If the above doesn't solve the issue, try the following config object:

const undoConfig = {
  initTypes: [ACTIONS.REPLACE],
  ignoreInitialState: true
}

HTH

[dotellie]

That almost worked. It doesn't throw an error when adding those parameters but it destroys every entity that comes after the first one (replacing them with a copy of the first entity). :sweat_smile:

Is it possible the problem is that I'm not calling undoable for every entity? Reading all that's here kinda makes me think that's what I have to do, but I don't really know how I would go about that... Right now I just have

const entity = undoable((state = {}, action) => { ... }, config)

[davidroeca]

@magnonellie I think your issue is this https://github.com/omnidan/redux-undo/issues/163

Check my latest comment there--it has to do with how the reducer is created. The undoable wrapper returns a function that is inherently stateful, which likely leads to this problem

[dotellie]

@davidroeca Yeah, I saw that one too, but I decided this was probably a better place to ask. Anyway, just to be clear, is the solution for now to essentially copy those functions since they aren't exported right now?

[davidroeca]

That may be one fix for now, though it depends on what's stored in your database. The reducer only works well with past/present/future once the state has been initialized, so you'll have to convert the modified state to something with the same format

[samlasserson]

@magnonellie undoable is a higher-order reducer, in that it takes a reducer as argument and returns a reducer. If you call it like that you will recreate the reducer on every action, surely? Where is the snippet you posted taken from?

[dotellie]

@davidroeca I may not have understood correctly, but reading the readme, it has a section where it says that I should be able to just pass in a normal value and redux-undo will convert it for me. Isn't it a bug then that it's not doing that? :thinking:

@samlasserson Sorry, I should have made my snippet a bit more clear. I'm referencing entity as my reducer later in the code. Should probably rethink the name of that now that you mention it. :nerd_face:

At any rate... I think I managed to fix the issue by just creating an object pretending to be the history when replace is called and removing the initTypes parameter. Passed the test I wrote and worked well enough with some basic manual testing.

Thanks for the help you two! I really appreciate it! :heart:

[davidroeca]

@magnonellie that section references initialization, not full state replacements.

If anything, it's an issue with lack of documentation and fundamental design constraints rather than a bug per se.

[corvana-nbaker]

I've tried to set this up for a document-based design just as @samlasserson has.

undoable() reducers are added dynamically to the application. Unfortunately when they're called they never receive the current state as there's no "state.present" in the undoable wrapper, just a state.history.

I understand initialState is being set in the createStore for now, but this won't work for my application as the store was created long ago

[samlasserson]

@corvana-nbaker What does your undoable config object look like? I have this working dynamically, i.e independent of initialState definition or store creation. Presumably your documents have some form of CREATE action, is this specified in an array of initTypes in config? Also, ignoreInitialState may need to be set to true (again in config), I think this is one of the signifiers for redux-undo to generate a history object.

[corvana-nbaker]

@samlasserson I think my issue was I was trying to add an undoable reducer to a portion of the state tree already in existence. Since then I've changed it so the creation of that portion of the tree is done After adding the undoable reducer and now I have history.

Thanks!

[omnidan]

released beta9-9-7 with the newHistory function, let me know if that solves your issues