Skip to content

Commit

Permalink
fix: allow extending only array, not items
Browse files Browse the repository at this point in the history
fixes #404
  • Loading branch information
aldeed committed Sep 8, 2020
1 parent 9100ca2 commit 9891a15
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 6 deletions.
23 changes: 17 additions & 6 deletions package/lib/SimpleSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -488,8 +488,11 @@ class SimpleSchema {
schemaObj = expandShorthand(schema);
}

const schemaKeys = Object.keys(schemaObj);
const combinedKeys = new Set([...Object.keys(this._schema), ...schemaKeys]);

// Update all of the information cached on the instance
Object.keys(schemaObj).forEach((fieldName) => {
schemaKeys.forEach((fieldName) => {
const definition = standardizeDefinition(schemaObj[fieldName]);

// Merge/extend with any existing definition
Expand All @@ -511,7 +514,7 @@ class SimpleSchema {
this._schema[fieldName] = definition;
}

checkAndScrubDefinition(fieldName, this._schema[fieldName], this._constructorOptions, schemaObj);
checkAndScrubDefinition(fieldName, this._schema[fieldName], this._constructorOptions, combinedKeys);
});

checkSchemaOverlap(this._schema);
Expand Down Expand Up @@ -1005,8 +1008,16 @@ function standardizeDefinition(def) {
return standardizedDef;
}

// Checks and mutates definition. Clone it first.
function checkAndScrubDefinition(fieldName, definition, options, fullSchemaObj) {
/**
* @summary Checks and mutates definition. Clone it first.
* Throws errors if any problems are found.
* @param {String} fieldName Name of field / key
* @param {Object} definition Field definition
* @param {Object} options Options
* @param {Set} allKeys Set of all field names / keys in entire schema
* @return {undefined} Void
*/
function checkAndScrubDefinition(fieldName, definition, options, allKeys) {
if (!definition.type) throw new Error(`${fieldName} key is missing "type"`);

// Validate the field definition
Expand Down Expand Up @@ -1034,7 +1045,7 @@ function checkAndScrubDefinition(fieldName, definition, options, fullSchemaObj)
if (SimpleSchema.isSimpleSchema(type)) {
Object.keys(type._schema).forEach((subKey) => {
const newKey = `${fieldName}.${subKey}`;
if (Object.prototype.hasOwnProperty.call(fullSchemaObj, newKey)) {
if (allKeys.has(newKey)) {
throw new Error(`The type for "${fieldName}" is set to a SimpleSchema instance that defines "${newKey}", but the parent SimpleSchema instance also tries to define "${newKey}"`);
}
});
Expand All @@ -1043,7 +1054,7 @@ function checkAndScrubDefinition(fieldName, definition, options, fullSchemaObj)

// If at least one of the possible types is Array, then make sure we have a
// definition for the array items, too.
if (couldBeArray && !Object.prototype.hasOwnProperty.call(fullSchemaObj, `${fieldName}.$`)) {
if (couldBeArray && !allKeys.has(`${fieldName}.$`)) {
throw new Error(`"${fieldName}" is Array type but the schema does not include a "${fieldName}.$" definition for the array items"`);
}

Expand Down
22 changes: 22 additions & 0 deletions package/lib/SimpleSchema.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,28 @@ describe('SimpleSchema', function () {

expect(mainSchema._schema['items.$'].type.definitions[0].type._schemaKeys).toEqual(['_id']);
});

it('can extend array definition only, without array item definition', function () {
const schema = new SimpleSchema({
myArray: {
type: Array,
},
'myArray.$': {
type: String,
allowedValues: ['foo', 'bar'],
},
});

expect(schema._schema.myArray.type.definitions[0].minCount).toBe(undefined);

schema.extend({
myArray: {
minCount: 1,
},
});

expect(schema._schema.myArray.type.definitions[0].minCount).toBe(1);
});
});

it('empty required array is valid', function () {
Expand Down

0 comments on commit 9891a15

Please sign in to comment.