r89913 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r89912‎ | r89913 | r89914 >
Date:05:13, 12 June 2011
Author:neilk
Status:resolved (Comments)
Tags:
Comment:
readiness events, that can occur only once, and can be subscribed to even if already occured
Modified paths:
  • /trunk/extensions/UploadWizard/resources/jquery/jquery.pubsub.js (modified) (history)

Diff [purge]

Index: trunk/extensions/UploadWizard/resources/jquery/jquery.pubsub.js
@@ -2,6 +2,9 @@
33 * Minimal pubsub framework
44 *
55 * Loosely based on https://github.com/phiggins42/bloody-jquery-plugins/pubsub.js, which is itself BSD-licensed.
 6+ * Concept of 'ready' events is new, though.
 7+ *
 8+ * @author Neil Kandalgaonkar <neilk@wikimedia.org>
69 */
710
811 ( function( $ ) {
@@ -9,6 +12,11 @@
1013 * Store of events -> array of listener callbacks
1114 */
1215 var subs = {};
 16+
 17+ /**
 18+ * Store of ready events, as object of event name -> argument array
 19+ */
 20+ var ready = {};
1321
1422 /**
1523 * Publish an event
@@ -18,27 +26,60 @@
1927 */
2028 $.publish = function( name /* , args... */ ) {
2129 var args = [].slice.call( arguments, 1 );
22 - $.each( subs[name], function( i, sub ) {
23 - sub.apply( null, args );
24 - } );
25 - return subs[name].length;
 30+ if ( typeof subs[name] !== 'undefined' && subs[name] instanceof Array ) {
 31+ $.each( subs[name], function( i, sub ) {
 32+ sub.apply( null, args );
 33+ } );
 34+ return subs[name].length;
 35+ }
 36+ return 0;
2637 };
2738
2839 /**
 40+ * Publish a ready event. Ready events occur once only, so
 41+ * subscribers will be called even if they subscribe later.
 42+ * Additional variadic arguments after the event name are passed as arguments to the subscriber functions
 43+ * @param {String} name of event
 44+ * @return {Number} number of subscribers
 45+ */
 46+ $.publishReady = function( name /*, args... */ ) {
 47+ if ( typeof ready[name] === 'undefined' ) {
 48+ var args = [].slice.call( arguments, 1 );
 49+ ready[name] = args;
 50+ $.publish.apply( null, arguments );
 51+ }
 52+ };
 53+
 54+ /**
2955 * Subscribe to an event.
3056 * @param {String} name of event to listen for
3157 * @param {Function} callback to run when event occurs
3258 * @return {Array} returns handle which can be used as argument to unsubscribe()
3359 */
3460 $.subscribe = function( name, fn ) {
35 - if (!subs[name]) {
 61+ if ( typeof subs[name] === 'undefined' ) {
3662 subs[name] = [];
3763 }
38 - subs[name].push(fn);
 64+ subs[name].push( fn );
3965 return [ name, fn ];
4066 };
4167
4268 /**
 69+ * Subscribe to a ready event. See publishReady().
 70+ * Subscribers will be called even if they subscribe long after the event fired.
 71+ * @param {String} name of event to listen for
 72+ * @param {Function} callback to run now (if event already occurred) or when event occurs
 73+ * @return {Array} returns handle which can be used as argument to unsubscribe()
 74+ */
 75+ $.subscribeReady = function( name, fn ) {
 76+ if ( ready[name] ) {
 77+ fn.apply( null, ready[name] );
 78+ } else {
 79+ $.subscribe( name, fn );
 80+ }
 81+ };
 82+
 83+ /**
4384 * Given the handle of a particular subscription, remove it
4485 * @param {Array} object returned by subscribe ( array of event name and callback )
4586 * @return {Boolean} success

Follow-up revisions

RevisionCommit summaryAuthorDate
r89987prevent subscriptions and readiness events from hanging around foreverneilk18:15, 13 June 2011
r89999renamed "timeline" test cases, which are now called "ready" events. Added tes...neilk19:20, 13 June 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   17:36, 13 June 2011

This seems like it'll leak objects into the 'ready' array, never to be removed. Probably ok for this usage for now since nobody'll sit on UploadWizard forever, but won't be suitable for longer-running web app views that might be used in future.

Needs test cases.

#Comment by NeilK (talk | contribs)   19:21, 13 June 2011

fixed the leaks in r89987

it did have tests, but they referred to old method names -- fixed in r89999

also added tests for new purge functions introduced in r89987

#Comment by Brion VIBBER (talk | contribs)   21:25, 13 June 2011

New tests now work, resolving.

Status & tagging log