Skip to content

Conversation

@jstirnaman
Copy link
Contributor

  • Removes unnecessary allOf.
  • Redundant allOf in discriminators causes some processors (Redoc and Redocly CLI) to
    create unintended implicit schema mappings.
  • For example, for PostCheck In Redoc, this generates the following possible request objects:
    • deadman
    • threshold
    • custom
      and, the unintended implicit objects:
    • Check
    • PostCheck

Redoc before (current prod):
prod-cloud-postcheck

Redoc after:
local-cloud-postcheck

This error from @redocly/cli split alerted me and helped me pinpoint the problem:

/usr/local/lib/node_modules/@redocly/cli/lib/commands/split/index.js:167
    const discriminatorPropSchema = obj.properties[obj.discriminator.propertyName];
                                                  ^

TypeError: Cannot read properties of undefined (reading 'type')
    at implicitlyReferenceDiscriminator (/usr/local/lib/node_modules/@redocly/cli/lib/commands/split/index.js:167:51)

- A nullable property requires a type.
- If the property may use one or more subschemas with different types, make each
  subschema nullable.
- Removes unnecessary allOf.
- Redundant allOf in discriminators causes some processors (Redoc and Redocly CLI) to
create unintended implicit schema mappings.
- For example, for PostCheck In Redoc, this generates the following possible request objects:
  - deadman
  - threshold
  - custom
  and, the unintended implicit objects:
  - Check
  - PostCheck
@jstirnaman jstirnaman requested a review from a team as a code owner January 19, 2023 21:28
@jstirnaman jstirnaman requested review from sanderson and removed request for a team January 19, 2023 21:28
@jstirnaman
Copy link
Contributor Author

@glinton @sanderson

Copy link
Contributor

@glinton glinton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

Copy link
Contributor

@sanderson sanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @jstirnaman!

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.

3 participants