Skip to content
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

cljs: JS objects are printed uselessly #356

Closed
DerGuteMoritz opened this issue Sep 27, 2022 · 2 comments
Closed

cljs: JS objects are printed uselessly #356

DerGuteMoritz opened this issue Sep 27, 2022 · 2 comments
Assignees
Milestone

Comments

@DerGuteMoritz
Copy link
Contributor

Plain JS objects are printed rather uselessly by default in cljs. E.g.

(log/info #js {:foo "bar"})

which results in:

2022-09-27T14:04:14.746Z INFO [user:1] - [object Object]

JS arrays are also a bit odd in that they lack delimiters. E.g.

(log/info #js [1 2 3])

which results in:

2022-09-27T14:04:15.919Z INFO [user:1] - 1,2,3

We've worked around this with the following middleware so far:

(defn pr-js-objects-in-log-args
  "This is needed because timbre internally calls `toString` on log
  args, which means plain JS objects are logged as `[object Object]`."
  [data]
  (update data :vargs (fn [vargs]
                        (map (fn [arg]
                               (if (string? arg)
                                 arg
                                 (pr-str arg)))
                             vargs))))

Which makes the above examples print this instead:

2022-09-27T14:05:43.211Z INFO [user:1] - #js {:foo "bar"}
2022-09-27T14:05:44.023Z INFO [user:1] - #js [1 2 3]

Maybe worth including something like this by default?

@ptaoussanis
Copy link
Member

@DerGuteMoritz Hi Moritz, thanks for pinging about this.
I'd like a little time to think about the best way to address this.

My quick first impression:

Middleware will work, but may be needlessly expensive.
In principle, might be better to transform vargs only when the :msg_ delay is actually evaluated.

Could provide some kind of option for a transform at that point, though need to think about the possibilities and tradeoffs.

Thoughts/ideas also welcome!

@ptaoussanis ptaoussanis added this to the v6 milestone Oct 18, 2022
@ptaoussanis ptaoussanis self-assigned this Oct 18, 2022
@ptaoussanis
Copy link
Member

This will be addressed in forthcoming release.

Two relevant changes:

  1. The default behaviour will be adjusted to match yours here.
  2. It'll be possible to swap in an alternative message fn.

Thanks again for pinging about this 👍

ptaoussanis added a commit that referenced this issue Oct 20, 2022
Enables user-configurable processing of vargs -> message
ptaoussanis added a commit that referenced this issue Oct 20, 2022
Status

  While now undocumented, backwards compatibility will be retained since:
    1. The back compatibility is cheap and simple
    2. Appenders in the wild may already depend on the argument
       (incl. several community appenders)

Motivation

  The problem with :msg_ is that its behaviour can't be easily customized,
  and customization can be useful (e.g. for #356).

  So instead of hard-coding a particular :msg_ generator as before, the
  responsibility of message generation will now be handled as just another
  aspect of output generation (i.e. handled by output-fns).

  Note that output fns already have rich customization facilities, incl.
  per-appender fn and options overrides.
ptaoussanis added a commit that referenced this issue Oct 22, 2022
Enables user-configurable processing of vargs -> message
ptaoussanis added a commit that referenced this issue Oct 22, 2022
Status

  While now undocumented, backwards compatibility will be retained since:
    1. The back compatibility is cheap and simple
    2. Appenders in the wild may already depend on the argument
       (incl. several community appenders)

Motivation

  The problem with :msg_ is that its behaviour can't be easily customized,
  and customization can be useful (e.g. for #356).

  So instead of hard-coding a particular :msg_ generator as before, the
  responsibility of message generation will now be handled as just another
  aspect of output generation (i.e. handled by output-fns).

  Note that output fns already have rich customization facilities, incl.
  per-appender fn and options overrides.
ptaoussanis added a commit that referenced this issue Oct 23, 2022
Enables user-configurable processing of vargs -> message
ptaoussanis added a commit that referenced this issue Oct 23, 2022
Status

  While now undocumented, backwards compatibility will be retained since:
    1. The back compatibility is cheap and simple
    2. Appenders in the wild may already depend on the argument
       (incl. several community appenders)

Motivation

  The problem with :msg_ is that its behaviour can't be easily customized,
  and customization can be useful (e.g. for #356).

  So instead of hard-coding a particular :msg_ generator as before, the
  responsibility of message generation will now be handled as just another
  aspect of output generation (i.e. handled by output-fns).

  Note that output fns already have rich customization facilities, incl.
  per-appender fn and options overrides.
ptaoussanis added a commit that referenced this issue Oct 23, 2022
Change (both Clj and Cljs):

  For print-style logging commands (`debug`, `info`, etc.):

    By default, `pr-str` (rather than `str`) is now called on all non-string
    arguments when generating `:msg_` and `:output_` vals in Timbre's log data
    map.

    I.e. will affect generated output.

  For format-style logging commands (`debugf`, `infof`, etc.):

    No change since format-style logging commands presume that a format
    pattern will explicitly specify the desired formatting, so arguments
    are not automatically formatted.

    I.e. should not affect generated output.

Motivation:

  My hope is that this change should be positive or neutral for the vast
  majority of users. There are several known cases where `pr-str` produces
  better output (e.g. for JS objects in Cljs), and currently no known
  cases where `pr-str` produces worse output (though please ping me if you
  know of something!).

To customise this behaviour (message generation), see the `default-output-fn`
docstring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants