Skip to content

fix(browser): keep json option and full context in child() - #49

Open
O1sumitkumar wants to merge 2 commits into
Logixia:mainfrom
O1sumitkumar:fix/browser-child-json-31
Open

O1sumitkumar wants to merge 2 commits into
Logixia:mainfrom
O1sumitkumar:fix/browser-child-json-31

Conversation

@O1sumitkumar

Copy link
Copy Markdown

What and why

BrowserLogger.child() had two problems (#31):

  1. The parent's json setting wasn't passed on. child() built a new BrowserLogger without json, so the constructor gave the child a fresh BrowserConsoleTransport({ json: false }). Children of a json: true logger printed pretty, grouped console output instead of JSON lines.
  2. const { _, ...rest } = extraContext stripped a key named _. The comment said this was meant to strip context, but the JSDoc example keeps context in the payload.

Changes in src/browser.ts:

  • json is now stored on this.config (default false). The built-in console transport reads it from there, and child() passes it to the new logger.
  • child() merges every key of extraContext over the parent context. The _ destructure and the misleading comments are removed. context stays in the payload, as the JSDoc example shows. That was the least surprising option the issue suggested.

No public API or type changes: json was already a BrowserLoggerConfig option.

Closes #31

How I tested it

New tests in src/__tests__/browser.test.ts (describe('BrowserLogger.child()')). They spy on console.log and JSON.parse the output:

  • A child of a json: true logger writes a JSON line with its context in payload.
  • The child payload has the parent context plus the new fields, including context.
  • A grandchild keeps both levels of context.

I ran these in a scratch setup with the repo's jest.config.js, tsconfig.json, eslint.config.mjs and .prettierrc, plus src/browser.ts and the src/utils files it imports, on Node 20:

  • npx jest src/__tests__/browser.test.ts: 13/13 pass. Against the unpatched browser.ts, the 3 new tests fail and the 10 existing ones pass.
  • npx tsc --noEmit -p tsconfig.json: passes.
  • npx eslint src/browser.ts src/__tests__/browser.test.ts: no errors or warnings.
  • npx prettier --check on both files: passes.

I did not run the full npm test suite across the whole repository. Only src/__tests__/browser.test.ts imports ../browser.

Checklist

  • PR title follows Conventional Commits (feat: ..., fix(transport): ..., docs: ...). It becomes the squash commit and drives the release.
  • npm run typecheck && npm run lint && npm test pass locally (ran typecheck/lint/jest on the changed files only, see above)
  • Tests added or updated for behaviour changes
  • README or examples updated if the public API changed (no public API change)

This branch has not been deployed

No deployments
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.

fix(browser): child() loses the json option and strips the wrong key

1 participant