effect icon indicating copy to clipboard operation
effect copied to clipboard

Schema: default class field constructors

Open patroza opened this issue 1 year ago • 5 comments

What is the problem this feature would solve?

I often find myself needing to provide default values at construction, but not at parsing. At parsing time, a missing value is often regarded as a bug; something in the storage or in flight is wrong, or perhaps there’s some backwards compatibility to deal with.

class Person extends S.Class<Person>()({
  name: S.string,
  createdAt: S.Date,
  posts: S.array(S.string)
}) {}
const person = new Person({ name: "Patrick", createdAt: new Date(), posts: [] })

What is the feature you are proposing to solve the problem?

Add a combinator to enhance property descriptors for fields in class schemas, so that constructor defaults can be expressed.

class Person extends S.Class<Person>()({
  name: S.string,
  createdAt: S.Date.pipe(S.withDefaultMake(() => new Date())),
  posts: S.array(S.string).pipe(S.withDefaultMake(() => []))
}) {}
const person = new Person({ name: "Patrick" })

Naive implementation;

  • combinator: https://github.com/patroza/effect/pull/1/commits/f8a0564f749cfc73f1aa529e1b167472225cfc54#diff-7da43f2b7b3dea1554f2627482ba41e4f885c908848b72704d9887ffcc065153R4675

  • constructor: https://github.com/patroza/effect/pull/1/commits/f8a0564f749cfc73f1aa529e1b167472225cfc54#diff-7da43f2b7b3dea1554f2627482ba41e4f885c908848b72704d9887ffcc065153R4557

What alternatives have you considered?

class Person extends S.Class<Person>()({
  name: S.string,
  createdAt: S.Date,
  posts: S.array(S.string)
}) {
  constructor(props: Omit<Person, "createdAt" | "posts" | keyof Data.Case> & { createdAt?: Date, posts?: string[] }, disableValidation?: boolean) {
    super({ createdAt: props.createdAt ?? new Date(), posts: props.date ?? [], ...posts} , disableValidation)
  }
}

but it is obviously tedious and it gets even more hairy if possibly all values have defaults and you would like to support new Person(). It's also not re-usable.

patroza avatar Jan 28 '24 11:01 patroza

@gcanti what's your thought about this? we mentioned a few times the possibility of adding a generic to Schema to represent the type from construction, this can be useful in many other cases such as for example branded types & refinements, being able to have a validating constructor kind of completes the idea of defining types (data models) through schema.

In the very old schema I had one and I have to say it was working kind of fine, given that we are stuffing Schema with params for context it might be the time to evaluate as a more general solution to this specific issue

mikearnaldi avatar Jan 31 '24 18:01 mikearnaldi

@mikearnaldi Schema classes already carry a constructor generic which is controlled by the property descriptors of the struct schema; https://github.com/Effect-TS/effect/blob/10bcbc64c602a32d74ff4c075cf3a6a9c1ebf005/packages/schema/src/Schema.ts#L4545 for struct schemas you can also have a createConstructor combinator or so.

As to validating constructors for branded types & refinements; you already carry the From and To, that is exactly the type of constructor I generally need:

export const addConstructor = <Self extends S.Schema<never, any, any>>(s: Self) => {
  return Object.assign(S.decodeSync(s) as SchemaConstructor<Self>, s)
}
export type SchemaConstructor<Self extends S.Schema<any, any>> = (
  i: S.Schema.From<Self>,
  options?: AST.ParseOptions
) => S.Schema.To<Self>

So I would personally not bother with an I on schemas.

patroza avatar Jan 31 '24 18:01 patroza

@patroza another alternative to consider:

Option 1:

class Person extends Schema.Class<Person>()({
  name: Schema.string,
  createdAt: Schema.Date,
  posts: Schema.array(Schema.string),
}) {
  constructor(data: { name: Person["name"] }) {
    super({ ...data, createdAt: new Date(), posts: [] });
  }
}

const person = new Person({
  name: "Stefan",
});

Option 2:

class Person extends Schema.Class<Person>()({
  name: Schema.string,
  createdAt: Schema.Date,
  posts: Schema.array(Schema.string),
}) {
  static make(data: { name: Person["name"] }) {
    return new Person({
      ...data,
      createdAt: new Date(), //default value
      posts: [], //default value
    });
  }
}

const person = Person.make({
  name: "Stefan",
});

steffanek avatar Jan 31 '24 20:01 steffanek

@steffanek option 1 breaks the constructor which is used by schema implementation on decode, Arbitrary creation etc, it will always create new date and posts array.

option 2 doesn’t break it but prevents you from supply alternative values. Then agai, it is also nice to have two constructors; the class constructor used for schema purposes and the make constructor for brand new instances.

My suggestions however tick both boxes; you can new up with defaults, or to restore or for copy and change.

patroza avatar Jan 31 '24 22:01 patroza

linked (in top post) naive implementation code updated with some fixes

patroza avatar Feb 02 '24 09:02 patroza

@gcanti I've added a general proposal for adding constructors to "bare" non Schema classes in #2443. The one proposed for Schema in #2319 is more advanced, because the defaults are carried with the field PropertyDescriptor, so that they can be picked and carried onward into new schemas, which is a very powerful concept, just like the other property descriptors.

@mikearnaldi I still think we don't need to carry an explicit I

  • Classes have their native constructors built-in. With the property descriptor proposal, this will be enhanced with defaults
  • Structs carry the To shape already, which apart from defaults, is the shape of constructor too, just like classes. With the property descriptor proposal, this will be enhanced with defaults, and can be derived via e.g something like S.structConstructor(structSchema)
  • Branded etc carry From-> To already, which is the shape of the constructor

patroza avatar Mar 30 '24 09:03 patroza