From 159098623d1857f93d2082b3d30bd5a566b13731 Mon Sep 17 00:00:00 2001 From: Cherif BOUCHELAGHEM Date: Wed, 9 Oct 2019 16:14:00 +0100 Subject: [PATCH 1/5] On error include the name of the property that is being set --- package.json | 1 + src/define.js | 14 ++++++++++++-- test/define-test.js | 19 +++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 363f8df..2bcacd5 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "can-queues": "^1.2.1", "can-reflect": "^1.17.9", "can-simple-observable": "^2.5.0", + "can-string": "^1.1.0", "can-string-to-any": "^1.2.0", "can-symbol": "^1.6.0", "can-type": "^1.0.0" diff --git a/src/define.js b/src/define.js index aeb1a47..c4384fe 100644 --- a/src/define.js +++ b/src/define.js @@ -18,13 +18,15 @@ var canLogDev = require("can-log/dev/dev"); var defineLazyValue = require("can-define-lazy-value"); var type = require("can-type"); +var canString = require('can-string'); var newSymbol = Symbol.for("can.new"), serializeSymbol = Symbol.for("can.serialize"), inSetupSymbol = Symbol.for("can.initializing"), isMemberSymbol = Symbol.for("can.isMember"), hasBeenDefinedSymbol = Symbol.for("can.hasBeenDefined"), - canMetaSymbol = Symbol.for("can.meta"); + canMetaSymbol = Symbol.for("can.meta"), + baseTypeSymbol = Symbol.for("can.baseType"); var eventsProto, define, make, makeDefinition, getDefinitionsAndMethods, getDefinitionOrMethod; @@ -688,7 +690,15 @@ make = { return setter; } else { return function setter(newValue){ - return set.call(this, canReflect.convert(newValue, type)); + try { + var val = canReflect.convert(newValue, type); + return set.call(this, val); + } catch (error) { + var typeName = canReflect.getName(type[baseTypeSymbol]); + var propType = canString.capitalize(typeof prop); + var message = newValue + ' is not of type ' + typeName + '. Property ' + prop + ' is using type: ' + propType; + throw new Error(message); + } }; } } diff --git a/test/define-test.js b/test/define-test.js index 9297840..19e2d35 100644 --- a/test/define-test.js +++ b/test/define-test.js @@ -2,6 +2,7 @@ const QUnit = require("steal-qunit"); const { mixinObject } = require("./helpers"); const { hooks } = require("../src/define"); const canReflect = require("can-reflect"); +const type = require('can-type'); QUnit.module("can-observable-mixin - define()"); @@ -188,3 +189,21 @@ QUnit.test("properties using value behavior reset when unbound", function(assert assert.equal(obj.derivedProp, undefined, "value reset"); }); + +QUnit.test('On error include the name of the property that is being set', function(assert) { + class Person extends mixinObject() { + static get props() { + return { + age: type.check(Number) + }; + } + } + + var farah = new Person(); + + try { + farah.age = '4'; + } catch (error) { + assert.equal(error.message, '4 is not of type Number. Property age is using type: String'); + } +}); From 2252d99b7f7eb5879e8a65d824583448212424c2 Mon Sep 17 00:00:00 2001 From: Cherif BOUCHELAGHEM Date: Wed, 9 Oct 2019 17:53:56 +0100 Subject: [PATCH 2/5] Include suggestions for what type to use --- src/define.js | 3 ++- test/define-test.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/define.js b/src/define.js index c4384fe..bf39d93 100644 --- a/src/define.js +++ b/src/define.js @@ -696,7 +696,8 @@ make = { } catch (error) { var typeName = canReflect.getName(type[baseTypeSymbol]); var propType = canString.capitalize(typeof prop); - var message = newValue + ' is not of type ' + typeName + '. Property ' + prop + ' is using type: ' + propType; + var message = newValue + ' is not of type ' + typeName + '. Property ' + prop + ' is using "type: ' + propType + '". '; + message += 'Use "' + prop + ': type.To' + typeName + '" to automatically convert values to ' + typeName + 's when setting the "' + prop + '" property.'; throw new Error(message); } }; diff --git a/test/define-test.js b/test/define-test.js index 19e2d35..c9e0189 100644 --- a/test/define-test.js +++ b/test/define-test.js @@ -204,6 +204,6 @@ QUnit.test('On error include the name of the property that is being set', functi try { farah.age = '4'; } catch (error) { - assert.equal(error.message, '4 is not of type Number. Property age is using type: String'); + assert.equal(error.message, '4 is not of type Number. Property age is using "type: String". Use "age: type.ToNumber" to automatically convert values to Numbers when setting the "age" property.'); } }); From 24c147ced22f7cdc716eb73c5b6e98d87c068aa8 Mon Sep 17 00:00:00 2001 From: Cherif BOUCHELAGHEM Date: Wed, 9 Oct 2019 18:34:24 +0100 Subject: [PATCH 3/5] make try/catch for dev only --- src/define.js | 6 ++++-- test/define-test.js | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/define.js b/src/define.js index bf39d93..c91fb66 100644 --- a/src/define.js +++ b/src/define.js @@ -690,9 +690,9 @@ make = { return setter; } else { return function setter(newValue){ + //!steal-remove-start try { - var val = canReflect.convert(newValue, type); - return set.call(this, val); + return set.call(this, canReflect.convert(newValue, type)); } catch (error) { var typeName = canReflect.getName(type[baseTypeSymbol]); var propType = canString.capitalize(typeof prop); @@ -700,6 +700,8 @@ make = { message += 'Use "' + prop + ': type.To' + typeName + '" to automatically convert values to ' + typeName + 's when setting the "' + prop + '" property.'; throw new Error(message); } + //!steal-remove-end + return set.call(this, canReflect.convert(newValue, type)); }; } } diff --git a/test/define-test.js b/test/define-test.js index c9e0189..02f5f91 100644 --- a/test/define-test.js +++ b/test/define-test.js @@ -3,6 +3,7 @@ const { mixinObject } = require("./helpers"); const { hooks } = require("../src/define"); const canReflect = require("can-reflect"); const type = require('can-type'); +const dev = require("can-test-helpers").dev; QUnit.module("can-observable-mixin - define()"); @@ -190,7 +191,7 @@ QUnit.test("properties using value behavior reset when unbound", function(assert assert.equal(obj.derivedProp, undefined, "value reset"); }); -QUnit.test('On error include the name of the property that is being set', function(assert) { +dev.devOnlyTest('On error include the name of the property that is being set', function(assert) { class Person extends mixinObject() { static get props() { return { From 04bcc7d4a52ba38ef7c5ef65d148e4c02fbfc2be Mon Sep 17 00:00:00 2001 From: Cherif BOUCHELAGHEM Date: Wed, 9 Oct 2019 19:48:18 +0100 Subject: [PATCH 4/5] Use type.convert --- src/define.js | 2 +- test/define-test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/define.js b/src/define.js index c91fb66..aa8f28c 100644 --- a/src/define.js +++ b/src/define.js @@ -697,7 +697,7 @@ make = { var typeName = canReflect.getName(type[baseTypeSymbol]); var propType = canString.capitalize(typeof prop); var message = newValue + ' is not of type ' + typeName + '. Property ' + prop + ' is using "type: ' + propType + '". '; - message += 'Use "' + prop + ': type.To' + typeName + '" to automatically convert values to ' + typeName + 's when setting the "' + prop + '" property.'; + message += 'Use "' + prop + ': type.convert(' + typeName + ')" to automatically convert values to ' + typeName + 's when setting the "' + prop + '" property.'; throw new Error(message); } //!steal-remove-end diff --git a/test/define-test.js b/test/define-test.js index 02f5f91..02be8e7 100644 --- a/test/define-test.js +++ b/test/define-test.js @@ -205,6 +205,6 @@ dev.devOnlyTest('On error include the name of the property that is being set', f try { farah.age = '4'; } catch (error) { - assert.equal(error.message, '4 is not of type Number. Property age is using "type: String". Use "age: type.ToNumber" to automatically convert values to Numbers when setting the "age" property.'); + assert.equal(error.message, '4 is not of type Number. Property age is using "type: String". Use "age: type.convert(Number)" to automatically convert values to Numbers when setting the "age" property.'); } }); From ea5b2c281ac9b80abb5aa63360ce2ce51aea931a Mon Sep 17 00:00:00 2001 From: Cherif BOUCHELAGHEM Date: Thu, 10 Oct 2019 15:31:08 +0100 Subject: [PATCH 5/5] Remove try/catch from webpack build --- package.json | 1 - src/define.js | 18 +++++++++--------- test/define-test.js | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index 2bcacd5..363f8df 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,6 @@ "can-queues": "^1.2.1", "can-reflect": "^1.17.9", "can-simple-observable": "^2.5.0", - "can-string": "^1.1.0", "can-string-to-any": "^1.2.0", "can-symbol": "^1.6.0", "can-type": "^1.0.0" diff --git a/src/define.js b/src/define.js index aa8f28c..40af769 100644 --- a/src/define.js +++ b/src/define.js @@ -18,7 +18,6 @@ var canLogDev = require("can-log/dev/dev"); var defineLazyValue = require("can-define-lazy-value"); var type = require("can-type"); -var canString = require('can-string'); var newSymbol = Symbol.for("can.new"), serializeSymbol = Symbol.for("can.serialize"), @@ -691,14 +690,15 @@ make = { } else { return function setter(newValue){ //!steal-remove-start - try { - return set.call(this, canReflect.convert(newValue, type)); - } catch (error) { - var typeName = canReflect.getName(type[baseTypeSymbol]); - var propType = canString.capitalize(typeof prop); - var message = newValue + ' is not of type ' + typeName + '. Property ' + prop + ' is using "type: ' + propType + '". '; - message += 'Use "' + prop + ': type.convert(' + typeName + ')" to automatically convert values to ' + typeName + 's when setting the "' + prop + '" property.'; - throw new Error(message); + if(process.env.NODE_ENV !== 'production') { + try { + return set.call(this, canReflect.convert(newValue, type)); + } catch (error) { + var typeName = canReflect.getName(type[baseTypeSymbol]); + var message = newValue + ' is not of type ' + typeName + '. Property ' + prop + ' is using "type: ' + typeName + '". '; + message += 'Use "' + prop + ': type.convert(' + typeName + ')" to automatically convert values to ' + typeName + 's when setting the "' + prop + '" property.'; + throw new Error(message); + } } //!steal-remove-end return set.call(this, canReflect.convert(newValue, type)); diff --git a/test/define-test.js b/test/define-test.js index 02be8e7..741f0b4 100644 --- a/test/define-test.js +++ b/test/define-test.js @@ -205,6 +205,6 @@ dev.devOnlyTest('On error include the name of the property that is being set', f try { farah.age = '4'; } catch (error) { - assert.equal(error.message, '4 is not of type Number. Property age is using "type: String". Use "age: type.convert(Number)" to automatically convert values to Numbers when setting the "age" property.'); + assert.equal(error.message, '4 is not of type Number. Property age is using "type: Number". Use "age: type.convert(Number)" to automatically convert values to Numbers when setting the "age" property.'); } });