Meteor-Community-Packages / meteor-simple-schema

Meteor integration package for simpl-schema
https://github.com/Meteor-Community-Packages/meteor-simple-schema
MIT License
920 stars 162 forks source link

Faulty ruleset when reusing objects with Array types #43

Closed andrenanninga closed 10 years ago

andrenanninga commented 10 years ago

With the creation of a SimpleSchema the used object may be altered causing any creation of SimpleSchema's using the same object to contain a faulty ruleset.

for example:

var example = {
    something: {
        label: 'something',
        type: [String]
    }
}

var schema = new SimpleSchema(example);

now when logging the original example object you'll get this:

{
    "something": {
        "label": "something"
    },
    "something.$": {
        "label": "something",
        "optional":true
    }
} 

I'm guessing this is used by SimpleSchema to handle Arrays as type.

The issue I encountered was when I later reusing this example object (and adding some new fields) that the something.$ was still present.

With this new ruleset the SimpleSchema will no longer accept an Array of strings.

// reusing the example variable
var otherSchema = new SimpleSchema(example);

var valid = otherSchema.newContext().validate({ 
    something: ['hello', 'world'] 
});

valid will be false and the following errors will be present:

[
    { 
        name: 'something.0',
        type: 'keyNotInSchema',
        message: 'something.0 is not allowed by the schema' 
    },
    { 
        name: 'something.1',
        type: 'keyNotInSchema',
        message: 'something.1 is not allowed by the schema' 
    }
]

Now I don't know if this is something known or something that should be fixed. It's something I came across and you should probably be vigilant for.

aldeed commented 10 years ago

@FakeYou, I pushed a fix, but I can't do a new release until they fix some issues with atmosphere. If you want to try it out, you can point to the master github repo. I didn't try out the fix, but I'm cloning the schema argument now, so I assume that will prevent the modifications you mention.

aldeed commented 10 years ago

Also, you could nest/combine the SimpleSchema instances instead of the objects, and that would work around your issue.

aldeed commented 10 years ago

New release with this fix is available now.

andrenanninga commented 10 years ago

Thanks I'll check it out.

I did notice you are using the underscore .clone method which only does a shallow clone so it still may prove a problem with nested schemas.

aldeed commented 10 years ago

I didn't test, but I'm pretty sure nested/combined schemas are fine because it just grabs the internal schemas and merges them, and the internal schemas, in turn, will have been cloned already.