-
Notifications
You must be signed in to change notification settings - Fork 184
Commit
Named after Fancy Shmancy (something expensive and impressive) it extends the Fancy reporter and allows you to set a custom log format using sprintf expressions
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
export { default as BasicReporter } from './basic' | ||
export { default as BrowserReporter } from './browser' | ||
export { default as FancyReporter } from './fancy' | ||
export { default as ShmancyReporter } from './shmancy' | ||
export { default as JSONReporter } from './json' | ||
export { default as WinstonReporter } from './winston' |
8 comments
on commit dc6121a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pi0 Although I expected like you that using sprintf would be more expensive, it appears that currently FancyReporter is about 1.6 times slower then ShmancyReporter (see the adapted demo in this PR)
This is partly because of the lots of writes in FancyReporter (in my tests it could be 10% faster if there was only one call to write) but also the color cache I (had to) use with Shmancy provides a great performance boost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, forgot to add shmancy self, see 2929648
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pimlie We can batch all write operations into a single call. Will do it. BTW if it is going to be an standard to support printf, we can add some utils into Basic reporter instead of introducing a new reporter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we would use printf by default we also need to add the date. Would probably be nice to be able to set a custom date format as well then (toLocaleFormat
instead of toLocaleTimeString
).
If I remove all color stuff from Shmancy, its ~25% slower then Basic (26ms vs 20ms for 10 demo iterations). Let me know if you want me to put the printf stuff in Basic and just keep Basic/Fancy as we have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pimlie I think that small diffs are not that important. But I agree to add time formatting option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, will remove shmancy then and incorporate printf in basic and set default formats for basic + fancy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind to use moment.js
for formatting the datetime? It seems there is no vanilla date formatter which works consistently along all browsers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pimlie Moment is too big! Maybe dayjs or an equalant with es support. Also browser implementation is separated.
I intentionally didn't move chalk dependent utils into utils, because rollup tree-shaking was not working. Do you confirm they are tree-shake in browser bundle?