types, traits, annotationTypes and securitySchemas in ast.toJSON()

[domarmstrong]

These are presented in the raml as an array of objects with a single key, which is the display name with the value underneath. This wrapper object is a real pain and doesn't need to be there. To get the value I would have to iterate the objects keys just to get to the bit I care about. Any chance of removing it?

[{
  "Rate Limited": {
    "name": "rate-limited",
    "displayName": "Rate Limited",
    "type": [
      "string"
    ],
    "default": "core",
    "example": null,
    "repeat": false,
    "required": true,
    "description": "This end point is rate-limited.\nValue refers to which service this rate-limit fall into.\n"
  }
}]

[{
  "name": "rate-limited",
  "displayName": "Rate Limited",
  "type": [
    "string"
  ],
  "default": "core",
  "example": null,
  "repeat": false,
  "required": true,
  "description": "This end point is rate-limited.\nValue refers to which service this rate-limit fall into.\n"
}]

[sichvoge]

I don't think we should do it since you need to represent the actual keys. What if the displayName is something else then the key name. Remember that the JSON object is used in very different scenarios and some require the key name which is very much different from the displayName.

[sichvoge]

Wait, reading you comment again; are you sure that the key name is the displayName? @ddenisenko can you please confirm and if so why?

[domarmstrong]

It is displayName for annotationTypes, it looks its name for the others. But anyway I can't see any reason that this single key object is helpful. Its more of a hindrance, all the information you need is on the object below, name, displayName etc. How is a wrapper of [{ [name]: { name ... } }] useful? It should either be an object { [name]: { name ... } } or unwrapped [{ name ... }, ...], or a non dynamic key [{ value: { name ... } }].

[sichvoge]

For me it is actually confusing that it is called name and not key since the spec and YAML always speaks about that. We should stick with the terminology used on both to keep a consistent approach. Although I agree that it is obsolete that we keep duplicated data.

As I said, to make sure we reflect the spec terminology and data structures, I'd suggest to have the following:

[{
  "rate-limited": {
    "displayName": "Rate Limited",
    "type": [
      "string"
    ],
    "default": "core",
    "example": null,
    "repeat": false,
    "required": true,
    "description": "This end point is rate-limited.\nValue refers to which service this rate-limit fall into.\n"
  }
}]

[KonstantinSviridov]

@sichvoge For some reason there is displayName used as key. I also think, we should use name instead.

[domarmstrong]

example A

[{
  "rate-limited": {
    "displayName": "Rate Limited",
    "type": [
      "string"
    ]
  }
}, {
  "rate-limited2": {
    "displayName": "Rate Limited2",
    "type": [
      "string"
    ]
  }
}]

Is not nice to work with. The following makes a lot more sense:

Example B

[{
  "key": "rate-limited",
  "displayName": "Rate Limited",
  "type": [
    "string"
  ]
}, {
  "key": "rate-limited2",
  "displayName": "Rate Limited2",
  "type": [
    "string"
  ]
}]

Or just:

example C

{
  "rate-limited": {
    "displayName": "Rate Limited",
    "type": [
      "string"
    ]
  },
  "rate-limited2": {
    "displayName": "Rate Limited2",
    "type": [
      "string"
    ]
  }
}

Working with above, log each types key and type..

A: nasty

types.forEach(type => {
  let key = Object.keys(type)[0];
  let type = types[key].type[0];
  console.log(key, type);
});

B: nice

types.forEach(type => console.log(type.key, type.type[0]));

C: ok

Object.keys(types).forEach(key => console.log(key, types[key].type[0]));

[domarmstrong]

To reflect the yaml it should actually be C, its not an array at all.

types:
  Error: !include types/Error.raml
  ResponseWrapper: !include types/ResponseWrapper.raml
  PositiveInteger: !include types/PositiveInteger.raml

[constantology]

+1 I have to do some horrible hacking around that... :S

[KonstantinSviridov]

Annotations serialization under displayName as key is fixed. Now they go under names.

As for the JSON format itself, it is designed to maintain compatibility with the old RAML 0.8 JS parser.

[sichvoge]

@domarmstrong @constantology whats your take on the compatibility to 0.8? You obviously want to have the same model used for both so that you don't have to do too many checks. Do you guys think it will create problems. I'd imagine yes.

[domarmstrong]

I didn't ever use 0.8 but this json structure is pretty nasty to work with. I would imagine this compatibility could have been handled in code by checking the version. I guess its too far down the line now to be changing structures though.

[kevinrenskers]

I completely agree, the wrapper objects are a pain and in raml2obj I am now taking steps to make the output nicer & more consistent. Would be nice if the parser itself did that :)

[kevinrenskers]

(So yeah, I went with a similar format as your Example B above)

[ashleydw]

I'm not sure if this is related, but it looks like expanded types don't recursively expand.

Example:

types:
  MyType:
    properties:
      id:
        type: number
      boundaries?:
        type: object[]
        description: An array of boundaries
        items:
          properties:
            id:
              type: string
            type:
              type: string
            boundaryAsWkt:
              type: string
              description: The well-known-text value of the boundary.

Expands toJSON():

(snipped):

{ name: 'boundaries',
  displayName: 'boundaries',
  type: [ 'array' ],
  required: false,
  description: 'An array of boundaries',
  items: 'object',
  __METADATA__: { primitiveValuesMeta: { displayName: [Object], required: [Object] } } }

I cannot access the expected propertied under the JSON type (this is actually coming from raml2obj but using 1.1.2 parser and expand(true)).

[ddenisenko]

@ashleydw parser does not expand types, but in this particular case the contents of items should be present in the output, its not an expansion. Posted it as a separate ticket: https://github.com/raml-org/raml-js-parser-2/issues/468

[KonstantinSviridov]

Hi everyone!

We can not modify the old JSON format as we want to prevent our users from compatibility failure.

Your demands have been considered in the new JOSN format design. In comparison with the old format the new one has most maps replaced by arrays in a natural way.

The new format JSON is available by means of load and loadSync methods. Please, refer to the getting started guide for details.

Regards, Konstantin