Proposal: Add type checking to can-define & can-observe

[justinbmeyer]

tldr; Lets create a type setting that enforces the type!

This was discussed on a recent live stream and a previous one (29:55).

Problem

Related to #173, it would be nice for type settings to enforce the type. For example, the following specifies that age will be of type number (or undefined or null):

var MyType = DefineMap.extend({
  age: {type: "number"}
});

But if initialized or set to a non-number value, it will try to type-coerce the value:

var myType = new MyType({age: "3"});
myType.age //=> 3

This can sometimes be what you want, more often it's not:

myType.age = "abc"
myType.age //-> NaN

Solution

I think it would be valuable to actually enforce type. Ideally, by throwing an error if the type was set to the wrong value:

var MyType = DefineMap.extend("MyType",{
  age: {typeCheck: "number"}
});

var myType = new MyType({age: "3"}); //-> THROWS "MyType{}.age set to a value that isn't MaybeNumber".

As type and typeCheck would be asymmetrical, I think we should move the existing type conversion behavior to a typeConvert property behavior:

var MyType = DefineMap.extend({
  age: {typeConvert: "number"}
});

type would still be backwards compatible, but deprecated in the docs.

Alternate APIs

• age: {checkType: MaybeNumber} and age: {convertType: MaybeNumber}
• age: {strictType: MaybeNumber}
• age: {type: "number", convert: false}
• age: {type: "number", strict: true}

Other Considerations

We support Type: Foo.

I think Type: Foo can also be deprecated. can-reflect.isConstructorLike() should be able to tell the difference between a constructor function and a normal function in all reasonable cases. In the cases where it can not, someone would just have to decorate it with the can.new symbol, to make a function that should be a constructor appear like a constructor.

can-observe

I'd like something similar to work for can-observe eventually, decorating class-fields:

Thing extends observe.Object {
  @observe.checkType(types.MaybeDate)
  dueDate = new Date();

  
  @observe.checkType(types.MaybeNumber)
  age = null;
}

[chasenlehara]

Some other ideas:

• checkType: default to false
• convertType: default to true

I think it’s nice to have “type” in the name so it’s clear it corresponds with the type specified, vs. any of the other properties.

[justinbmeyer]

@chasenlehara would these replace "type"? We could do something like:

• age: {checkType: "number"} and
• age: {convertType: "number"} <-- this is the current behavior

I like this because it would work well with possible can-observe decorators:

class MyType extends observe.Object {
  @observe.checkType( [Number, null] )
  age=null
}

[phillipskevin]

What would we do if both properties are the same then?

If both are false, I guess it would be fine.

If both are true, would you ignore the conversion and throw?

[chasenlehara]

That’s an interesting idea… running with that, maybe something convertTo: "number"?

My only concern would be if both type and convertTo are provided: of course we can throw an error and provide docs, but maybe it would be confusing?

Also for Type, I might want null/undefined passed to my constructor function so I can handle that however it’s appropriate for the type. I think just one boolean option (like convertType) would be nice.

[phillipskevin]

I think I would prefer just sticking with the current properties and adding some constructors that handle throwing if passed the wrong thing

{
  Type: StrictNumber
}

[justinbmeyer]

@chasenlehara as I'm creating can-data-types, I like seeing type somewhere in the property name. These are going to be more well known constructs as can-query-logic and hopefully <can-crud> come to being.

[justinbmeyer]

@phillipskevin Technically, this makes things a bit tricker (and harder for folks that want to create their own types).

Currently, the type objects like MaybeType have 2 methods: can.new and can.isMember. We can already use isMember to check if a value being set isMember and throw an error. No need to create a bunch of other types to handle this case.

Oh, there's another reason ... the error propagation should contain the name of the property ....

isMember allows us to do something like this:

set age(value){
  if(!MaybeNumber.isMember(value) ){
    throw prop+"value isn't MaybeNumber".
  }
}

We'd have to use a try/catch/throw if it was entirely w/i the can.new call:

set age(value){
  try{
     var converted = MaybeNumber.new(value)
  } catch(e){
     throw prop+"value isn't MaybeNumber".
  }
  ...
}