This is an automated email from the ASF dual-hosted git repository.
dpogue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-common.git
The following commit(s) were added to refs/heads/master by this push:
new 2e68581 feat(logger): Logger improvements (#248)
2e68581 is described below
commit 2e68581fbebbe7a258f8a872ada328cc51f03c55
Author: Darryl Pogue <[email protected]>
AuthorDate: Fri Aug 28 01:54:06 2026 -0700
feat(logger): Logger improvements (#248)
* fix(logger): Ensure emitters can't get double-subscribed
* refactor(logger): Use util.styleText when possible
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.
---
spec/CordovaLogger.spec.js | 129 ++++++++++++++++++++++++++++++++-------------
src/CordovaLogger.js | 60 +++++++++++++++++----
2 files changed, 140 insertions(+), 49 deletions(-)
diff --git a/spec/CordovaLogger.spec.js b/spec/CordovaLogger.spec.js
index 31ba2b3..d7244fc 100644
--- a/spec/CordovaLogger.spec.js
+++ b/spec/CordovaLogger.spec.js
@@ -19,8 +19,9 @@
const CordovaError = require('../src/CordovaError');
const CordovaLogger = require('../src/CordovaLogger');
-const EventEmitter = require('events').EventEmitter;
+const { EventEmitter } = require('node:events');
+const hasStyleText = CordovaLogger.__hasStyleText;
const DEFAULT_LEVELS = ['verbose', 'normal', 'warn', 'info', 'error',
'results'];
describe('CordovaLogger class', function () {
@@ -105,58 +106,110 @@ describe('CordovaLogger class', function () {
const emitter = new EventEmitter().on('newListener',
listenerSpy);
logger.subscribe(emitter);
});
+
+ it('should avoid subscribing the same emitter multiple times',
function () {
+ spyOn(logger, 'log');
+
+ const emitter = new EventEmitter();
+ logger.subscribe(emitter);
+ logger.subscribe(emitter);
+
+ emitter.emit('info', 'This is an error message');
+
+ expect(logger.log).toHaveBeenCalledTimes(1);
+ });
});
- describe('log method', function () {
- function CursorSpy (name) {
- const cursorMethods = ['reset', 'write', 'bold'];
- const spy = jasmine.createSpyObj(name, cursorMethods);
+ if (hasStyleText) {
+ describe('log method', function () {
+ beforeEach(function () {
+ logger.stdoutCursor = jasmine.createSpyObj('stdout',
['write']);
+ logger.stderrCursor = jasmine.createSpyObj('stderr',
['write']);
+ });
- // Make spy methods chainable, as original Cursor acts
- cursorMethods.forEach(function (method) {
spy[method].and.returnValue(spy); });
+ it('Test 010 : should ignore message if severity is less than
logger\'s level', function () {
+ logger.setLevel('error').log('verbose', 'some_messgge');
+ expect(logger.stdoutCursor.write).not.toHaveBeenCalled();
+ expect(logger.stderrCursor.write).not.toHaveBeenCalled();
+ });
- spy.fg = jasmine.createSpyObj('fg', ['grey', 'yellow', 'blue',
'red']);
+ it('Test 011 : should log everything except error messages to
stdout', function () {
+ logger.setLevel('verbose');
+ DEFAULT_LEVELS.forEach(function (level) {
+ logger.log(level, 'message');
+ });
- return spy;
- }
+
expect(logger.stdoutCursor.write.calls.count()).toBe((DEFAULT_LEVELS.length -
1));
+ expect(logger.stderrCursor.write.calls.count()).toBe(1);
+ });
- beforeEach(function () {
- logger.stdoutCursor = new CursorSpy('stdoutCursor');
- logger.stderrCursor = new CursorSpy('stderrCursor');
- });
+ it('Test 012 : should log Error objects to stderr despite of
loglevel', function () {
+ logger.setLevel('verbose').log('verbose', new Error());
+ expect(logger.stdoutCursor.write).not.toHaveBeenCalled();
+ expect(logger.stderrCursor.write).toHaveBeenCalled();
+ });
+
+ it('Test 013 : should handle CordovaError instances separately
from Error ones', function () {
+ const errorMock = new CordovaError();
+ spyOn(errorMock,
'toString').and.returnValue('error_message');
- it('Test 010 : should ignore message if severity is less than
logger\'s level', function () {
- logger.setLevel('error').log('verbose', 'some_messgge');
- expect(logger.stdoutCursor.write).not.toHaveBeenCalled();
- expect(logger.stderrCursor.write).not.toHaveBeenCalled();
+ logger.setLevel('verbose').log('verbose', errorMock);
+ expect(errorMock.toString).toHaveBeenCalled();
+
expect(logger.stderrCursor.write.calls.argsFor(0)).toMatch('Error:
error_message');
+ });
});
+ } else {
+ describe('log method (ansi)', function () {
+ function CursorSpy (name) {
+ const cursorMethods = ['reset', 'write', 'bold'];
+ const spy = jasmine.createSpyObj(name, cursorMethods);
+
+ // Make spy methods chainable, as original Cursor acts
+ cursorMethods.forEach(function (method) {
spy[method].and.returnValue(spy); });
+
+ spy.fg = jasmine.createSpyObj('fg', ['grey', 'yellow',
'blue', 'red']);
+
+ return spy;
+ }
- it('Test 011 : should log everything except error messages to
stdout', function () {
- logger.setLevel('verbose');
- DEFAULT_LEVELS.forEach(function (level) {
- logger.log(level, 'message');
+ beforeEach(function () {
+ logger.stdoutCursor = new CursorSpy('stdoutCursor');
+ logger.stderrCursor = new CursorSpy('stderrCursor');
});
- // Multiply calls number to 2 because 'write' method is get
called twice (with message and EOL)
-
expect(logger.stdoutCursor.write.calls.count()).toBe((DEFAULT_LEVELS.length -
1) * 2);
- expect(logger.stderrCursor.write.calls.count()).toBe(1 * 2);
- });
+ it('Test 010 : should ignore message if severity is less than
logger\'s level', function () {
+ logger.setLevel('error').log('verbose', 'some_messgge');
+ expect(logger.stdoutCursor.write).not.toHaveBeenCalled();
+ expect(logger.stderrCursor.write).not.toHaveBeenCalled();
+ });
- it('Test 012 : should log Error objects to stderr despite of
loglevel', function () {
- logger.setLevel('verbose').log('verbose', new Error());
- expect(logger.stdoutCursor.write).not.toHaveBeenCalled();
- expect(logger.stderrCursor.write).toHaveBeenCalled();
- });
+ it('Test 011 : should log everything except error messages to
stdout', function () {
+ logger.setLevel('verbose');
+ DEFAULT_LEVELS.forEach(function (level) {
+ logger.log(level, 'message');
+ });
+
+ // Multiply calls number to 2 because 'write' method is
get called twice (with message and EOL)
+
expect(logger.stdoutCursor.write.calls.count()).toBe((DEFAULT_LEVELS.length -
1) * 2);
+ expect(logger.stderrCursor.write.calls.count()).toBe(1 *
2);
+ });
- it('Test 013 : should handle CordovaError instances separately
from Error ones', function () {
- const errorMock = new CordovaError();
- spyOn(errorMock, 'toString').and.returnValue('error_message');
+ it('Test 012 : should log Error objects to stderr despite of
loglevel', function () {
+ logger.setLevel('verbose').log('verbose', new Error());
+ expect(logger.stdoutCursor.write).not.toHaveBeenCalled();
+ expect(logger.stderrCursor.write).toHaveBeenCalled();
+ });
- logger.setLevel('verbose').log('verbose', errorMock);
- expect(errorMock.toString).toHaveBeenCalled();
-
expect(logger.stderrCursor.write.calls.argsFor(0)).toMatch('Error:
error_message');
+ it('Test 013 : should handle CordovaError instances separately
from Error ones', function () {
+ const errorMock = new CordovaError();
+ spyOn(errorMock,
'toString').and.returnValue('error_message');
+
+ logger.setLevel('verbose').log('verbose', errorMock);
+ expect(errorMock.toString).toHaveBeenCalled();
+
expect(logger.stderrCursor.write.calls.argsFor(0)).toMatch('Error:
error_message');
+ });
});
- });
+ }
describe('adjustLevel method', function () {
it('Test 014 : should properly adjust log level', function () {
diff --git a/src/CordovaLogger.js b/src/CordovaLogger.js
index 405d3be..06e63dd 100644
--- a/src/CordovaLogger.js
+++ b/src/CordovaLogger.js
@@ -17,11 +17,22 @@
under the License.
*/
-const ansi = require('ansi');
-const EventEmitter = require('events').EventEmitter;
-const EOL = require('os').EOL;
+const { EventEmitter } = require('node:events');
+const { EOL } = require('node:os');
+const util = require('node:util');
const formatError = require('./util/formatError');
+const hasStyleText = (function () {
+ try {
+ if (util.styleText(['bold', 'red'], 'test') !== '') {
+ return true;
+ }
+ } catch { }
+ return false;
+})();
+
+const ansi = !hasStyleText ? require('ansi') : null;
+
const INSTANCE_KEY = Symbol.for('org.apache.cordova.common.CordovaLogger');
/**
@@ -48,6 +59,11 @@ class CordovaLogger {
static get RESULTS () { return 'results'; }
+ /**
+ * @private This is just exposed temporarily for unit testing.
+ */
+ static get __hasStyleText () { return hasStyleText; }
+
/**
* Static method to create new or acquire existing instance.
*
@@ -62,6 +78,8 @@ class CordovaLogger {
return global[INSTANCE_KEY];
}
+ #subscriptions = new WeakSet();
+
constructor () {
/** @private */
this.levels = {};
@@ -72,10 +90,17 @@ class CordovaLogger {
/** @private */
this.stderr = process.stderr;
- /** @private */
- this.stdoutCursor = ansi(this.stdout);
- /** @private */
- this.stderrCursor = ansi(this.stderr);
+ if (hasStyleText) {
+ /** @private */
+ this.stdoutCursor = this.stdout;
+ /** @private */
+ this.stderrCursor = this.stderr;
+ } else {
+ /** @private */
+ this.stdoutCursor = ansi(this.stdout);
+ /** @private */
+ this.stderrCursor = ansi(this.stderr);
+ }
this.addLevel(CordovaLogger.VERBOSE, 1000, 'grey');
this.addLevel(CordovaLogger.NORMAL, 2000);
@@ -118,11 +143,17 @@ class CordovaLogger {
}
const color = this.colors[logLevel];
- if (color) {
- cursor.bold().fg[color]();
- }
- cursor.write(message).reset().write(EOL);
+ if (!hasStyleText) {
+ if (color) {
+ cursor.bold().fg[color]();
+ }
+
+ cursor.write(message).reset().write(EOL);
+ } else {
+ const styles = color ? ['bold', color] : [];
+ cursor.write(util.styleText(styles, message) + EOL);
+ }
return this;
}
@@ -213,6 +244,11 @@ class CordovaLogger {
throw new Error('Subscribe method only accepts an EventEmitter
instance as argument');
}
+ // Check to make sure we don't subscribe the same emitter multiple
times
+ if (this.#subscriptions.has(eventEmitter)) {
+ return this;
+ }
+
eventEmitter.on('verbose', this.verbose)
.on('log', this.normal)
.on('info', this.info)
@@ -221,6 +257,8 @@ class CordovaLogger {
// Set up event handlers for logging and results emitted as events.
.on('results', this.results);
+ this.#subscriptions.add(eventEmitter);
+
return this;
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
