Add a flag to specify 'additionalProperties' value on objects' schemas #1
Conversation
README.md
Outdated
| var all = compile('models.proto'); | ||
| var single = compile('models.proto', 'MyModel'); | ||
| var all = compile('models.proto', true); | ||
| var single = compile('models.proto', 'MyModel', true); |
There was a problem hiding this comment.
Can we make the API compile(filename, model, opts) where opts is an object {additionalProperties: true}.
index.js
Outdated
| module.exports = function(filename, model,allow_additional_props) { | ||
| var compiler = new Compiler(filename); | ||
| return compiler.compile(model); | ||
| return compiler.compile(model,allow_additional_props); |
There was a problem hiding this comment.
Keep in mind that model is optional, so the third argument could become the second. I'd use something like
if (typeof model === 'object') {
opts = model;
model = null;
}There was a problem hiding this comment.
Yeah, this is a problem that can also happen in another method.. I think it's resolve. How about we make the opts solution you described above the 1st argument in all these methods. That way you have an easy way to support other flags later on. Opinion ?
There was a problem hiding this comment.
You mean make model part of the opts? We could, but that would be backward incompatible...
There was a problem hiding this comment.
I meant make this one compile(opts, model) and make all the other function calls also use opts as the first parameter.
index.js
Outdated
| */ | ||
| Compiler.prototype.build = function(type, from, base, key) { | ||
| var res = this.resolve(type, from, base, key); | ||
| Compiler.prototype.build = function(type, from, allow_additional_props, base, key) { |
There was a problem hiding this comment.
Make opts the last argument.
…irst argument in every method so we don't have to do parameter checks in methods
|
Latest commit should address your point. Let me know if you want me to fix anything else. |
|
Hi @devongovett , do you have time for a review of my updated PR ? |
|
Hello @devongovett, I am taking over this PR. Do you need any additional changes from us to merge this PR ? |
The JSON-Schema definition allows for a
additionalPropertiesboolean field to be added to each object definition. It defaults to true but, when false, the validator using the schemas will complain about extraneous fields.My project currently needs this so I've tried to update the code to allow this. At the moment, I've added a flag that will apply to all protobuf messages.
If this PR is ok for you, could you issue a release of the package on the NPM repo ?
Thanks