Skip to content

feat(logger): Logger improvements#248

Open
dpogue wants to merge 2 commits intoapache:masterfrom
dpogue:logger-fixes
Open

feat(logger): Logger improvements#248
dpogue wants to merge 2 commits intoapache:masterfrom
dpogue:logger-fixes

Conversation

@dpogue
Copy link
Member

@dpogue dpogue commented Jan 24, 2026

Platforms affected

All

Motivation and Context

  • Sometimes (particularly in cordova-ios test cases) it seems like the CordovaEventEmitter gets subscribed to CordovaLogger multiple times, resulting in duplicate log output lines.

  • Node 20.12.0 and newer include util.styleText for handling console text styling. Prefer to use stdlib functionality rather than external dependencies.

Description

  • Added a WeakMap to store subscribed emitters in CordovaLogger and no-op if the same emitter is subscribed more than once

  • If util.styleText is available, use that instead of the ansi module for formatting the CordovaLogger output.

    • In the next major, we can drop the ansi dependency and clean up the internals of CordovaLogger

Testing

Added new test cases for changed code.

Visually tested output with both util.styleText and ansi to confirm the output is identical.

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change

@dpogue dpogue requested a review from erisu January 24, 2026 06:35
@dpogue dpogue self-assigned this Jan 24, 2026
@codecov-commenter
Copy link

codecov-commenter commented Jan 24, 2026

Codecov Report

❌ Patch coverage is 77.55102% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.17%. Comparing base (86a9779) to head (afe49b4).

Files with missing lines Patch % Lines
src/CordovaLogger.js 77.55% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #248      +/-   ##
==========================================
- Coverage   95.40%   95.17%   -0.24%     
==========================================
  Files          20       20              
  Lines        3917     3955      +38     
==========================================
+ Hits         3737     3764      +27     
- Misses        180      191      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

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

There seems to be an issue with util.styleText when running tests.

  • 20.9.0 passes.
  • 20.20.0 passes.
  • 20.12.0 fails.

There appears to be a difference in the behavior of util.styleText between version 20.12.0 and newer.

The following error will appear:

CordovaLogger class instance log method Test 011 : should log everything except error messages to stdout

  • TypeError: The argument 'format' must be one of: 'reset', 'bold', 'dim', 'italic', 'underline', 'blink', 'inverse', 'hidden', 'strikethrough', 'doubleunderline', 'black', 'red', 'green', 'yellow', 'blue', 'magenta', 'cyan', 'white', 'bgBlack', 'bgRed', 'bgGreen', 'bgYellow', 'bgBlue', 'bgMagenta', 'bgCyan', 'bgWhite', 'framed', 'overlined', 'gray', 'redBright', 'greenBright', 'yellowBright', 'blueBright', 'magentaBright', 'cyanBright', 'whiteBright', 'bgGray', 'bgRedBright', 'bgGreenBright', 'bgYellowBright', 'bgBlueBright', 'bgMagentaBright', 'bgCyanBright', 'bgWhiteBright'. Received [ 'bold', 'grey' ]

The same error will print also for Test 012 and 013

In the next major when we drop Node 20 support, we can remove the ansi
dependency and refactor CordovaLogger to clean up the exposed API a bit.
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.

3 participants