From 1448610146c7b07c37c72bd31709d692808818ef Mon Sep 17 00:00:00 2001 From: Bradley Momberger Date: Wed, 30 Aug 2017 17:20:35 -0400 Subject: [PATCH 1/2] Warn when a constructor is supplied as a type definition (likely should be a Type) (#205) --- can-define.js | 11 +++++++++++ define-test.js | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/can-define.js b/can-define.js index 1082858..9098a53 100644 --- a/can-define.js +++ b/can-define.js @@ -182,6 +182,17 @@ define.property = function(objPrototype, prop, definition, dataInitializers, com var type = definition.type; + //!steal-remove-start + if (type && canReflect.isConstructorLike(type)) { + dev.warn( + "can-define: the definition for " + + prop + + (objPrototype.constructor.shortName ? " on " + objPrototype.constructor.shortName : "") + + " uses a constructor for \"type\". Did you mean \"Type\"?" + ); + } + //!steal-remove-end + // Special case definitions that have only `type: "*"`. if (type && onlyType(definition) && type === define.types["*"]) { Object.defineProperty(objPrototype, prop, { diff --git a/define-test.js b/define-test.js index 2d94fc4..9398f8a 100644 --- a/define-test.js +++ b/define-test.js @@ -5,6 +5,7 @@ var CanList = require("can-define/list/list"); var canBatch = require("can-event/batch/batch"); var each = require("can-util/js/each/each"); var canSymbol = require("can-symbol"); +var canDev = require("can-util/js/dev/dev"); QUnit.module("can-define"); @@ -1368,3 +1369,41 @@ QUnit.test('define() should add a CID (#246)', function() { var g = new Greeting(); QUnit.ok(g._cid, "should have a CID property"); }); + +QUnit.test("warn on using a Constructor for small-t type definintions", function() { + expect(2); + var oldWarn = canDev.warn; + canDev.warn = function(mesg) { + QUnit.equal(mesg, "can-define: the definition for currency uses a constructor for \"type\". Did you mean \"Type\"?"); + }; + + function Currency() { + return this; + } + Currency.prototype = { + symbol: "USD" + }; + + function VM() {} + define(VM.prototype, { + currency: { + type: Currency, // should be `Type: Currency` + value: new Currency({}) + } + }); + + canDev.warn = function(mesg) { + QUnit.equal(mesg, "can-define: the definition for currency on VM2 uses a constructor for \"type\". Did you mean \"Type\"?"); + }; + + function VM2() {} + VM2.shortName = "VM2"; + define(VM2.prototype, { + currency: { + type: Currency, // should be `Type: Currency` + value: new Currency({}) + } + }); + + canDev.warn = oldWarn; +}); \ No newline at end of file From eeb2aab5dff7b21c72322575ab64c8ca5b7f50b1 Mon Sep 17 00:00:00 2001 From: Bradley Momberger Date: Wed, 30 Aug 2017 18:10:21 -0400 Subject: [PATCH 2/2] Guard dev warning test behind System.env check --- define-test.js | 66 ++++++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/define-test.js b/define-test.js index 9398f8a..e04de66 100644 --- a/define-test.js +++ b/define-test.js @@ -1370,40 +1370,44 @@ QUnit.test('define() should add a CID (#246)', function() { QUnit.ok(g._cid, "should have a CID property"); }); -QUnit.test("warn on using a Constructor for small-t type definintions", function() { - expect(2); - var oldWarn = canDev.warn; - canDev.warn = function(mesg) { - QUnit.equal(mesg, "can-define: the definition for currency uses a constructor for \"type\". Did you mean \"Type\"?"); - }; +if (System.env.indexOf("production") < 0) { - function Currency() { - return this; - } - Currency.prototype = { - symbol: "USD" - }; + QUnit.test("warn on using a Constructor for small-t type definintions", function() { + expect(2); + var oldWarn = canDev.warn; + canDev.warn = function(mesg) { + QUnit.equal(mesg, "can-define: the definition for currency uses a constructor for \"type\". Did you mean \"Type\"?"); + }; - function VM() {} - define(VM.prototype, { - currency: { - type: Currency, // should be `Type: Currency` - value: new Currency({}) - } - }); + function Currency() { + return this; + } + Currency.prototype = { + symbol: "USD" + }; - canDev.warn = function(mesg) { - QUnit.equal(mesg, "can-define: the definition for currency on VM2 uses a constructor for \"type\". Did you mean \"Type\"?"); - }; + function VM() {} + define(VM.prototype, { + currency: { + type: Currency, // should be `Type: Currency` + value: new Currency({}) + } + }); + + canDev.warn = function(mesg) { + QUnit.equal(mesg, "can-define: the definition for currency on VM2 uses a constructor for \"type\". Did you mean \"Type\"?"); + }; + + function VM2() {} + VM2.shortName = "VM2"; + define(VM2.prototype, { + currency: { + type: Currency, // should be `Type: Currency` + value: new Currency({}) + } + }); - function VM2() {} - VM2.shortName = "VM2"; - define(VM2.prototype, { - currency: { - type: Currency, // should be `Type: Currency` - value: new Currency({}) - } + canDev.warn = oldWarn; }); - canDev.warn = oldWarn; -}); \ No newline at end of file +} \ No newline at end of file