Repository navigation
fix: add setBindings to the browser logger - #2471
Conversation
|
For transparency: after opening this I noticed #2403 tackles the same issue and predates this PR. I'd normally close mine in favor of the earlier one, but the implementations differ in ways that matter, so leaving this open for the maintainers to pick:
Happy to close either way if the maintainers prefer building on #2403. |
|
Re the red CI: the failing test ( |
|
Please rebase |
The browser logger did not implement setBindings even though the types declare it on LoggerExtras, so calling logger.setBindings() in a browser build threw "setBindings is not a function". Merge the new bindings into the logger's bindings, mirror them into _logEvent for transmit, and rebuild the level functions so subsequent log calls prepend the updated bindings. Fixes pinojs#2021
fa07b69 to
91fb6bb
Compare
|
Rebased on main (now includes the #2470 flush-test fix). Browser tests and the previously failing transport test both pass locally. |
Fixes #2021
Problem
The browser logger (
browser.js) never implementedsetBindings, even though the TypeScript types declare it onLoggerExtras— so any code path that resolves thebrowserfield ofpackage.jsonthrowsTypeError: logger.setBindings is not a function. (To answer the question in the issue thread: it's a JS bug — a missing method in the browser implementation, not a typings problem. The named/default import distinction in the report was a red herring; both exports point at the same function, what differs is whether the bundler pickedpino.jsorbrowser.js.)Fix
Implement
setBindingson the browser logger:bindings(children inherit the method through the prototype chain and merge into their own bindings),_logEvent.bindingssobrowser.transmitreceives them, matching whatchild()does,Consistent with Node semantics: like
asChindingsinlib/proto.js, updating a parent's bindings does not retroactively affect already-created children.Testing
New
test/browser-set-bindings.test.js(tape, same style as the other browser tests): method exists, bindings appear in subsequent logs, repeated calls merge, child loggers merge with their own bindings, works withcustomLevels(the issue's repro), and bindings are transmitted. All existing browser tests and lint pass.