Browse Source

Utils.mix() should not override existing properties.

Piotrek Koszuliński 10 years ago
parent
commit
8382bc3016
2 changed files with 38 additions and 2 deletions
  1. 7 1
      packages/ckeditor5-engine/src/utils.js
  2. 31 1
      packages/ckeditor5-engine/tests/utils.js

+ 7 - 1
packages/ckeditor5-engine/src/utils.js

@@ -125,7 +125,9 @@ const utils = {
 	 *
 	 *		utils.mix( Editor, SomeMixin, ... );
 	 *
-	 *		new Editor().a(); -> 'a'
+	 *		new Editor().a(); // -> 'a'
+	 *
+	 * Note: Properties which already exist in the base class will not be overriden.
 	 *
 	 * @param {Function} [baseClass] Class which prototype will be extended.
 	 * @param {Object} [...mixins] Objects from which to get properties.
@@ -134,6 +136,10 @@ const utils = {
 		mixins.forEach( ( mixin ) => {
 			Object.getOwnPropertyNames( mixin ).concat( Object.getOwnPropertySymbols( mixin ) )
 				.forEach( ( key ) => {
+					if ( key in baseClass.prototype ) {
+						return;
+					}
+
 					const sourceDescriptor = Object.getOwnPropertyDescriptor( mixin, key );
 					sourceDescriptor.enumerable = false;
 

+ 31 - 1
packages/ckeditor5-engine/tests/utils.js

@@ -156,7 +156,7 @@ describe( 'utils', () => {
 			}
 		};
 
-		it( 'mixes 2nd+ param\'s properties into the first class', () => {
+		it( 'mixes 2nd+ argument\'s properties into the first class', () => {
 			class Foo {}
 			utils.mix( Foo, MixinA, MixinB );
 
@@ -259,5 +259,35 @@ describe( 'utils', () => {
 
 			expect( foo[ symbolA ]() ).to.equal( 'a' );
 		} );
+
+		it( 'does not copy already existing properties', () => {
+			class Foo {
+				a() {
+					return 'foo';
+				}
+			}
+			utils.mix( Foo, MixinA, MixinB );
+
+			const foo = new Foo();
+
+			expect( foo.a() ).to.equal( 'foo' );
+			expect( foo.b() ).to.equal( 'b' );
+		} );
+
+		it( 'does not copy already existing properties - properties deep in the proto chain', () => {
+			class Foo {
+				a() {
+					return 'foo';
+				}
+			}
+			class Bar extends Foo {}
+
+			utils.mix( Bar, MixinA, MixinB );
+
+			const bar = new Bar();
+
+			expect( bar.a() ).to.equal( 'foo' );
+			expect( bar.b() ).to.equal( 'b' );
+		} );
 	} );
 } );