Skip to content

feat(browser): support nestedKey in asObject mode and browser.write - #2509

Open
lx3133584 wants to merge 1 commit into
pinojs:mainfrom
lx3133584:feat/browser-nested-key
Open

feat(browser): support nestedKey in asObject mode and browser.write#2509
lx3133584 wants to merge 1 commit into
pinojs:mainfrom
lx3133584:feat/browser-nested-key

Conversation

@lx3133584

Copy link
Copy Markdown

Problem

When using pino in browser / isomorphic environments with browser.asObject: true (or custom browser.write), the nestedKey configuration option was ignored. Logged payload objects were assigned directly to the top-level log object rather than nested under nestedKey, creating an inconsistency between Node.js and browser runtimes.

Root Cause

  • setOpts in browser.js did not store nestedKey.
  • asLogObject unconditionally called Object.assign(logObject, argsCloned.shift()) for all object arguments instead of nesting payload objects under logObject[opts.nestedKey] while keeping bindings at the top level.

Fix

  • Stored nestedKey: opts.nestedKey || opts.browser.nestedKey || null in setOpts.
  • Updated asLogObject to assign payload objects (lvl === 0) to logObject[opts.nestedKey] while assigning binding objects (lvl > 0) to the top-level logObject.

Testing

  • Added unit tests in test/browser.test.js verifying that nestedKey nests payload objects under the configured key in root and child loggers.
  • Verified all 198 browser tests and lint pass cleanly.

Fixes pinojs#2138

Signed-off-by: Liang Xu <lx3133584@users.noreply.github.com>

@kilisamemarisaaa kilisamemarisaaa left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one correctness issue in the collision path. I reproduced it against this head with Node 24.12.0 and compared the emitted objects with the regular Node logger behavior; details are inline.

Comment thread browser.js
while (lvl-- && typeof argsCloned[0] === 'object') {
Object.assign(logObject, argsCloned.shift())
const target = (lvl === 0 && opts.nestedKey)
? (logObject[opts.nestedKey] = logObject[opts.nestedKey] || {})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reuses a same-named child binding as the payload container. On this head, child({ payload: 'binding' }).info({ foo: 'bar' }, 'hello') emits {"payload":"binding","msg":"hello"} in browser asObject mode, so the logged object is lost; the Node logger emits payload: { foo: "bar" }.

If the binding is an object, Object.assign mutates that shared binding instead: two successive logs with { first: 1 } and { second: 2 } make both already-collected records end up with payload: { binding: true, first: 1, second: 2 }.

Please create a fresh nested payload object for each log (overriding a colliding binding) rather than using ||, and add regression cases for primitive/object collisions plus two consecutive log calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants