xaviergonz / mobx-keystone

A MobX powered state management solution based on data trees with first class support for Typescript, support for snapshots, patches and much more
https://mobx-keystone.js.org
MIT License
549 stars 25 forks source link

Switch prop type and prop transform output type? #369

Open sisp opened 2 years ago

sisp commented 2 years ago

While adding the stringToBigIntTransform prop transform, I found it not very intuitive to give the prop the encoded type (e.g. string) and then apply a prop transform to get the desired type (e.g. bigint). Instead, I think the opposite order would be more intuitive, e.g.:

@model("M")
class M extends Model({
  int: prop<bigint>().withTransform(bigintToStringTransform()),
  date: prop<Date>().withTransform(dateToTimestampTransform()),
  // ...
}) {
  // ...
}

What do you think?

sisp commented 2 years ago

On the other hand, when using my suggestion with tProp, the input of the prop transform would not be validated whereas with the current approach it is, e.g.:

@model("M")
class M extends Model({
  int: tProp(types.string).withTransform(stringToBigIntTransform()),
  date: tProp(types.number).withTransform(timestampToDateTransform()),
  // ...
}) {
  //
}

Nevertheless, this API feels not so intuitive, at least to me. :thinking:

sisp commented 2 years ago

Perhaps tProp(...).withTransform(...) could return a new tProp with the new runtime type? Each prop transform would need to provide it, of course. E.g.:

@model("M")
class M extends Model({
  int: tProp(types.bigint).withTransform(bigintToStringTransform()), // -> tProp(types.string).withTransform(bigIntToStringTransform())
  date: tProp(types.date).withTransform(dateToTimestampTransform()), // -> tProp(types.number).withTransform(dateToTimestampTransform())
  // ...
}) {
  // ...
}

Do you know what I mean?

sisp commented 2 years ago

The more I think about it, the more it feels related to the other discussions about dropping prop in favor of only using tProp (https://github.com/xaviergonz/mobx-keystone/issues/299 / https://github.com/xaviergonz/mobx-keystone/issues/299#issuecomment-905784266, https://github.com/xaviergonz/mobx-keystone/issues/308).

What I mean is: Property transforms in the general case are not a feature of model props but an encoder/decoder mechanism of runtime types/schemas. Similarly, as discussed in https://github.com/xaviergonz/mobx-keystone/issues/299, deeply nested optional fields with default values can only be expressed properly using a runtime schema. So I think the ultimate solution to this and the other topics may be building out the runtime types beyond mere type-checking.