Browse Source

Merge pull request #118 from ckeditor/t/114

Changed: Make EmitterMixin#_listeningTo and _emitterId private.
Piotrek Koszuliński 9 years ago
parent
commit
6c56c4110d

+ 4 - 15
packages/ckeditor5-utils/src/dom/emittermixin.js

@@ -8,6 +8,7 @@
  */
 
 import EmitterMixin from '../emittermixin';
+import { _getEmitterListenedTo, _setEmitterId } from '../emittermixin';
 import uid from '../uid';
 import extend from '../lib/lodash/extend';
 import isNative from '../lib/lodash/isNative';
@@ -109,19 +110,7 @@ const DomEmitterMixin = extend( {}, EmitterMixin, {
 	 * @return {module:utils/dom/emittermixin~ProxyEmitter} ProxyEmitter instance or null.
 	 */
 	_getProxyEmitter( node ) {
-		let proxy, emitters, emitterInfo;
-
-		// Get node UID. It allows finding Proxy Emitter for this DOM Node.
-		const uid = getNodeUID( node );
-
-		// Find existing Proxy Emitter for this DOM Node among emitters.
-		if ( ( emitters = this._listeningTo ) ) {
-			if ( ( emitterInfo = emitters[ uid ] ) ) {
-				proxy = emitterInfo.emitter;
-			}
-		}
-
-		return proxy || null;
+		return _getEmitterListenedTo( this, getNodeUID( node ) );
 	}
 } );
 
@@ -142,7 +131,7 @@ export default DomEmitterMixin;
  *     |     emitter: ProxyEmitter, |                +------------------------+                      +------------v----------+
  *     |     callbacks: {           |                | events: {              |                      | Node (HTMLElement)    |
  *     |       click: [ callbacks ] |                |   click: [ callbacks ] |                      +-----------------------+
- *     |     }                      |                | },                     |                      | data-cke-expando: UID |
+ *     |     }                      |                | },                     |                      | data-ck-expando: UID  |
  *     |   }                        |                | _domNode: Node,        |                      +-----------------------+
  *     | }                          |                | _domListeners: {},     |                                   |
  *     | +------------------------+ |                | _emitterId: UID        |                                   |
@@ -164,7 +153,7 @@ class ProxyEmitter {
 	 */
 	constructor( node ) {
 		// Set emitter ID to match DOM Node "expando" property.
-		this._emitterId = getNodeUID( node );
+		_setEmitterId( this, getNodeUID( node ) );
 
 		// Remember the DOM Node this ProxyEmitter is bound to.
 		this._domNode = node;

+ 56 - 7
packages/ckeditor5-utils/src/emittermixin.js

@@ -11,6 +11,9 @@ import EventInfo from './eventinfo';
 import uid from './uid';
 import priorities from './priorities';
 
+const _listeningTo = Symbol( 'listeningTo' );
+const _emitterId = Symbol( 'emitterId' );
+
 /**
  * Mixin that injects the events API into its host.
  *
@@ -150,14 +153,18 @@ const EmitterMixin = {
 		//     ...
 		// }
 
-		if ( !( emitters = this._listeningTo ) ) {
-			emitters = this._listeningTo = {};
+		if ( !this[ _listeningTo ] ) {
+			this[ _listeningTo ] = {};
 		}
 
-		if ( !( emitterId = emitter._emitterId ) ) {
-			emitterId = emitter._emitterId = uid();
+		emitters = this[ _listeningTo ];
+
+		if ( !_getEmitterId( emitter ) ) {
+			_setEmitterId( emitter );
 		}
 
+		emitterId = _getEmitterId( emitter );
+
 		if ( !( emitterInfo = emitters[ emitterId ] ) ) {
 			emitterInfo = emitters[ emitterId ] = {
 				emitter: emitter,
@@ -191,8 +198,8 @@ const EmitterMixin = {
 	 * `event`.
 	 */
 	stopListening( emitter, event, callback ) {
-		let emitters = this._listeningTo;
-		let emitterId = emitter && emitter._emitterId;
+		let emitters = this[ _listeningTo ];
+		let emitterId = emitter && _getEmitterId( emitter );
 		let emitterInfo = emitters && emitterId && emitters[ emitterId ];
 		let eventCallbacks = emitterInfo && event && emitterInfo.callbacks[ event ];
 
@@ -224,7 +231,7 @@ const EmitterMixin = {
 			for ( emitterId in emitters ) {
 				this.stopListening( emitters[ emitterId ].emitter );
 			}
-			delete this._listeningTo;
+			delete this[ _listeningTo ];
 		}
 	},
 
@@ -362,6 +369,48 @@ const EmitterMixin = {
 
 export default EmitterMixin;
 
+/**
+ * Checks if `listeningEmitter` listens to an emitter with given `listenedToEmitterId` and if so, returns that emitter.
+ * If not, returns `null`.
+ *
+ * @protected
+ * @param {module:utils/emittermixin~EmitterMixin} listeningEmitter Emitter that listens.
+ * @param {String} listenedToEmitterId Unique emitter id of emitter listened to.
+ * @returns {module:utils/emittermixin~EmitterMixin|null}
+ */
+export function _getEmitterListenedTo( listeningEmitter, listenedToEmitterId ) {
+	if ( listeningEmitter[ _listeningTo ] && listeningEmitter[ _listeningTo ][ listenedToEmitterId ] ) {
+		return listeningEmitter[ _listeningTo ][ listenedToEmitterId ].emitter;
+	}
+
+	return null;
+}
+
+/**
+ * Sets emitter's unique id.
+ *
+ * **Note:** `_emitterId` can be set only once.
+ *
+ * @protected
+ * @param {module:utils/emittermixin~EmitterMixin} emitter Emitter for which id will be set.
+ * @param {String} [id] Unique id to set. If not passed, random unique id will be set.
+ */
+export function _setEmitterId( emitter, id ) {
+	if ( !emitter[ _emitterId ] ) {
+		emitter[ _emitterId ] = id || uid();
+	}
+}
+
+/**
+ * Returns emitter's unique id.
+ *
+ * @protected
+ * @param {module:utils/emittermixin~EmitterMixin} emitter Emitter which id will be returned.
+ */
+export function _getEmitterId( emitter ) {
+	return emitter[ _emitterId ];
+}
+
 // Gets the internal `_events` property of the given object.
 // `_events` property store all lists with callbacks for registered event names.
 // If there were no events registered on the object, empty `_events` object is created.

+ 0 - 8
packages/ckeditor5-utils/tests/dom/emittermixin.js

@@ -184,8 +184,6 @@ describe( 'DomEmitterMixin', () => {
 			domEmitter.listenTo( node1, 'event1', spy1 );
 			domEmitter.listenTo( node2, 'event2', spy2 );
 
-			expect( domEmitter ).to.have.property( '_listeningTo' );
-
 			node1.dispatchEvent( new Event( 'event1' ) );
 			node2.dispatchEvent( new Event( 'event2' ) );
 
@@ -196,8 +194,6 @@ describe( 'DomEmitterMixin', () => {
 
 			sinon.assert.calledOnce( spy1 );
 			sinon.assert.calledOnce( spy2 );
-
-			expect( domEmitter ).to.not.have.property( '_listeningTo' );
 		} );
 
 		it( 'should stop listening to everything what left', () => {
@@ -214,8 +210,6 @@ describe( 'DomEmitterMixin', () => {
 			domEmitter.listenTo( node2, 'event1', spy3 );
 			domEmitter.listenTo( node2, 'event2', spy4 );
 
-			expect( domEmitter ).to.have.property( '_listeningTo' );
-
 			domEmitter.stopListening( node1, 'event1', spy1 );
 			domEmitter.stopListening( node2, 'event1' );
 
@@ -240,8 +234,6 @@ describe( 'DomEmitterMixin', () => {
 			sinon.assert.calledOnce( spy2 );
 			sinon.assert.notCalled( spy3 );
 			sinon.assert.calledOnce( spy4 );
-
-			expect( domEmitter ).to.not.have.property( '_listeningTo' );
 		} );
 
 		it( 'should not stop other nodes when a non-listened node is provided', () => {

+ 51 - 21
packages/ckeditor5-utils/tests/emittermixin.js

@@ -4,12 +4,16 @@
  */
 
 import EmitterMixin from 'ckeditor5-utils/src/emittermixin';
+import { _getEmitterListenedTo, _getEmitterId, _setEmitterId } from 'ckeditor5-utils/src/emittermixin';
 import EventInfo from 'ckeditor5-utils/src/eventinfo';
 
 describe( 'EmitterMixin', () => {
 	let emitter, listener;
 
-	beforeEach( refreshEmitter );
+	beforeEach( () => {
+		emitter = getEmitterInstance();
+		listener = getEmitterInstance();
+	} );
 
 	describe( 'fire', () => {
 		it( 'should execute callbacks in the right order without priority', () => {
@@ -376,8 +380,6 @@ describe( 'EmitterMixin', () => {
 	} );
 
 	describe( 'listenTo', () => {
-		beforeEach( refreshListener );
-
 		it( 'should properly register callbacks', () => {
 			let spy = sinon.spy();
 
@@ -408,8 +410,6 @@ describe( 'EmitterMixin', () => {
 	} );
 
 	describe( 'stopListening', () => {
-		beforeEach( refreshListener );
-
 		it( 'should stop listening to given event callback', () => {
 			let spy1 = sinon.spy();
 			let spy2 = sinon.spy();
@@ -480,8 +480,6 @@ describe( 'EmitterMixin', () => {
 			listener.listenTo( emitter1, 'event1', spy1 );
 			listener.listenTo( emitter2, 'event2', spy2 );
 
-			expect( listener ).to.have.property( '_listeningTo' );
-
 			emitter1.fire( 'event1' );
 			emitter2.fire( 'event2' );
 
@@ -492,8 +490,6 @@ describe( 'EmitterMixin', () => {
 
 			sinon.assert.calledOnce( spy1 );
 			sinon.assert.calledOnce( spy2 );
-
-			expect( listener ).to.not.have.property( '_listeningTo' );
 		} );
 
 		it( 'should not stop other emitters when a non-listened emitter is provided', () => {
@@ -1094,18 +1090,6 @@ describe( 'EmitterMixin', () => {
 		} );
 	} );
 
-	function refreshEmitter() {
-		emitter = getEmitterInstance();
-	}
-
-	function refreshListener() {
-		listener = getEmitterInstance();
-	}
-
-	function getEmitterInstance() {
-		return Object.create( EmitterMixin );
-	}
-
 	function assertDelegated( evtArgs, { expectedName, expectedSource, expectedPath, expectedData } ) {
 		const evtInfo = evtArgs[ 0 ];
 
@@ -1115,3 +1099,49 @@ describe( 'EmitterMixin', () => {
 		expect( evtArgs.slice( 1 ) ).to.deep.equal( expectedData );
 	}
 } );
+
+describe( 'emitter id', () => {
+	let emitter;
+
+	beforeEach( () => {
+		emitter = getEmitterInstance();
+	} );
+
+	it( 'should be undefined before it is set', () => {
+		expect( _getEmitterId( emitter ) ).to.be.undefined;
+	} );
+
+	it( 'should be settable but only once', () => {
+		_setEmitterId( emitter, 'abc' );
+
+		expect( _getEmitterId( emitter ) ).to.equal( 'abc' );
+
+		_setEmitterId( emitter, 'xyz' );
+
+		expect( _getEmitterId( emitter ) ).to.equal( 'abc' );
+	} );
+} );
+
+describe( '_getEmitterListenedTo', () => {
+	let emitter, listener;
+
+	beforeEach( () => {
+		emitter = getEmitterInstance();
+		listener = getEmitterInstance();
+	} );
+
+	it( 'should return null if listener do not listen to emitter with given id', () => {
+		expect( _getEmitterListenedTo( listener, 'abc' ) ).to.be.null;
+	} );
+
+	it( 'should return emitter with given id', () => {
+		listener.listenTo( emitter, 'eventName', () => {} );
+		const emitterId = _getEmitterId( emitter );
+
+		expect( _getEmitterListenedTo( listener, emitterId ) ).to.equal( emitter );
+	} );
+} );
+
+function getEmitterInstance() {
+	return Object.create( EmitterMixin );
+}