Stricter field type, and nullable by default

[kiaking]

This is still just an idea, so looking for feedback.

What do you think about making model attributes more strict? Currently, we have several fields with primitive types, such as string, number. etc. At the moment, these fields will cast values rather than restrict them. So passing true to string field will make it 'true'.

The idea is to make them fail instead of casting.

class User extends Model {
  static entity = 'users'

  @Str() name!: string
}

new User({ name: true }) // <- Error('name is not string')

In addition to this, how about if we make all the fields nullable by default. Because in the frontend, usually many fields are going to be nullable.

So instead of having nullable option, we'll add NonNullable decorator (if this is possible).

class User extends Model {
  static entity = 'users'

  // This field is number, and not nullable.
  @Num()
  @NonNullable()
  id!: number

  // This field is string, or null.
  @Str()
  name!: string | null
}

The default value should behave the same,

class User extends Model {
  static entity = 'users'

  // This field is nullable, but if we pass `null`, it becomes `John Doe`.
  // so technically it wouldn't be `null`.
  @Str('John Doe')
  name!: string
}

[superfreddialpad]

I think that making field types stricter is a great idea, I think it's more in-line with the spirit of typescript. If one wishes for true to exist as "true" in a string field, they should cast it themselves and not rely on the orm to do it automatically, which could lead to difficult-to-track-down bugs. Better to throw an Error and have it show up right away.

I don't have a strong opinion on whether to make attributes nullable by default, but I think that is orthogonal to the behavior of the default value. As per the spec [emphasis mine]:

4.4.15 null value
primitive value that represents the intentional absence of any object value

I contend that null should be a valid value for a nullable field, and that undefined should be the only value that uses the default. So an example user class:

class User extends Model {
  static entity = 'users'
  @Num(50)
  @NonNullable()
  age!: number

  @Str('John Doe')
  name!: string | null
}

should have the following behavior:

const r = store.$repo(User);

// saves { age: 50, name: 'John Doe' }
r.save({});

// saves { age: 50, name: null }
r.save({ name: null });

// throws Error - age is non-nullable
r.save({ age: null });

I'm also curious about specification of behavior for fields without a default. I don't know what the correct/current behavior is, but for example:

class Post extends Model {
  @Str()
  @NonNullable()
  body!: string

  @Str()
  title: string | null
}

const r = store.$repo(Model);

// throws Error - body is non-nullable and has no default
r.save({ title: "B" });

// what about a nullable field with no default?
// throws Error - makes the most sense to me.
// { title: null, body: "test" } - rather this be expressed as `Str(null)`
// { title: undefined, body: "test" }
// { body: "test" }
r.save({ body: "test" });

it's possible that this diverges too much from the current behavior, but this is how I think it should behave based on my understanding of the meaning of null.

[kiaking]

Thanks for the feedback!

I'm also curious about specification of behavior for fields without a default. I don't know what the correct/current behavior is, but for example:

Yes, I think this is how it should work if make null by default and add NonNullable option. If the field is NonNullable, then either passing in null or undefined to the persistent method should throw. The nullable field without default value is same as setting default value as null.