Add propertiesRequired option to Node.js API and CLI #1570 #1621
Conversation
|
| @@ -101,6 +101,7 @@ The following flags are supported in the CLI: | |||
| | `--alphabetize` | | `false` | Sort types alphabetically | | |||
| | `--array-length` | | `false` | Generate tuples using array `minItems` / `maxItems` | | |||
| | `--default-non-nullable` | | `false` | Treat schema objects with default values as non-nullable | | |||
| | `--properties-required` | | `false` | Treat schema objects without `required` as having all properties required. | | |||
There was a problem hiding this comment.
Minor change, and this is a little more verbose, but is a little more descriptive as to what it does.
| | `--properties-required` | | `false` | Treat schema objects without `required` as having all properties required. | | |
| | `--properties-required-by-default` | | `false` | Treat schema objects without `required` as having all properties required. | |
| @@ -28,6 +28,27 @@ describe("transformSchemaObject > object", () => { | |||
| // options: DEFAULT_OPTIONS, | |||
| }, | |||
| ], | |||
| [ | |||
| "basic > options.propertiesRequired: tre", | |||
There was a problem hiding this comment.
nit: not particular about the naming of the test, but it should just describe it鈥檚 not just about setting the flag to true; it鈥檚 about the behavior of true + the required array
| "basic > options.propertiesRequired: tre", | |
| "options > propertiesRequired + required array", |
| given: { type: "object", properties: { string: { type: "string" } } }, | ||
| want: `{ | ||
| string: string; | ||
| }`, |
|
This looks good! I鈥檓 going to fix CI on |
There was a problem hiding this comment.
Thanks so much! If it鈥檚 all right, I鈥檓 going to merge since CI is failing, and there鈥檚 a lint error, but otherwise your PR looks great.
Since your PR allowed this, I think I鈥檒l add a minor change to the flag but keep everything else as-is. Thanks! Will release this in a new release of @next
Changes
Closes #1570.
Adds a
propertiesRequiredoption to the Node.js API and global context and exposes this as--properties-requiredin the CLI.More nuanced description
Schema objects are allowed to have a
requiredproperty that is set to an array of keys inpropertiesthat are considered required. The default forrequiredis[]as specified by https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-validation-00#section-6.5.3 and adopted by openapi via https://swagger.io/specification/#data-types. This may not be the intent of many API designers, so thepropertiesRequiredoption changes the interpretation of section 6.5.3 fromto the following:
How to Review
Pay special attention to the naming of the option and cli flag. I'm open to any better names you can come up with 馃槃 My approach was inspired by the behaviour of
additionalPropertiesanddefaultNonNullable.Checklist
docs/updated (if necessary)pnpm run update:examplesrun (only applicable for openapi-typescript)