-
-
Notifications
You must be signed in to change notification settings - Fork 231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Schema: default class field constructors #1997
Comments
@gcanti what's your thought about this? we mentioned a few times the possibility of adding a generic to 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 Schema classes already carry a constructor generic which is controlled by the property descriptors of the struct schema; effect/packages/schema/src/Schema.ts Line 4545 in 10bcbc6
for struct schemas you can also have a createConstructor combinator or so. As to validating constructors for branded types & refinements; you already carry the 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 |
@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 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. |
linked (in top post) naive implementation code updated with some fixes |
@gcanti I've added a general proposal for adding constructors to "bare" non Schema classes in #2443. @mikearnaldi I still think we don't need to carry an explicit
|
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.
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.
Naive implementation;
combinator: patroza@f8a0564#diff-7da43f2b7b3dea1554f2627482ba41e4f885c908848b72704d9887ffcc065153R4675
constructor: patroza@f8a0564#diff-7da43f2b7b3dea1554f2627482ba41e4f885c908848b72704d9887ffcc065153R4557
What alternatives have you considered?
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.
The text was updated successfully, but these errors were encountered: