New Options

[okwolf]

Today the only available option is log to provide a custom logging function. It's a good escape hatch that's highly flexible, but I'm wondering if there should be other options in between. Not proposing to copy every option redux-logger supports, but maybe some are worth the effort.

Here are some proposed options:

• [ ] filter - provide a filter function to decide which actions to log and which to ignore. The filter function is passed the {name, data} object describing each action. Defaults to show all actions by always returning true.
• [ ] format - function to pre-process the state values before being passed to the logger. This allows for returning a new object with the state reorganized to be more readable or in a preferred String format. Defaults to the identity function.
• [ ] diff - try to print an intelligent diff of what state the action updated. In the case of redux-logger deep-diff was used. Defaults to false.
• [ ] performance - show how long the action took from start to end. Defaults to false.
• [ ] timing - show the time the action began and/or ended. Defaults to false.
• [ ] sameStateWarning - include a warning when not returning a new state object from an action. Defaults to false. See #3.

[jorgebucaran]

@okwolf Maybe list what options redux-logger provides (or any other logger) and then narrow the list down to only a few couple of must-haves.

[okwolf]

@jbucaran good point. For now I'm going to keep drawing inspiration from redux-logger since I know it best, but I'm open to suggestions for other good loggers to borrow features from.

[okwolf]

@jbucaran see the updated list of proposed options above.

[jorgebucaran]

Excellent list @okwolf!

Other than sameStateWarning, these options sound good to me.

IMO filter, format and diff, in that order, should be the priority.

After that I'd consider ~timing~ (performance would be better since you'll be using the Performance API).

What do you think?

[okwolf]

@jbucaran I agree with the priority, reordered the list. I also like the shorter name suggestions. To me performance is a better fit if the info displayed is how long each action took to complete but timing makes more sense if we're printing the full timestamp of the start/end to look through the log for which actions were clumped together in time.

[jorgebucaran]

timing makes more sense if you're printing the full timestamp of the start/end to look through the log for which actions were clumped together in time.

Gotcha. So, the question is which one is more useful or can we have both? ;)

[okwolf]

@jbucaran Gotcha. So, the question is which one is more useful or can we have both? ;)

I think we should either have an option for only one of them, or both options. But not one option that does both.

[okwolf]

@jbucaran for filter do you think there's any value in providing the prevState and/or nextState for the filter function to decide logability? How about format for actions? 🤔

[jorgebucaran]

I thought filter would just be an array with action names.

For format, I had forgotten we already have a custom log function, so now I think it's useless or rather, I'd prefer to use options.log than provide a cryptic format string.

[okwolf]

@jbucaran I thought filter would just be an array with action names.

That would be an alternate option, but my original proposal would take the full {name, data} object in case you wanted to decide based on different data for actions with the same name. Or if you had a pattern/prefix/suffix/RegExp to match for the names.

@jbucaran For format, I had forgotten we already have a custom log function, so now I think it's useless or rather, I'd prefer to use options.log than provide a cryptic format string.

Yeah if you provide log then you get full control over the formatting. This option would just be for overriding the formatting of state but leaving the rest of the look-and-feel to the log messages the default.

[jorgebucaran]

IMO sounds like it's trying to do too much. I would start with the minimal things that can be added to improve the logger just a bit. If a use case for another X or Y come up in the future, then definitely why not! :)

Alternatively, you could think for a way to add "plugins" to the logger, so that those solutions can exist outside @hyperapp/logger. Similar to how we push some things outside core in hyperapp.

[okwolf]

@jbucaran Alternatively, you could think for a way to add "plugins" to the logger, so that those solutions can exist outside @hyperapp/logger. Similar to how we push some things outside core in hyperapp.

One approach for extension would be to provide an option for plugins/mixins/middleware - each of which is a function that takes the parameters for the log function { prevState, action, nextState } as arguments and then returns the next one or false to cancel the log operation. I'm not sure if it would make sense to ever have more than one, but that would be a consistent API. This would supersede any need for implementing filter or format.

[jorgebucaran]

This would supersede any need for implementing filter or format.

That would be pretty nice.

I think a way to do this could be to emit your own log event so that users, as well as mixins can tap into that event to provide filter / format functionality.

This then raises the issue of letting you define mixins in mixins again.

[okwolf]

Also timing and performance could be pushed outside the library by adding start and end properties to the action object to be used as desired.

Come to think of it, sameStateWarning could also be done inside a mixin-like thing or even using the existing log option.

Now I'm not sure which options should be priority anymore. 😕

[jorgebucaran]

Timing action duration could be up to a different mixin too.

[okwolf]

@jbucaran Timing action duration could be up to a different mixin too.

True, it could. But it might also make sense to group the performance information with the rest of the action info and state values in the log.

That code looks a bit familiar 😉 And your ignore reminds me of what you originally thought the filter option was for.

[naugtur]

As for same state warning, if it's there it should be in by default and point to a page with explanations why it's bad and what should be done about it.

My reasoning is, if you know what the flag is for, you don't need it much anymore.