Field extensions

[nmay231]

Please check if the PR fulfills these requirements

• The commit message(s) are descriptive of the changes made
• The PR contains changes that are focused and differs from other PRs
• Tests for the changes have been added where needed
• Docs have been added / updated

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Adds a config option extension that allows the user to add different "field extensions." The combineExtensions helper allows the user to use multiple at once.

Included is the much requested flattenState and actionField.

What is the current behavior? (You can also link to an open issue here)

Requested in #179 and #233.

There are also WIP versions for helping with #150 and #237.

What is the new behavior?

Extensions would be added like so.

undoable(reducer, {
  extension: actionField({ insertMethod: 'inline' })
})

// alternatively, use combineExtensions to add multiple extensions
undoable(reducer, {
  extension: combineExtensions(
    flattenState(),
    actionField({ includeAction: (action) => action.included !== undefined })
  ),
  disableWarnings: true
})

disableWarnings disables the warnings that are logged by each field extension that describes how some state might be overridden.

Extension descriptions:

/**
 * The flattenState() field extension allows the user to access fields normally like
 * `state.field` instead of `state.present.field`.
 *
 * Warning: if your state has fields that conflict with redux-undo's like .past or .index
 * they will be overridden. You must access them as `state.present.past` or `state.present.index`
 */

/**
 * The actionField() field extension allows users to insert the last occuring action
 * into their state.
 *
 * @param {Object} config - Configure actionField()
 *
 * @param {string} config.insertMethod - How the last action will be inserted. Possible options are:
 *   - actionType: { ...state, actionType: 'LAST_ACTION' }
 *   - action: { ...state, action: { type: 'LAST_ACTION', ...actionPayload } }
 *   - inline: { ...state, present: { action: { type: 'LAST', ...payload }, ...otherFields } }
 *
 * @param {actionFieldIncludeAction} config.includeAction - A filter function that decides if
 * the action is inserted into history.
 */

Note when using flattenState: all the normal state is still there (.past, .index, etc.) but the user's present state is just copied down a level.

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

This should not break any existing code

Other information:

Docs still have to be updated. However, the readme probably needs a little cleanup anyways.

Docs have been added

[coveralls]

Coverage increased (+0.6%) to 98.305% when pulling 0261d1d on field-extensions into 931b2ec on master.

[nmay231]

@omnidan , I was able to add the docs, and this should be ready to merge as long as it looks good to you. It should definitely be squashed to not have all the annoying commits 😄

One thing to note, I removed some of the extraneous types like IncludeAction and just typed the functions directly. For some reason, CombineFilters (the type) was exported so I put a deprecation flag explaining to just use typeof combineFilters (the function). No code should break though.

Cheers 👍

[omnidan]

Awesome PR and idea! Thanks for implementing field extensions. 😁

I was wondering whether we should put certain field extensions into the core redux-undo library or rather move them into a separate library/project? This would reduce the bundle size for those who do not use field extensions at all. What do you think?

I also noticed some field extensions are not documented and I think the disableWarnings option could be an argument to the field extension functions instead of a redux-undo wide option.

Otherwise this looks great and it will solve many issues people have had with redux-undo! 🎉

[omnidan]

maybe we should only show this warning if we detect that the state contains one of redux-undo's field names, or an array/primitive?

[nmay231]

I thought about that. The issue is you would be checking the returned object for every action that was dispatched to the wrapped reducer (there is no way to check only the first time and then trust it from then on).

I thought it would be best to warn the user exactly once at initialization, no matter what the user has in their state. It should also be very clear in the documentation that this is the case (I can make it more explicit).

Thoughts?

[omnidan]

yeah, I see, that makes sense. then let's keep it as is but put the disableWarnings option as an argument to the function. let's also separate the field extensions into a separate package.

[nmay231]

Sounds good 👌

[omnidan]

also here, should we only warn when we detect that it is overriding something?

[nmay231]

Same idea as above.

[omnidan]

document these in README as well?

do you think the field extensions should be part of redux-undo core, or maybe a separate library? I don't think everyone will need it and it slightly increases the bundle size

[nmay231]

I had briefly mentioned in the PR description that these were WIP and would come out soon, but I realize that it is silly to leave them there. I'll remove them until they are fully implemented. 👍

As for separating them into a different package, that sounds like a good idea! They are purposely optional and, yes, not everyone needs it in their bundle (though tree-shaking is starting to mitigate the problem).

[omnidan]

this could be an argument to the extension function instead of a redux-undo wide config

[nmay231]

We can do that.