Console Group Rework

[pl12133]

Address #66 by grouping arguments together and calling console.group once all the arguments are available.

[peteruithoven]

@pl12133 Thanks for picking this up so quickly!

[pl12133]

@peteruithoven No problem! Your reference to redux-logger was a really good idea and I think I'd like to copy their style for these debug messages.

Your issue actually made me realize a larger issue: the undoable debug option is causing all actions to be logged, not just the ones from the undoable reducer, so that is actually rather important.

[peteruithoven]

I notices the following commit message:

Actions which are not handled by the undoable should not be logged to the console.

Because it's coupled with a few other changes I'm having a hard time estimating how this will work. Doesn't the undoable handle all actions, so it can store them in history? Or are logging all actions except the ones that are ignored (by the filter function)?

[pl12133]

I think we're thinking of slightly different cases here, I'll give some examples to show the two different cases I have in mind.

Example 1: Undoable root reducer

import {combineReducers} from 'redux';
import reducerA from 'reducerA';
import reducerB from 'reducerB';
import undoable from 'redux-undo';

const rootReducer = combineReducers({
  reducerA,
  reducerB 
});

export default undoable(rootReducer)

Example 2: Undoable child reducer

import {combineReducers} from 'redux';
import reducerA from 'reducerA';
import reducerB from 'reducerB';
import undoable from 'redux-undo';

const rootReducer = combineReducers({
  undoable(reducerA),
  reducerB 
});

export default rootReducer

If I understand you correctly you are thinking of Example 1 but I was adressing a bug that occurs in Example 2.

So in Example 2, if the undoable reducer has debug: true, it would print ALL actions going through the rootReducer. The user would expect to see debug messages for actions relating to reducerA, but they would also receive debug messages related to reducerB. My change should fix this problem, and only show debug messages relating to reducerA.

Hopefully that clarifies what I meant in my commit message 😸

[teameh]

Looking good!

[pl12133]

Fast-forwarded and squashed commits.

[teameh]

My change should fix this problem, and only show debug messages relating to reducerA.

Although the code looks good and I understand why you came to this, I'm not sure if that is prefereble. When not working on your reducers debug is probably off right? And then when you turn it on, I would like to be informed of all actions that are passed into my reducer, also those that do not result in insertions in our history. I kinda liked our "side effect" that showed: "not insterted: unchanged state"...

[peteruithoven]

Logging or not logging depending on whether the actions are related to the undoable reducer seems like a separate issue? Maybe we could move that to a separate pull request? I'm actually of the opinion that it should be optional, it is useful to see for example when tweaking the filter function.

[teameh]

Well this PR is changing that logic so it might be handy to discuss here right? Otherwise we get another PR that might change it back and we've got a more confusing git history.

[pl12133]

The problem with displaying all actions that go through an undoable reducer is that every time an action occurs the undoable reducer will only be able to print its piece of the state, not the entire state tree. If a users wants to just view (state, action) => newState, they should probably be using redux-logger. You should still be able to see all messages related to the filter function in order to debug that effectively.

[teameh]

Fair point. I think you're right 👍. It could be useful to add some docs about logging then. In which cases you need redux-logger for example.

[pl12133]

Oh and I'm not sure about not calling debug.end() this could have been the same call where the history is created right? So that will then not appear in the log.. Right?

Yes, that is correct and a good point. redux-logger also hides @@redux/INIT and @@redux/PROBE. I'm not sure if we need to include those actions, but maybe someone would want to include them.

[teameh]

Showing the @@redux/PROBE only causes confusion because devtools will complain if the result of the probe (equal state) is not right and devtools and the logger both don't show it.

I someone would like to include them one could always manually console log the actions passed into the reducer..

[teameh]

Fair point. I think you're right 👍. It could be useful to add some docs about logging then. In which cases you need redux-logger for example.

Is this something we would like to do before merging this? Otherwise this is good to go right?

[pl12133]

I believe this is fine to be merged. With the each action handled by an undoable will show prevState, action, and nextState. Then it will print a short message about which branch of the switch (action.type) statement was taken.

[omnidan]

Sorry it has taken me so long - thanks a lot! 😁 I released beta7 so feel free to try it out and let me know if it works 😉