fixes #63, handles primitive return types

canjs · Jun 15, 2018 · 781f628 · 781f628
1 parent 8bdfbe1
commit 781f628
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 5 deletions.
diff --git a/reflections/shape/shape-test.js b/reflections/shape/shape-test.js
@@ -377,6 +377,67 @@ QUnit.test(".serialize with recursive data structures", function(){
 	QUnit.equal(s.prop, s, "Object points to itself");
 });
 
+if(typeof Map !== "undefined") {
+	QUnit.test("objects that serialize to strings should cache properly", function(){
+		function SimpleType(){}
+		getSetReflections.setKeyValue(SimpleType.prototype, canSymbol.for("can.serialize"), function(){
+			return "baz";
+		});
+		var obj = new SimpleType();
+		var p = {
+			foo: obj, bar: obj
+		};
+		deepEqual(shapeReflections.serialize(p, Map), {foo:"baz", bar:"baz"});
+	});
+
+	QUnit.test("throw error when serializing circular reference", function(){
+		function SimpleType(){}
+		var a = new SimpleType();
+		var b = new SimpleType();
+		a.b = b;
+		b.a = a;
+		getSetReflections.setKeyValue(a, canSymbol.for("can.serialize"), function(){
+			return {
+				b: shapeReflections.serialize(this.b)
+			};
+		});
+		getSetReflections.setKeyValue(b, canSymbol.for("can.serialize"), function(){
+			return {
+				a: shapeReflections.serialize(this.a)
+			};
+		});
+
+		try{
+			shapeReflections.serialize(a, Map);
+			QUnit.ok(false);
+		}catch(e){
+			QUnit.ok(true);
+		}
+	});
+
+	QUnit.test("throw should not when serializing circular reference properly", function(){
+		function SimpleType(){}
+		var a = new SimpleType();
+		var b = new SimpleType();
+		a.b = b;
+		b.a = a;
+		getSetReflections.setKeyValue(a, canSymbol.for("can.serialize"), function(proto){
+			return proto.b = shapeReflections.serialize(this.b);
+		});
+		getSetReflections.setKeyValue(b, canSymbol.for("can.serialize"), function(proto){
+			return proto.a = shapeReflections.serialize(this.a);
+		});
+
+		try{
+			shapeReflections.serialize(a, Map);
+			QUnit.ok(true);
+		}catch(e){
+			QUnit.ok(false);
+		}
+	});
+}
+
+
 QUnit.test("updateDeep basics", function(){
 
 	var obj = {
@@ -518,7 +579,7 @@ QUnit.test("updateDeep recurses correctly (#73)", function(){
 QUnit.module('can-reflect: shape reflections: proto chain');
 
 QUnit.test("hasKey", function() {
-	/*var objHasKey = {};
+	var objHasKey = {};
 	Object.defineProperty(objHasKey, "_keys", {
 		value: { foo: true }
 	});
@@ -539,8 +600,7 @@ QUnit.test("hasKey", function() {
 	QUnit.ok(!shapeReflections.hasKey(objHasOwnKey, "bar") , "returns false when hasOwnKey Symbol returns false");
 
 	objHasOwnKey.bar = "baz";
-	QUnit.ok(shapeReflections.hasKey(objHasOwnKey, "bar") , "returns true when hasOwnKey Symbol returns false but `in` returns true");*/
-	debugger;
+	QUnit.ok(shapeReflections.hasKey(objHasOwnKey, "bar") , "returns true when hasOwnKey Symbol returns false but `in` returns true");
 
 	QUnit.ok(shapeReflections.hasKey(55, "toFixed") , "works on primitives");
 	QUnit.ok(shapeReflections.hasKey(true, "valueOf") , "works on primitives");

diff --git a/reflections/shape/shape.js b/reflections/shape/shape.js
@@ -124,7 +124,15 @@ function makeSerializer(methodName, symbolsToCheck){
 		if(!serializeMap) {
 			serializeMap = {
 				unwrap: MapType ? new MapType() : new ArrayMap(),
-				serialize: MapType ? new MapType() : new ArrayMap()
+				serialize: MapType ? new MapType() : new ArrayMap(),
+				isSerializing: {
+					unwrap: new MapType(),
+					serialize: new MapType()
+				},
+				circularReferenceIsSerializing: {
+					unwrap: new MapType(),
+					serialize: new MapType()
+				}
 			};
 			firstSerialize = true;
 		}
@@ -144,6 +152,10 @@ function makeSerializer(methodName, symbolsToCheck){
 			if(serializeMap) {
 
 				if( serializeMap[methodName].has(value) ) {
+					// if we are in the process of serializing the first time, setup circular reference detection.
+					if(serializeMap.isSerializing[methodName].has(value)) {
+						serializeMap.circularReferenceIsSerializing[methodName].set(value, true);
+					}
 					return serializeMap[methodName].get(value);
 				} else {
 					serializeMap[methodName].set(value, serialized);
@@ -153,7 +165,25 @@ function makeSerializer(methodName, symbolsToCheck){
 			for(var i = 0, len = symbolsToCheck.length ; i< len;i++) {
 				var serializer = value[symbolsToCheck[i]];
 				if(serializer) {
-					var result =  serializer.call(value, serialized);
+					// mark that we are serializing
+					serializeMap.isSerializing[methodName].set(value, true);
+					var result = serializer.call(value, serialized);
+					serializeMap.isSerializing[methodName].delete(value);
+
+					// if the result differs, but this was circular, blow up.
+					if(result !== serialized) {
+						// jshint -W073
+						if(serializeMap.circularReferenceIsSerializing[methodName].has(value)) {
+							// Circular references should use a custom serializer
+							// that sets the serialized value on the object
+							// passed to it as the first argument e.g.
+							// function(proto){
+							//   return proto.a = canReflect.serialize(this.a);
+							// }
+							throw new Error("Cannot serialize cirular reference!");
+						}
+						serializeMap[methodName].set(value, result);
+					}
 					if(firstSerialize) {
 						serializeMap = null;
 					}