Skip to content

Commit

Permalink
Only type errors should be catched (#149)
Browse files Browse the repository at this point in the history
* Only type errors should be catched

* Update can-type version

* Update can-type version
  • Loading branch information
cherifGsoul authored Nov 1, 2019
1 parent fa5a085 commit daeb3cd
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 14 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"can-simple-observable": "^2.5.0",
"can-string-to-any": "^1.2.0",
"can-symbol": "^1.6.0",
"can-type": "^1.0.0"
"can-type": "^1.1.0"
},
"devDependencies": {
"@babel/cli": "^7.4.4",
Expand Down
14 changes: 9 additions & 5 deletions src/define.js
Original file line number Diff line number Diff line change
Expand Up @@ -694,11 +694,15 @@ make = {
try {
return set.call(this, canReflect.convert(newValue, type));
} catch (error) {
var typeName = canReflect.getName(type[baseTypeSymbol]);
var valueType = typeof newValue;
var message = '"' + newValue + '"' + ' ('+ valueType + ') 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);
if (error.type === 'can-type-error') {
var typeName = canReflect.getName(type[baseTypeSymbol]);
var valueType = typeof newValue;
var message = '"' + newValue + '"' + ' ('+ valueType + ') 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);
} else {
throw error;
}
}
}
//!steal-remove-end
Expand Down
48 changes: 40 additions & 8 deletions test/define-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@ const { mixinObject } = require("./helpers");
const { hooks } = require("../src/define");
const canReflect = require("can-reflect");
const type = require('can-type');
const Observation = require('can-observation');
const dev = require("can-test-helpers").dev;

QUnit.module("can-observable-mixin - define()");

QUnit.test("Can define stuff", function(assert) {
class Faves extends mixinObject() {
static get props() {
return {
static get props() {
return {
color: {
default: "blue"
}
};
}
};
}
}

let faves = new Faves();
Expand All @@ -30,13 +31,13 @@ QUnit.test("Does not throw if no define is provided", function(assert) {

QUnit.test("Stuff is defined in constructor for non-element classes", function(assert) {
class Faves extends mixinObject(Object) {
static get props() {
return {
static get props() {
return {
color: {
default: "blue"
}
};
}
};
}

constructor() {
super();
Expand Down Expand Up @@ -226,3 +227,34 @@ dev.devOnlyTest('Handle types as arrays to fix "Right-hand side of instanceof is
assert.ok(error.message);
}
});

dev.devOnlyTest('Only can-type error should be catched', function(assert) {
class T extends mixinObject() {
static get props() {
return {
aStr: {
type: type.maybe(String),
default: "Hi"
},
get foo() {
return this.aStr.toUpperCase();
}
};
}
}

var t = new T();

var obs = new Observation(function() {
return t.foo;
});
canReflect.onValue(obs, function() {});

try {
t.aStr = null;
} catch (error) {
assert.equal(error.type, undefined, 'can-type error only thrown on wrong types');
}

});

0 comments on commit daeb3cd

Please sign in to comment.