-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New Options #2
Comments
@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. |
@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. |
@jbucaran see the updated list of proposed options above. |
Excellent list @okwolf! Other than IMO After that I'd consider What do you think? |
@jbucaran I agree with the priority, reordered the list. I also like the shorter name suggestions. To me |
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. |
@jbucaran for |
I thought For format, I had forgotten we already have a custom |
That would be an alternate option, but my original proposal would take the full
Yeah if you provide |
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. |
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 |
That would be pretty nice. I think a way to do this could be to This then raises the issue of letting you define mixins in mixins again. |
Also Come to think of it, Now I'm not sure which options should be priority anymore. 😕 |
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 That code looks a bit familiar 😉 And your |
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. |
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 returningtrue
.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 tofalse
.performance
- show how long the action took from start to end. Defaults tofalse
.timing
- show the time the action began and/or ended. Defaults tofalse
.sameStateWarning
- include a warning when not returning a new state object from an action. Defaults tofalse
. See Logs the same object as prev and next state #3.The text was updated successfully, but these errors were encountered: