From dc0f9b3a3a75e80a0c5f09dd76b4df1fcc05080d Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sat, 17 Sep 2016 15:50:47 -0700 Subject: [PATCH] resourceloader: Deprecate mw.Map This has never been used outside mediawiki.js for things like mw.config, and mw.messages. The public API of those objects will remain, but the constructor will no longer be supported in the future in order to allow these to be backed by native Map in the future (with a minimal polyfill rather than a complete one). * Remove 'mw.Map(valuesObject)'. (follows-up 63f8d7b9e5). This is incompatible with native Map and isn't actually used anywhere. * Deprecate mw.Map#values. Undocumented and considered private. A few uses have appeared in the wild so keep a short deprecation period. * Update the MediaWiki QUnit runner to construct new maps in setup/teardown instead of swapping the internal values object directly. Also update the runner to update mediawiki.jqueryMsg's parser default since it kept a reference to the first mw.messages object. * Add 'logName' parameter to mw.log.deprecate(). Default logName of property key made sense for its original use case (deprecation warnings for global variables, which are a property of the window object). But for keys of any other object, we'll want to some context for the name. * Remove redundant test expect numbers. (Follows-up 7c36375, c4c7007d) * Ensure mw.Map.prototype.constructor points to mw.Map. This was broken by 5819c37b3, which accidentally made it point to Object. Bug: T146432 Change-Id: I7ff0b230608f5ca582fc6752b59c4bb998c78ba2 --- maintenance/jsduck/categories.json | 1 - resources/src/mediawiki/mediawiki.js | 88 ++++++++----------- resources/src/mediawiki/mediawiki.user.js | 2 +- tests/qunit/data/testrunner.js | 28 ++++-- .../resources/mediawiki/mediawiki.test.js | 14 ++- .../mediawiki/mediawiki.user.test.js | 2 +- 6 files changed, 62 insertions(+), 73 deletions(-) diff --git a/maintenance/jsduck/categories.json b/maintenance/jsduck/categories.json index 068ee8c960..aad85da4a4 100644 --- a/maintenance/jsduck/categories.json +++ b/maintenance/jsduck/categories.json @@ -6,7 +6,6 @@ "name": "Base", "classes": [ "mw", - "mw.Map", "mw.Message", "mw.loader", "mw.loader.store", diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js index 484930a007..6280c95eaa 100644 --- a/resources/src/mediawiki/mediawiki.js +++ b/resources/src/mediawiki/mediawiki.js @@ -66,48 +66,20 @@ }() ); /** - * Create an object that can be read from or written to from methods that allow + * Create an object that can be read from or written to via methods that allow * interaction both with single and multiple properties at once. * - * @example - * - * var collection, query, results; - * - * // Create your address book - * collection = new mw.Map(); - * - * // This data could be coming from an external source (eg. API/AJAX) - * collection.set( { - * 'John Doe': 'john@example.org', - * 'Jane Doe': 'jane@example.org', - * 'George van Halen': 'gvanhalen@example.org' - * } ); - * - * wanted = ['John Doe', 'Jane Doe', 'Daniel Jackson']; - * - * // You can detect missing keys first - * if ( !collection.exists( wanted ) ) { - * // One or more are missing (in this case: "Daniel Jackson") - * mw.log( 'One or more names were not found in your address book' ); - * } - * - * // Or just let it give you what it can. Optionally fill in from a default. - * results = collection.get( wanted, 'nobody@example.com' ); - * mw.log( results['Jane Doe'] ); // "jane@example.org" - * mw.log( results['Daniel Jackson'] ); // "nobody@example.com" - * + * @private * @class mw.Map * * @constructor - * @param {Object|boolean} [values] The value-baring object to be mapped. Defaults to an - * empty object. - * For backwards-compatibility with mw.config, this can also be `true` in which case values - * are copied to the Window object as global variables (T72470). Values are copied in - * one direction only. Changes to globals are not reflected in the map. + * @param {boolean} [global=false] Whether to synchronise =values to the global + * window object (for backwards-compatibility with mw.config; T72470). Values are + * copied in one direction only. Changes to globals do not reflect in the map. */ - function Map( values ) { - if ( values === true ) { - this.values = {}; + function Map( global ) { + this.internalValues = {}; + if ( global === true ) { // Override #set to also set the global variable this.set = function ( selection, value ) { @@ -125,11 +97,16 @@ } return false; }; - - return; } - this.values = values || {}; + // Deprecated since MediaWiki 1.28 + log.deprecate( + this, + 'values', + this.internalValues, + 'mw.Map#values is deprecated. Use mw.Map#get() instead.', + 'Map-values' + ); } /** @@ -142,8 +119,8 @@ * @param {Mixed} value */ function setGlobalMapValue( map, key, value ) { - map.values[ key ] = value; - mw.log.deprecate( + map.internalValues[ key ] = value; + log.deprecate( window, key, value, @@ -153,6 +130,8 @@ } Map.prototype = { + constructor: Map, + /** * Get the value of one or more keys. * @@ -162,7 +141,7 @@ * @param {Mixed} [fallback=null] Value for keys that don't exist. * @return {Mixed|Object| null} If selection was a string, returns the value, * If selection was an array, returns an object of key/values. - * If no selection is passed, the 'values' container is returned. (Beware that, + * If no selection is passed, the internal container is returned. (Beware that, * as is the default in JavaScript, the object is returned by reference.) */ get: function ( selection, fallback ) { @@ -181,14 +160,14 @@ } if ( typeof selection === 'string' ) { - if ( !hasOwn.call( this.values, selection ) ) { + if ( !hasOwn.call( this.internalValues, selection ) ) { return fallback; } - return this.values[ selection ]; + return this.internalValues[ selection ]; } if ( selection === undefined ) { - return this.values; + return this.internalValues; } // Invalid selection key @@ -207,12 +186,12 @@ if ( $.isPlainObject( selection ) ) { for ( s in selection ) { - this.values[ s ] = selection[ s ]; + this.internalValues[ s ] = selection[ s ]; } return true; } if ( typeof selection === 'string' && arguments.length > 1 ) { - this.values[ selection ] = value; + this.internalValues[ selection ] = value; return true; } return false; @@ -229,13 +208,13 @@ if ( $.isArray( selection ) ) { for ( s = 0; s < selection.length; s++ ) { - if ( typeof selection[ s ] !== 'string' || !hasOwn.call( this.values, selection[ s ] ) ) { + if ( typeof selection[ s ] !== 'string' || !hasOwn.call( this.internalValues, selection[ s ] ) ) { return false; } } return true; } - return typeof selection === 'string' && hasOwn.call( this.values, selection ); + return typeof selection === 'string' && hasOwn.call( this.internalValues, selection ); } }; @@ -475,11 +454,14 @@ * @param {string} key Name of property to create in `obj` * @param {Mixed} val The value this property should return when accessed * @param {string} [msg] Optional text to include in the deprecation message + * @param {string} [logName=key] Optional custom name for the feature. + * This is used instead of `key` in the message and `mw.deprecate` tracking. */ log.deprecate = !Object.defineProperty ? function ( obj, key, val ) { obj[ key ] = val; - } : function ( obj, key, val, msg ) { - msg = 'Use of "' + key + '" is deprecated.' + ( msg ? ( ' ' + msg ) : '' ); + } : function ( obj, key, val, msg, logName ) { + logName = logName || key; + msg = 'Use of "' + logName + '" is deprecated.' + ( msg ? ( ' ' + msg ) : '' ); var logged = new StringSet(); function uniqueTrace() { var trace = new Error().stack; @@ -498,14 +480,14 @@ enumerable: true, get: function () { if ( uniqueTrace() ) { - mw.track( 'mw.deprecate', key ); + mw.track( 'mw.deprecate', logName ); mw.log.warn( msg ); } return val; }, set: function ( newVal ) { if ( uniqueTrace() ) { - mw.track( 'mw.deprecate', key ); + mw.track( 'mw.deprecate', logName ); mw.log.warn( msg ); } val = newVal; diff --git a/resources/src/mediawiki/mediawiki.user.js b/resources/src/mediawiki/mediawiki.user.js index 52a1efb42b..63e7de87ef 100644 --- a/resources/src/mediawiki/mediawiki.user.js +++ b/resources/src/mediawiki/mediawiki.user.js @@ -89,7 +89,7 @@ * @return {number} Current user's id, or 0 if user is anonymous */ getId: function () { - return mw.config.get( 'wgUserId', 0 ); + return mw.config.get( 'wgUserId' ) || 0; }, /** diff --git a/tests/qunit/data/testrunner.js b/tests/qunit/data/testrunner.js index 79f37dca44..b79c1927f3 100644 --- a/tests/qunit/data/testrunner.js +++ b/tests/qunit/data/testrunner.js @@ -167,10 +167,11 @@ */ QUnit.newMwEnvironment = ( function () { var warn, error, liveConfig, liveMessages, + MwMap = mw.config.constructor, // internal use only ajaxRequests = []; - liveConfig = mw.config.values; - liveMessages = mw.messages.values; + liveConfig = mw.config; + liveMessages = mw.messages; function suppressWarnings() { warn = mw.log.warn; @@ -198,14 +199,14 @@ // NOTE: It is important that we suppress warnings because extend() will also access // deprecated properties and trigger deprecation warnings from mw.log#deprecate. suppressWarnings(); - copy = $.extend( {}, liveConfig, custom ); + copy = $.extend( {}, liveConfig.get(), custom ); restoreWarnings(); return copy; } function freshMessagesCopy( custom ) { - return $.extend( /*deep=*/true, {}, liveMessages, custom ); + return $.extend( /*deep=*/true, {}, liveMessages.get(), custom ); } /** @@ -231,8 +232,15 @@ setup: function () { // Greetings, mock environment! - mw.config.values = freshConfigCopy( localEnv.config ); - mw.messages.values = freshMessagesCopy( localEnv.messages ); + mw.config = new MwMap(); + mw.config.set( freshConfigCopy( localEnv.config ) ); + mw.messages = new MwMap(); + mw.messages.set( freshMessagesCopy( localEnv.messages ) ); + // Update reference to mw.messages + mw.jqueryMsg.setParserDefaults( { + messages: mw.messages + } ); + this.suppressWarnings = suppressWarnings; this.restoreWarnings = restoreWarnings; @@ -251,8 +259,12 @@ $( document ).off( 'ajaxSend', trackAjax ); // Farewell, mock environment! - mw.config.values = liveConfig; - mw.messages.values = liveMessages; + mw.config = liveConfig; + mw.messages = liveMessages; + // Restore reference to mw.messages + mw.jqueryMsg.setParserDefaults( { + messages: liveMessages + } ); // As a convenience feature, automatically restore warnings if they're // still suppressed by the end of the test. diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js index ab463a9720..1518a80cad 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js @@ -65,10 +65,11 @@ ); } ); - QUnit.test( 'mw.Map', 35, function ( assert ) { + QUnit.test( 'mw.Map', function ( assert ) { var arry, conf, funky, globalConf, nummy, someValues; conf = new mw.Map(); + // Dummy variables funky = function () {}; arry = []; @@ -126,15 +127,15 @@ lorem: 'ipsum' }, 'Map.get returns multiple values correctly as an object' ); - assert.deepEqual( conf, new mw.Map( conf.values ), 'new mw.Map maps over existing values-bearing object' ); - assert.deepEqual( conf.get( [ 'foo', 'notExist' ] ), { foo: 'bar', notExist: null }, 'Map.get return includes keys that were not found as null values' ); // Interacting with globals and accessing the values object + this.suppressWarnings(); assert.strictEqual( conf.get(), conf.values, 'Map.get returns the entire values object by reference (if called without arguments)' ); + this.restoreWarnings(); conf.set( 'globalMapChecker', 'Hi' ); @@ -170,11 +171,7 @@ } } ); - QUnit.test( 'mw.config', 1, function ( assert ) { - assert.ok( mw.config instanceof mw.Map, 'mw.config instance of mw.Map' ); - } ); - - QUnit.test( 'mw.message & mw.messages', 100, function ( assert ) { + QUnit.test( 'mw.message & mw.messages', function ( assert ) { var goodbye, hello; // Convenience method for asserting the same result for multiple formats @@ -189,7 +186,6 @@ } assert.ok( mw.messages, 'messages defined' ); - assert.ok( mw.messages instanceof mw.Map, 'mw.messages instance of mw.Map' ); assert.ok( mw.messages.set( 'hello', 'Hello awesome world' ), 'mw.messages.set: Register' ); hello = mw.message( 'hello' ); diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js index 5122dcdc4f..7f6efa0c77 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js @@ -22,7 +22,7 @@ QUnit.test( 'getters (anonymous)', function ( assert ) { // Forge an anonymous user mw.config.set( 'wgUserName', null ); - delete mw.config.values.wgUserId; + mw.config.set( 'wgUserId', null ); assert.strictEqual( mw.user.getName(), null, 'getName()' ); assert.strictEqual( mw.user.isAnon(), true, 'isAnon()' ); -- 2.20.1