Closed YOU54F closed 11 months ago
Looks ok to me. Only thing is, the files in dist
gets built automatically on npm release, so you don't have to commit that. I don't like that we have the compiled code in the repo, but it's something I haven't got around to fixing..
Only thing is, the files in dist gets built automatically on npm release, so you don't have to commit that. I don't like that we have the compiled code in the repo, but it's something I haven't got around to fixing..
Hmmm. yeah I did think that, but thought that wasn't the yak to be shaving right now.
I would also agree that the dist folder isn't relevant in here, we distribute as an npm package that can be executed easily with npx
and users can build from source anyway
Done, I've removed the dist
folder see #41 and also fixed up the bash script that we use for build and release as it was swallowing errors (no -e flag) #40
will pull changes into here and then sort tests out
There is one final case that’s not currently considered in the release above - where additionalProperties is set to an schema.
For example, given this definition:
Pet:
type: object
required:
- name
- photoUrls
additionalProperties:
type: string
properties:
id:
type: integer
format: int64
category:
$ref: '#/definitions/Category'
name:
type: string
example: doggie
this body would be allowed:
"body": {
"someField": "some string"
}
this body would not (foo
is a number, not a string):
"body": {
"someField": "some string",
"foo": 1
}
Clearly, in some cases, this behaviour could lead to dangerous outcomes. In other cases, it could make complete sense - for example, an API that responds with dynamic keys.
This scenario is hard to police, because you can also put in a strict subschema that specifies specifically what fields are and are not allowed.
So for now, I think it’s the right decision to leave this as is, but I wanted to document it against the issue so we have a trace of it.
I've created an example project that shows how to effectively allow additionalProperties: true
if this is truly needed.
…opts.additionalPropertiesInResponse is false
cc @vwong / @mefellows
output
This doesn't fail, against the master build. The https://petstore.swagger.io/v2/swagger.json has been downloaded and modified, to explicitly add
additionalProperties: true
in thePet
schema