Native types definitions instead of strings

[Devoter]

I have a suggestion. It seems to me more intuitive to use native classes e.g.: Boolean, String, Object, Array, Symbol or MyOwnClass instead of strings. For example:

@argument(Boolean) enabled = true;

vs

@argument('boolean') enabled = true;

[pzuraq]

The reason we went with string types was to be as close as we could to approximating the Typescript type system. For instance, there are primitive types in TS such as any and object which don’t have an equivalent native JS class. I’m open to adding it as an option for those that do though, what do you think @alexlafroscia?

[Devoter]

I think you can add Any class as like as your Action class.

[alexlafroscia]

I really like this idea! It’s something I’ve been thinking about lately, as it would be nice to pass in a constructor rather than a complex shapeOf description.

@Devoter I really like that idea! It doesn’t even need to be a class; an exported Symbol would work for this too.

I’d be happy to take a pass at supporting this!

[pzuraq]

Ah FWIW, you definitely can pass in constructors as is, I was just referring to primitive types. That’s what the instanceof checks are for.

[pzuraq]

https://github.com/ember-decorators/argument/blob/master/addon/-private/validators.js#L43-L44

[Devoter]

@pzuraq Right, but there is no implementation for Any type definition and no words in the documentation.

[alexlafroscia]

I opened a PR that addresses two things that this PR requests:

• Better documentation around passing a constructor to @argument (I also added more tests around this use-case)
• Exporting an Any type that can be passed to @argument

One thing to not, though: Boolean is not the same as boolean. Since Boolean is the "boxed" version of the boolean primitive, I'm not sure that this library should implicitly covert between the two.

That essentially means that this will thrown an error:

class Whatever {
  @argument(Boolean)
  prop = false;
}

but these will not:

class Whatever {
  @argument(Boolean)
  prop = new Boolean(false);

  @argument('boolean')
  otherProp = false;
}

One thought to address this would be exporting a series of validators like this:

// add/types.js
export const Boolean = or(
  new TypeMatchValidator('boolean'),
  new InstanceOfValidator(Boolean)
)

for each of the Primitive types that would allow either the primitive or boxed versions to be provided.

[Devoter]

@alexlafroscia I think this is not a good idea to override built-in classes with your own validators. You may use typeof() instead of instanceof for the Primitive types.

[pzuraq]

I definitely think that it's a bad idea to export another type, that could get confusing fast. We would either need to have custom logic specifically for primitive types that can knows to compare them against the boxed version as well when they are received, or we should just stick with strings, IMO. In other words either:

class Whatever {
  @argument(Boolean)
  otherProp = false;
}

Should Just Work™️ or we should just stick with what we have. I think this form would be nice, but it would also be more logic to add to the validators, and means we can't actually check if the prop is the unboxed value only, but I think that would generally be alright.

[alexlafroscia]

@alexlafroscia I think this is not a good idea to override built-in classes with your own validators. You may use typeof() instead of instanceof for the Primitive types.

Just a note: I'm definitely not suggesting that we would override the way that passing the Boolean constructor would work. My example there was something you could import from the library to validate against either the boxed or unboxed version.

I definitely think that it's a bad idea to export another type, that could get confusing fast.

In that case, I think we should just stick with what we have, where the constructor for a primitive may not work with the unboxed version but there's the string version of the type that can be used instead.