Skip to content
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

Add array type/array object literal syntax #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lvivier
Copy link
Contributor

@lvivier lvivier commented Feb 10, 2019

@Marak this is a bigger one. This PR extends the array-typed properties in mschema:

{
  var point = {
    "name": {
      "type": "string"
    },
    "coords": {
      "type": "array",      // can use "array" type
      "required": true,     // can be required
      "items": [{           // `items` can be a string, object, or array of strings/objects
        "type": "number",   // if an array, the length has to match and the type(s) of each item
        "min": -90,
        "max": 90
      }, {
        "type": "number",   // each member can have a different type
        "min": -180,        // and different constraints
        "max": 180
      }]
    },
    "tags": {
      "type": "array",
      "minItems": 1,        // can have minimum # of items
      "maxItems": 10,       // and maximum # of items
      "items": {
        "type": "string"    // all items must be of the given type
      }
    }
  };
}

This PR also resolves an issue where an array is accepted as a value for object-typed properties.

  - allows object notation to define array properties
  - `items` constraint for type of array items
  - items can have a single type or fixed-length array of types
  - `minItems` constraint for arrays
  - `maxItems` constraint for arrays
  - array values not accepted for object type
@Marak
Copy link
Contributor

Marak commented Feb 12, 2019

The introduction of the items keyword seems like a major change to the way we are handling arrays and may cause some issues.

At first glance it looks like we might be OK to merge, but I'm going to want to pull locally and perform a bit more testing. Will let you know soon.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants