From 53f4874803796e82a7010090f35c6fde749c3cb7 Mon Sep 17 00:00:00 2001 From: Tim Buschtoens Date: Mon, 28 Aug 2017 19:37:33 +0200 Subject: [PATCH] Use internal console log functions The global console object can not reliably be assumed to be the wrapped console object with all the individual logging functions, it may also be the native console object that only has the "print" function. Let the Console object export individual log functions to use internally that are guaranteed to work at all times - even if the application code messes with the console object. In tests the functions simply redirect to the native console directly. Change-Id: I9c9b01a9e22a8a0a5ab4561f5843b6dad41032e9 --- src/tabris/Animation.js | 7 ++- src/tabris/CanvasContext.js | 3 +- src/tabris/Console.js | 18 +++++- src/tabris/Document.js | 5 +- src/tabris/Layout.js | 11 ++-- src/tabris/NativeObject.js | 11 ++-- src/tabris/Tabris.js | 9 +-- src/tabris/WebSocket.js | 3 +- src/tabris/property-types.js | 3 +- src/tabris/version.js | 8 +-- src/tabris/widgets/CollectionView.js | 7 ++- src/tabris/widgets/TabFolder.js | 3 +- test/tabris/Console.test.js | 83 +++++++++++++++++++++------- 13 files changed, 117 insertions(+), 54 deletions(-) diff --git a/src/tabris/Animation.js b/src/tabris/Animation.js index d7028a76d..f17eba6e0 100644 --- a/src/tabris/Animation.js +++ b/src/tabris/Animation.js @@ -1,4 +1,5 @@ import NativeObject from './NativeObject'; +import {warn} from './Console'; const ANIMATABLE_PROPERTIES = ['opacity', 'transform']; @@ -59,15 +60,15 @@ export function animate(properties, options) { this._encodeProperty(this._getTypeDef(property), properties[property]); this._storeProperty(property, animatedProps[property], options); } catch (ex) { - console.warn(this + ': Ignored invalid animation property value for "' + property + '"'); + warn(this + ': Ignored invalid animation property value for "' + property + '"'); } } else { - console.warn(this + ': Ignored invalid animation property "' + property + '"'); + warn(this + ': Ignored invalid animation property "' + property + '"'); } } for (let option in options) { if (!(option in PROPERTIES) && option !== 'name') { - console.warn(this + ': Ignored invalid animation option "' + option + '"'); + warn(this + ': Ignored invalid animation option "' + option + '"'); } } return new Promise((resolve, reject) => { diff --git a/src/tabris/CanvasContext.js b/src/tabris/CanvasContext.js index c478c58d2..839b8cd19 100644 --- a/src/tabris/CanvasContext.js +++ b/src/tabris/CanvasContext.js @@ -2,6 +2,7 @@ import {colorStringToArray, colorArrayToString} from './util-colors'; import {fontStringToObject, fontObjectToString} from './util-fonts'; import ImageData from './ImageData'; import GC from './GC'; +import {warn} from './Console'; export default class CanvasContext { @@ -293,7 +294,7 @@ function defineProperty(context, name) { context._gc.addOperation(name); prop.addOperations.call(context, context._state[name]); } catch (error) { - console.warn('Unsupported value for ' + name + ': ' + value); + warn('Unsupported value for ' + name + ': ' + value); } } }); diff --git a/src/tabris/Console.js b/src/tabris/Console.js index ce8130543..63eb1bf38 100644 --- a/src/tabris/Console.js +++ b/src/tabris/Console.js @@ -1,5 +1,22 @@ import {format} from './Formatter'; +const defaultConsole = global.console.print + ? createConsole(global.console) + : global.console; + +if (!defaultConsole.debug) { + // The native node console has no "debug" method + defaultConsole.debug = function(...args) { + defaultConsole.log(...args); + }; +} + +export const debug = function(...args) { defaultConsole.debug(...args); }; +export const info = function(...args) { defaultConsole.info(...args); }; +export const log = function(...args) { defaultConsole.log(...args); }; +export const warn = function(...args) { defaultConsole.warn(...args); }; +export const error = function(...args) { defaultConsole.error(...args); }; + export function createConsole(nativeConsole) { let console = {}; for (let name of ['debug', 'info', 'log', 'warn', 'error']) { @@ -9,4 +26,3 @@ export function createConsole(nativeConsole) { } return console; } - diff --git a/src/tabris/Document.js b/src/tabris/Document.js index 0725d10ba..4dab84f18 100644 --- a/src/tabris/Document.js +++ b/src/tabris/Document.js @@ -1,4 +1,5 @@ import Event, {addDOMEventTargetMethods} from './Event'; +import {log, error} from './Console'; export function addDOMDocument(target) { @@ -62,8 +63,8 @@ function handleElementInserted(parent, child, target) { try { result = tabris._client.loadAndExecute(child.src, '', ''); } catch (ex) { - console.error('Error loading ' + child.src + ':', ex); - console.log(ex.stack); + error('Error loading ' + child.src + ':', ex); + log(ex.stack); if (typeof child.onerror === 'function') { child.onerror.call(target, ex); } diff --git a/src/tabris/Layout.js b/src/tabris/Layout.js index d8d26673b..2b405ef43 100644 --- a/src/tabris/Layout.js +++ b/src/tabris/Layout.js @@ -1,5 +1,6 @@ import {omit} from './util'; import {types} from './property-types'; +import {warn} from './Console'; export default { @@ -7,27 +8,27 @@ export default { let result = layoutData; if ('centerX' in result) { if (('left' in result) || ('right' in result)) { - console.warn('Inconsistent layoutData: centerX overrides left and right'); + warn('Inconsistent layoutData: centerX overrides left and right'); result = omit(result, ['left', 'right']); } } if ('baseline' in result) { if (('top' in result) || ('bottom' in result) || ('centerY' in result)) { - console.warn('Inconsistent layoutData: baseline overrides top, bottom, and centerY'); + warn('Inconsistent layoutData: baseline overrides top, bottom, and centerY'); result = omit(result, ['top', 'bottom', 'centerY']); } } else if ('centerY' in result) { if (('top' in result) || ('bottom' in result)) { - console.warn('Inconsistent layoutData: centerY overrides top and bottom'); + warn('Inconsistent layoutData: centerY overrides top and bottom'); result = omit(result, ['top', 'bottom']); } } if ('left' in result && 'right' in result && 'width' in result) { - console.warn('Inconsistent layoutData: left and right are set, ignore width'); + warn('Inconsistent layoutData: left and right are set, ignore width'); result = omit(result, ['width']); } if ('top' in result && 'bottom' in result && 'height' in result) { - console.warn('Inconsistent layoutData: top and bottom are set, ignore height'); + warn('Inconsistent layoutData: top and bottom are set, ignore height'); result = omit(result, ['height']); } return result; diff --git a/src/tabris/NativeObject.js b/src/tabris/NativeObject.js index 0abbbee83..8f38b5cc0 100644 --- a/src/tabris/NativeObject.js +++ b/src/tabris/NativeObject.js @@ -1,4 +1,5 @@ import {types} from './property-types'; +import {warn} from './Console'; import EventObject from './EventObject'; import Events from './Events'; @@ -63,7 +64,7 @@ export default class NativeObject extends EventsClass { $getProperty(name) { if (this._isDisposed) { - console.warn('Cannot get property "' + name + '" on disposed object'); + warn('Cannot get property "' + name + '" on disposed object'); return; } let getter = this.$getPropertyGetter(name) || this._getStoredProperty; @@ -73,7 +74,7 @@ export default class NativeObject extends EventsClass { $setProperty(name, value) { if (this._isDisposed) { - console.warn('Cannot set property "' + name + '" on disposed object'); + warn('Cannot set property "' + name + '" on disposed object'); return; } let typeDef = this._getTypeDef(name); @@ -81,7 +82,7 @@ export default class NativeObject extends EventsClass { try { encodedValue = this._encodeProperty(typeDef, value); } catch (ex) { - console.warn(this + ': Ignored unsupported value for property "' + name + '": ' + ex.message); + warn(this + ': Ignored unsupported value for property "' + name + '": ' + ex.message); return; } let setter = this.$getPropertySetter(name) || this._storeProperty; @@ -236,7 +237,7 @@ function setExistingProperty(name, value) { if (name in this) { this[name] = value; } else { - console.warn('Unknown property "' + name + '"'); + warn('Unknown property "' + name + '"'); } } @@ -281,7 +282,7 @@ function wrapCoder(fn, args) { } function readOnlySetter(name) { - console.warn(`Can not set read-only property "${name}"`); + warn(`Can not set read-only property "${name}"`); } function defaultSetter(name, value) { diff --git a/src/tabris/Tabris.js b/src/tabris/Tabris.js index 7a9090673..5e1de87a5 100644 --- a/src/tabris/Tabris.js +++ b/src/tabris/Tabris.js @@ -1,6 +1,7 @@ import Events from './Events'; import NativeBridge from './NativeBridge'; import ProxyStore from './ProxyStore'; +import {error, log} from './Console'; export default class Tabris { @@ -38,14 +39,14 @@ export default class Tabris { try { returnValue = proxy._trigger(event, param); } catch (error) { - console.error(error); - console.log(error.stack); + error(error); + log(error.stack); } } this.trigger('flush'); } catch (ex) { - console.error(ex); - console.log(ex.stack); + error(ex); + log(ex.stack); } return returnValue; } diff --git a/src/tabris/WebSocket.js b/src/tabris/WebSocket.js index 669414fc6..d5424db84 100644 --- a/src/tabris/WebSocket.js +++ b/src/tabris/WebSocket.js @@ -1,6 +1,7 @@ import NativeObject from './NativeObject'; import Event, {addDOMEventTargetMethods, defineEventHandlerProperties} from './Event'; import {omit} from './util'; +import {warn} from './Console'; const CONNECTING = 0; const OPEN = 1; @@ -85,7 +86,7 @@ export default class WebSocket { } set bufferedAmount(bufferedAmount) { - console.warn('Can not set read-only property "bufferedAmount"'); + warn('Can not set read-only property "bufferedAmount"'); } get bufferedAmount() { diff --git a/src/tabris/property-types.js b/src/tabris/property-types.js index 122c42a72..310cb8bb1 100644 --- a/src/tabris/property-types.js +++ b/src/tabris/property-types.js @@ -4,6 +4,7 @@ import {colorArrayToString, colorStringToArray} from './util-colors'; import {fontObjectToString, fontStringToObject} from './util-fonts'; import NativeObject from './NativeObject'; import WidgetCollection from './WidgetCollection'; +import {warn} from './Console'; export let types = { @@ -128,7 +129,7 @@ export let types = { } }); if ('scale' in value && ('width' in value || 'height' in value)) { - console.warn('Image scale is ignored if width or height are given'); + warn('Image scale is ignored if width or height are given'); } return imageToArray(value); }, diff --git a/src/tabris/version.js b/src/tabris/version.js index e0e548cd1..999d31128 100644 --- a/src/tabris/version.js +++ b/src/tabris/version.js @@ -1,3 +1,4 @@ +import {error} from './Console'; export function checkVersion(tabrisVersionString, clientVersionString) { if (!clientVersionString) { @@ -6,12 +7,7 @@ export function checkVersion(tabrisVersionString, clientVersionString) { let tabrisVersion = tabrisVersionString.split('.'); let clientVersion = clientVersionString.split('.'); if (tabrisVersion[0] !== clientVersion[0] || tabrisVersion[1] !== clientVersion[1]) { - printError(`Version mismatch: JavaScript module "tabris" (version ${tabrisVersionString}) ` + + error(`Version mismatch: JavaScript module "tabris" (version ${tabrisVersionString}) ` + `is incompatible with the native tabris platform (version ${clientVersionString}).`); } } - -// At this point the wrapped console object may not be available -function printError(msg) { - console.print ? console.print('error', msg) : console.error(msg); -} diff --git a/src/tabris/widgets/CollectionView.js b/src/tabris/widgets/CollectionView.js index 875ad7dd3..1043a9fcd 100644 --- a/src/tabris/widgets/CollectionView.js +++ b/src/tabris/widgets/CollectionView.js @@ -1,6 +1,7 @@ import NativeObject from '../NativeObject'; import Widget from '../Widget'; import Composite from './Composite'; +import {warn} from '../Console'; const EVENT_TYPES = ['refresh', 'select', 'scroll']; @@ -130,8 +131,8 @@ export default class CollectionView extends Composite { } cell._parent = this; this._addChild(cell); - cell._setParent = () => console.warn('Cannot re-parent collection view cell'); - cell.dispose = () => console.warn('Cannot dispose of collection view cell'); + cell._setParent = () => warn('Cannot re-parent collection view cell'); + cell.dispose = () => warn('Cannot dispose of collection view cell'); return cell; } @@ -241,7 +242,7 @@ function encodeCellHeight(value) { if (isNumber(value)) { return Math.max(-1, value); } - console.warn('Invalid cell height: ' + value); + warn('Invalid cell height: ' + value); } let triggerChangeFirstVisibleIndex = createDelegate('firstVisibleIndex'); diff --git a/src/tabris/widgets/TabFolder.js b/src/tabris/widgets/TabFolder.js index f6e474983..fc40e20e7 100644 --- a/src/tabris/widgets/TabFolder.js +++ b/src/tabris/widgets/TabFolder.js @@ -1,6 +1,7 @@ import NativeObject from '../NativeObject'; import Composite from './Composite'; import Tab from './Tab'; +import {warn} from '../Console'; const EVENT_TYPES = ['select', 'scroll']; @@ -49,7 +50,7 @@ NativeObject.defineProperties(TabFolder.prototype, { selection: { set(name, tab) { if (this._children.indexOf(tab) < 0) { - console.warn('Can not set TabFolder selection to ' + tab); + warn('Can not set TabFolder selection to ' + tab); return; } this._nativeSet('selection', tab.cid); diff --git a/test/tabris/Console.test.js b/test/tabris/Console.test.js index 8209d76e2..fe50b4680 100644 --- a/test/tabris/Console.test.js +++ b/test/tabris/Console.test.js @@ -1,36 +1,77 @@ -import {expect, spy, restore} from '../test'; +import {expect, spy, stub, restore} from '../test'; import {createConsole} from '../../src/tabris/Console'; +import * as defaultConsole from '../../src/tabris/Console'; -describe('Console', function() { - - let console, nativeConsole; +const methods = ['debug', 'log', 'info', 'warn', 'error']; +const realConsole = console; - beforeEach(function() { - nativeConsole = { - print: spy() - }; - console = createConsole(nativeConsole); - }); +describe('Console', function() { afterEach(restore); - ['debug', 'log', 'info', 'warn', 'error'].forEach(method => describe(method, function() { + describe('default', function() { + + let globalConsole = {}; - it('succeeds without arguments', function() { - console[method](); - expect(nativeConsole.print).to.have.been.calledWithMatch(method, ''); + beforeEach(function() { + globalConsole = {}; + methods.forEach(method => { + globalConsole[method] = stub(); + spy(realConsole, method); + }); + global.console = globalConsole; }); - it('concatenates multiple arguments', function() { - console[method]('foo', 23); - expect(nativeConsole.print).to.have.been.calledWithMatch(method, 'foo 23'); + afterEach(function() { + global.console = realConsole; }); - it('replaces placeholders with arguments', function() { - console[method]('foo %dk', 23); - expect(nativeConsole.print).to.have.been.calledWithMatch(method, 'foo 23k'); + methods.forEach(method => describe(method, function() { + + it('forwards call to real console', function() { + defaultConsole[method](1, 2, 3); + expect(realConsole[method]).to.have.been.calledWith(1, 2, 3); + }); + + it('does not call current global console', function() { + defaultConsole[method](1, 2, 3); + expect(globalConsole[method]).not.to.have.been.calledWith(1, 2, 3); + }); + + })); + + }); + + describe('new instance', function() { + + let console, nativeConsole; + + beforeEach(function() { + nativeConsole = { + print: spy() + }; + console = createConsole(nativeConsole); }); - })); + ['debug', 'log', 'info', 'warn', 'error'].forEach(method => describe(method, function() { + + it('succeeds without arguments', function() { + console[method](); + expect(nativeConsole.print).to.have.been.calledWithMatch(method, ''); + }); + + it('concatenates multiple arguments', function() { + console[method]('foo', 23); + expect(nativeConsole.print).to.have.been.calledWithMatch(method, 'foo 23'); + }); + + it('replaces placeholders with arguments', function() { + console[method]('foo %dk', 23); + expect(nativeConsole.print).to.have.been.calledWithMatch(method, 'foo 23k'); + }); + + })); + + }); });