-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
types: implement Schema.discriminator #11855
Conversation
Regarding your comment returning void. Actually by using discriminator we mutate the schema. Maybe it would ne better to actually instantiate a new Schema instance and apply the discriminator on it. Idk. Is it possible to type mutation in typescript? |
applied changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One quick tweak, otherwise LGTM 👍
Schema.prototype.discriminator = function(name, schema) { | ||
this._applyDiscriminators = {}; | ||
this._applyDiscriminators[name] = schema; | ||
this._applyDiscriminators = Object.assign({}, this._applyDiscriminators, { [name]: schema }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need to create a new object every single time. Just Object.assign(this._applyDiscriminators, { [name]: schema });
should be enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit Tests fail if i apply your Suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't see why. But I'll merge this and try it out for myself.
I've not used this before actually, but according to the docs or "JS-docs" the example shows that discriminator will return a model, and it is a model FN. |
Schema.prototype.discriminator = function(name, schema) { | ||
this._applyDiscriminators = {}; | ||
this._applyDiscriminators[name] = schema; | ||
this._applyDiscriminators = Object.assign({}, this._applyDiscriminators, { [name]: schema }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't see why. But I'll merge this and try it out for myself.
Resolves #11837