浏览代码

Fixed: Template normalization modifies original definition.

Aleksander Nowodzinski 9 年之前
父节点
当前提交
e890a6bf71
共有 2 个文件被更改,包括 90 次插入9 次删除
  1. 42 8
      packages/ckeditor5-ui/src/template.js
  2. 48 1
      packages/ckeditor5-ui/tests/template.js

+ 42 - 8
packages/ckeditor5-ui/src/template.js

@@ -10,6 +10,7 @@
 import CKEditorError from '../utils/ckeditorerror.js';
 import mix from '../utils/mix.js';
 import EmitterMixin from '/ckeditor5/utils/emittermixin.js';
+import cloneDeepWith from '../utils/lib/lodash/clonedeepwith.js';
 
 const bindToSymbol = Symbol( 'bindTo' );
 const bindIfSymbol = Symbol( 'bindIf' );
@@ -28,7 +29,9 @@ export default class Template {
 	 * @param {ui.TemplateDefinition} def The definition of the template.
 	 */
 	constructor( def ) {
-		normalize( def );
+		const defClone = clone( def );
+
+		normalize( defClone );
 
 		/**
 		 * Definition of this template.
@@ -36,7 +39,7 @@ export default class Template {
 		 * @readonly
 		 * @member {ui.TemplateDefinition} ui.Template#definition
 		 */
-		this.definition = def;
+		this.definition = defClone;
 	}
 
 	/**
@@ -479,12 +482,23 @@ Template.bind = ( observable, emitter ) => {
  * @param {ui.TemplateDefinition} extDef An extension to existing instance or definition.
  */
 Template.extend = ( instanceOrDef, extDef ) => {
-	normalize( extDef );
+	const extDefClone = clone( extDef );
+
+	normalize( extDefClone );
 
 	if ( instanceOrDef instanceof Template ) {
-		extendTemplateDefinition( instanceOrDef.definition, extDef );
-	} else {
-		extendTemplateDefinition( instanceOrDef, extDef );
+		extendTemplateDefinition( instanceOrDef.definition, extDefClone );
+	}
+	// Extend a particular child in existing template instance.
+	//
+	//		Template.extend( instance.definition.children[ 0 ], {
+	//			attributes: {
+	//				class: 'd'
+	//			}
+	//		} );
+	//
+	else {
+		extendTemplateDefinition( instanceOrDef, extDefClone );
 	}
 };
 
@@ -632,6 +646,27 @@ function getAttributeUpdater( el, attrName, ns = null ) {
 	};
 }
 
+/*
+ * Clones definition of the template.
+ *
+ * @ignore
+ * @private
+ * @param {ui.TemplateDefinition} def
+ * @returns {ui.TemplateDefinition}
+ */
+function clone( def ) {
+	const clone = cloneDeepWith( def, value => {
+		// Don't clone Template.bind* bindings because there are references
+		// to Observable and DOMEmitterMixin instances inside, which are external
+		// to the Template.
+		if ( value && value.type ) {
+			return value;
+		}
+	} );
+
+	return clone;
+}
+
 /**
  * Normalizes given {@link ui.TemplateDefinition}.
  *
@@ -643,8 +678,7 @@ function getAttributeUpdater( el, attrName, ns = null ) {
  *
  * @ignore
  * @private
- * @param {ui.TemplateValueSchema} valueSchema
- * @returns {Array}
+ * @param {ui.TemplateDefinition} def
  */
 function normalize( def ) {
 	if ( def.text ) {

+ 48 - 1
packages/ckeditor5-ui/tests/template.js

@@ -26,7 +26,8 @@ describe( 'Template', () => {
 				tag: 'p'
 			};
 
-			expect( new Template( def ).definition ).to.equal( def );
+			expect( new Template( def ).definition ).to.not.equal( def );
+			expect( new Template( def ).definition.tag ).to.equal( 'p' );
 		} );
 
 		it( 'normalizes template definition', () => {
@@ -81,6 +82,29 @@ describe( 'Template', () => {
 			expect( def.on[ 'c@span' ][ 0 ].type ).to.be.a( 'symbol' );
 			expect( def.on[ 'c@span' ][ 1 ].type ).to.be.a( 'symbol' );
 		} );
+
+		it( 'does not modify passed definition', () => {
+			const def = {
+				tag: 'p',
+				attributes: {
+					a: 'foo',
+				},
+				children: [
+					{
+						tag: 'span'
+					}
+				]
+			};
+			const tpl = new Template( def );
+
+			expect( def ).to.not.equal( tpl.definition );
+			expect( def.attributes ).to.not.equal( tpl.definition.attributes );
+			expect( def.children ).to.not.equal( tpl.definition.children );
+			expect( def.children[ 0 ] ).to.not.equal( tpl.definition.children[ 0 ] );
+
+			expect( tpl.definition.attributes.a[ 0 ] ).to.equal( 'foo' );
+			expect( def.attributes.a ).to.equal( 'foo' );
+		} );
 	} );
 
 	describe( 'render', () => {
@@ -1194,6 +1218,29 @@ describe( 'Template', () => {
 			bind = Template.bind( observable, emitter );
 		} );
 
+		it( 'does not modify passed definition', () => {
+			const def = {
+				tag: 'p',
+				attributes: {
+					a: 'foo',
+				}
+			};
+			const ext = {
+				attributes: {
+					b: 'bar'
+				}
+			};
+			const tpl = new Template( def );
+
+			Template.extend( tpl, ext );
+
+			expect( def.attributes.a ).to.equal( 'foo' );
+			expect( ext.attributes.b ).to.equal( 'bar' );
+
+			expect( tpl.definition.attributes.a[ 0 ] ).to.equal( 'foo' );
+			expect( tpl.definition.attributes.b[ 0 ] ).to.equal( 'bar' );
+		} );
+
 		describe( 'attributes', () => {
 			it( 'extends existing - simple', () => {
 				extensionTest(