dancasey / json-schema-default-instance

Creates an object as an instance of the given schema using its default properties.
https://www.npmjs.com/package/json-schema-default-instance
3 stars 1 forks source link

Better default resolving #6

Open alexkuz opened 7 years ago

alexkuz commented 7 years ago

I introduced requiredOnly earlier (https://github.com/dancasey/json-schema-default-instance/pull/1), but now I feel like it was a wrong way. I think there is a way to cover both options without additional flag.

If we explicitly set default value in schema, like this:

{
  "type": "object",
  "properties": {
    "prop": {
      "type": "integer"
    }
  },
  "default": {
    "prop": 1
  }
}

or this:

{
  "type": "object",
  "properties": {
    "prop": {
      "type": "integer",
      "default": 1
    }
  }
}

it should resolve to: { prop: 1 }.

If there is an external schema with default value:

{
  "type": "object",
  "properties": {
    "prop": {
      "$ref": "#/definitions/prop"
    }
  },
  "definitions": {
    "prop": {
      "type": "integer",
      "default": 1
    }
  }
}

it should be ignored and resolved to { }, unless required is set:

{
  "type": "object",
  "properties": {
    "prop": {
      "$ref": "#/definitions/prop"
    }
  },
  "required": ["prop"],
  "definitions": {
    "prop": {
      "type": "integer",
      "default": 1
    }
  }
}

Also, I guess default value for object type should override all property defaults, so:

{
  "type": "object",
  "properties": {
    "prop1": {
      "type": "integer",
      "default": 11
    },
    "prop2": {
      "type": "integer",
      "default": 111
    }
  },
  "default": {
    "prop1": 1
  }
}

should be resolved to { prop1: 1 }.

What do you think?

dancasey commented 7 years ago

Yeah, I think that makes sense. If the default value is set internally, then it makes sense to populate it. Otherwise, there's no reason I can think of to have a default value there.

So if default is local, instantiate it, such that topmost default overrides lower levels. If it's external, only do so if the property is required. I think this will simplify the implementation and use, so 👍