-
Notifications
You must be signed in to change notification settings - Fork 127
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
Support for functions on sub-docs #497
base: master
Are you sure you want to change the base?
Conversation
…njected via the models .define method during createModel
address: { | ||
street: String, | ||
geoCode: function() { | ||
return this.street; |
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.
So what's the difference with define
? This doesn't seem to allow things that are currently not allowed right?
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.
The difference is that (from my understanding of the doco) define
can only add functions to the top level model object, not sub-docs/sub-objects.
In the case of basicModelThreeSpec
, address
is a sub-object of the model that owns the function geocode
, rather than the function having to be defined onto the model itself.
The changes also apply to arrays of sub-objects by allowing each object to have functions added to them based on the schema definition, as demonstrated on line 113 (addresses being an array of address objects each with a geocode
within the model)
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.
Hum, that's still adding a decent chunk of complexity without providing a new functionality, so I'm not sure it's worth the maintenance effort.
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.
Maybe I'm not explaining it well enough. The changes help address the query at the end of #86 where cur3n4 asks for this:
var User = thinky.createModel("User", {
id: String,
contact: {
email: String,
someMethod: type.method().default(function(someParameter1, someParameter2) {
// More complex logic than this...
return this.email == someParameter1 || this.email == someParameter2;
}
}
});
Aside from defining virtual properties, there is (AFAIK) no way to define a method or function on a sub-document of a model?
Using my changes, the answer to the above would be:
var User = thinky.createModel("User", {
id: String,
contact: {
email: String,
someMethod: function(someParameter1, someParameter2) {
// More complex logic than this...
return this.email == someParameter1 || this.email == someParameter2;
}
}
});
Keen for some feedback on some small additions I made that allow for the attachment of functions to sub-docs. I was porting a codebase from MongoDB to RethinkDB and this functionality was required.
Sub-doc functions can be defined by simply adding then as a member of the object that would otherwise define the schema (as shown in the tests). When a document is instantiated, the '_overrideProperties' method is invoked to define getters/setters for any sub-doc properties such that the prototype can be injected. It does something similar for arrays of sub-docs by hijacking the push method on the attached array.
To support this, I've had to add the '_shadowObject' property to the document, which does mess with validation (which I've accounted for by adding exceptions in validation and ensuring it doesn't make it into the saveableCopy) and ideally I'd like to be able to hide it - but not sure what the best way of doing that is.