Merge pull request #256 from canjs/register-shouldnt-read-data

Register shouldn't read data
canjs · Nov 25, 2019 · f3f2a40 · f3f2a40
2 parents 01713af + ad4aa9e
commit f3f2a40
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 32 deletions.
diff --git a/can-route.js b/can-route.js
@@ -6,6 +6,7 @@
 var Bind = require("can-bind");
 var queues = require("can-queues");
 var Observation = require("can-observation");
+var type = require("can-type");
 
 var namespace = require("can-namespace");
 var devLog = require("can-log/dev/dev");
@@ -176,6 +177,27 @@ canReflect.assignMap(canRoute, {
 
 	// ## canRoute.start
 	start: function (val) {
+		if (canRoute.data instanceof RouteData) {
+			var routeData = canRoute.data;
+			var definePropertyWithDefault = function(defaults, name) {
+				var defaultValue = defaults[name];
+				var propertyType = defaultValue != null ? type.maybeConvert(defaultValue.constructor) : type.maybeConvert(String);
+				canReflect.defineInstanceKey(routeData.constructor, name, {
+					type: propertyType
+				});
+			};
+
+			canReflect.eachKey(canRoute.routes, function(route) {
+				canReflect.eachIndex(route.names, function (name) {
+					definePropertyWithDefault(route.defaults, name);
+				});
+
+				canReflect.eachKey(route.defaults, function(value, key) {
+					definePropertyWithDefault(route.defaults, key);
+				});
+			});
+		}
+
 		if (val !== true) {
 			canRoute._setup();
 			if (isBrowserWindow() || isWebWorker()) {
@@ -192,6 +214,7 @@ canReflect.assignMap(canRoute, {
 				updateUrl();
 			}
 		}
+
 		return canRoute;
 	},
 	// ## canRoute.url
@@ -346,13 +369,6 @@ Object.defineProperty(canRoute, "data", {
 		}
 	},
 	set: function(data) {
-		//!steal-remove-start
-		if (typeof process !== "undefined" && process.env.NODE_ENV !== "production") {
-			if (Object.keys(registerRoute.routes).length > 0) {
-				devLog.warn("can-route: Setting can-route.data after calling can-route.register() may result in unexpected behavior. Set can-route.data before calling can-route.register().");
-			}
-		}
-		//!steal-remove-end
 		if ( canReflect.isConstructorLike(data) ) {
 			data = new data();
 		}

diff --git a/src/register.js b/src/register.js
@@ -3,14 +3,13 @@
 var canReflect = require("can-reflect");
 
 var dev = require("can-log/dev/dev");
-var type = require("can-type");
 
 var bindingProxy = require("./binding-proxy");
 var regexps = require("./regexps");
 
 var diff = require("can-diff/list/list");
 var diffObject = require("can-diff/map/map");
-var RouteData = require("./routedata");
+
 // `RegExp` used to match route variables of the type '{name}'.
 // Any word character or a period is matched.
 
@@ -102,25 +101,7 @@ var RouteRegistry = {
 				}
 			});
 		}
-			//!steal-remove-end
-
-			// Assign to the instance props
-			if (this.data instanceof RouteData) {
-				var routeData = this.data;
-				var definePropertyWithDefault = function(name) {
-					var defaultValue = defaults[name];
-					var propertyType = defaultValue != null ? type.maybeConvert(defaultValue.constructor) : type.maybeConvert(String);
-
-					canReflect.defineInstanceKey(routeData.constructor, name, {
-						type: propertyType
-					});
-				};
-				canReflect.eachIndex(names, definePropertyWithDefault);
-				canReflect.eachKey(defaults, function(value, key){
-					definePropertyWithDefault(key);
-				});
-
-			}
+		//!steal-remove-end
 
 		// Add route in a form that can be easily figured out.
 		return RouteRegistry.routes[url] = {

diff --git a/test/route-data-test.js b/test/route-data-test.js
@@ -5,7 +5,6 @@ var canReflect = require("can-reflect");
 var canSymbol = require("can-symbol");
 var mockRoute = require("./mock-route-binding");
 var RouteData = require("../src/routedata");
-var testHelpers = require("can-test-helpers");
 
 require("can-observation");
 
@@ -105,11 +104,37 @@ QUnit.test("Default map registers defaults as properties", function(assert) {
     canRoute.start();
 });
 
-testHelpers.dev.devOnlyTest("should warn when .data is set after .register() is called", function (assert) {
-	var teardown = testHelpers.dev.willWarn(/Set can-route.data before/);
+QUnit.test("route.register should not read route.data, register first", function (assert) {
+	var done = assert.async();
+	mockRoute.start();
 
 	canRoute.register("{page}/{subpage}");
 	canRoute.data = new RouteData();
 
-	assert.equal(teardown(), 1);
+	canRoute._onStartComplete = function () {
+		assert.ok('page' in canRoute.data);
+		assert.ok('subpage' in canRoute.data);
+		done();
+		mockRoute.stop();
+	};
+
+	canRoute.start();
+});
+
+QUnit.test("route.register should not read route.data 2, start first", function (assert) {
+	var done = assert.async();
+	mockRoute.start();
+
+	canRoute.data = new RouteData();
+	canRoute.register("{page}/{subpage}");
+
+	canRoute._onStartComplete = function () {
+		assert.ok('page' in canRoute.data);
+		assert.ok('subpage' in canRoute.data);
+		done();
+		mockRoute.stop();
+	};
+
+
+	canRoute.start();
 });
diff --git a/test/route-observable-test.js b/test/route-observable-test.js
@@ -340,6 +340,7 @@ if ("onhashchange" in window) {
 
 			canRoute.routes = {};
 			canRoute.register("{type}/{id}");
+			canRoute.start();
 
 			canReflect.update(canRoute.data, {});