Remove all usages of component marked x-internal

[rubypersona]

Is your feature request related to a problem? Please describe.

We're using the remove-x-internal decorator:

decorators:
      remove-x-internal:
        internalFlagProperty: 'x-remove-from-github'

I have a component that looks like this:

x-remove-from-github: true
title: candy-cane
type: object

And another component (let's call it christmas-tree) that references this component:

type: array
items:
  discriminator:
    propertyName: type
    mapping:
      ...
      candy-cane: ../candy-cane.yaml
  anyOf:
    - $ref: ../candy-cane.yaml
    - ...

When we bundle with x-remove-from-github, the resulting spec has candy-cane removed from components (as expected), but it's still referenced in components/schema/christmas-tree, which results in the spec being invalid.

Describe the solution you'd like

Is there a way for all references to the candy-cane component to be removed when x-remove-from-github is applied?

Describe alternatives you've considered

I looked into using decorators, but couldn't find a way to determine whether a given node has a given property, and then delete it from the spec. If there is a way to do this with decorators, some guidance would be appreciated. Thanks!

[jeremyfiel]

How would your discriminator work if you remove the reference to the schema being discriminated?

[rubypersona]

@jeremyfiel is your concern what happens if an API response does contain an object of type candy-cane? That's a valid point to consider. Are there still concerns if we know the API hasn't started returning this type of object yet (let's say that feature is in development and we proactively updated our OpenAPI schema but we don't want to expose this object type publicly yet)?

[jeremyfiel]

If the schema with the discriminator is encountered, no discrimination would be available if the candy cane is removed

What behavior would you expect from that schema without the discriminator working?

[rubypersona]

Oh the schema actually looks like this:

type: array
items:
  discriminator:
    propertyName: type
    mapping:
      candy-cane: ../candy-cane.yaml
      popcorn: ../popcorn.yaml
      cranberry: ../cranberry.yaml
  anyOf:
    - $ref: ../candy-cane.yaml
    - $ref: ../popcorn.yaml
    - $ref: ../cranberry.yaml

[jeremyfiel]

This needs more investigation. Someone will come back to you soon. Thanks for your patience

[rubypersona]

Friendly ping here, thanks!

[tatomyr]

Hmm... The reference is being correctly removed from the christmas-tree component as well as the candy-cane component itself. The only leftover is the reference inside the discriminator mapping.

For example, the original API description:

openapi: 3.1.0
info: {}
paths:
  /test:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/christmas-tree'
components:
  schemas:
    christmas-tree:
      type: array
      items:
        discriminator:
          propertyName: type
          mapping:
            candy-cane: '#/components/schemas/candy-cane' # <-- THIS SHOULD BE REMOVED
            popcorn: '#/components/schemas/popcorn'
            cranberry: '#/components/schemas/cranberry'
        anyOf:
          - $ref: '#/components/schemas/candy-cane'
          - $ref: '#/components/schemas/popcorn'
          - $ref: '#/components/schemas/cranberry'
    candy-cane:
      x-remove-from-github: true
      title: candy-cane
      type: object
      properties:
        type:
          type: string
          enum: [candy-cane]
    popcorn:
      type: object
      properties:
        type:
          type: string
          enum: [popcorn]
    cranberry:
      type: object
      properties:
        type:
          type: string
          enum: [cranberry]

The bundle results in:

openapi: 3.1.0
info: {}
paths:
  /test:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/christmas-tree'
components:
  schemas:
    christmas-tree:
      type: array
      items:
        discriminator:
          propertyName: type
          mapping:
            candy-cane: '#/components/schemas/candy-cane'
            popcorn: '#/components/schemas/popcorn'
            cranberry: '#/components/schemas/cranberry'
        anyOf:
          - $ref: '#/components/schemas/popcorn'
          - $ref: '#/components/schemas/cranberry'
    popcorn:
      type: object
      properties:
        type:
          type: string
          enum:
            - popcorn
    cranberry:
      type: object
      properties:
        type:
          type: string
          enum:
            - cranberry

It's possible to remove the remaining references using custom decorators. There's a similar issue with a decorator example: https://github.com/Redocly/redocly-cli/issues/1671#issuecomment-2301278151 which you might find helpful.

However, I think this could also be fixed in the original remove-x-internal decorator. Let me try that and I'll update you here.

Sorry for the delay!

[tatomyr]

Actually, I think I've fixed that here: https://github.com/Redocly/redocly-cli/pull/1694/files#diff-a1ba985b0b0ab2998bb2679b71cce4267f2103867106142cc73b5c263fd3ab9f Please see the updated decorator. I encourage you to try the new decorator via a custom plugin instead of the existing one.

Example plugin (my-plugin.mjs):

export default function plugin() {
  id: 'test-plugin',
  decorators: {
    'remove-x-internal': RemoveXInternal /* you have to translate TS into plain JS yourself */
  }
}

Your redocly.yaml:

plugins:
- my-plugin.mjs
decorators:
    test-plugin/remove-x-internal:
        internalFlagProperty: 'x-remove-from-github'

Please let me know if that works for you.

[rubypersona]

Hi @tatomyr, thanks for working with us on this. We have tried using the custom plugin as you recommended and it's not quite working yet - here's what we've found so far:

On this line, I think it's supposed to say for (const mapping of Object.keys(parent.discriminator.mapping)) (rather than in), otherwise mapping is an index (like 0, 1, 2) rather than a key in parent.discriminator.mapping.

Once I changed that, we're seeing that, for example, parent.discriminator.mapping[mapping] is #/components/schemas/report-social-media while node[i].$ref is ./report-sec-action-lookup.yaml. Perhaps because our schema components are in different files and we've already partially processed the bundling by this point?

Let us know if there's other troubleshooting we can do. Thanks!

[tatomyr]

Thank you for the feedback @rubypersona!

I think it's supposed to say for (const mapping of Object.keys(parent.discriminator.mapping))

Essentially this is an equivalent to in in that case.

parent.discriminator.mapping[mapping] is #/components/schemas/report-social-media while node[i].$ref is ./report-sec-action-lookup.yaml.

Interesting. Yes, I missed the case when discriminator mapping uses file references. Could you replace enter with leave in the plugin? This should do the trick:

  return {
    any: {
      enter: (node, ctx) => { // <-- replace with `leave`
        removeInternal(node, ctx);
      },
    },
  };

This change, however, makes a couple of e2e tests fail... I'll investigate that later.

[rubypersona]

Hi @tatomyr - good news, replacing enter with leave is definitely helping things; the references that we were hoping to be removed are now gone in the bundled spec.

However, we noticed that there are now some unexpected additions to our bundled OpenAPI spec. Providing an example below:

File: openapi/paths/transaction-types/{transaction-type-id}.yaml

get:
  x-remove-from-github: true
  ...
  responses:
    '200':
      description: OK
      content:
        application/json:
          examples:
            Result:
              summary: OK
              $ref: ../../components/examples/response-bodies/transaction-types/transaction-type.yaml
          schema:
            type: object
            additionalProperties: false
            properties:
              data:
                $ref: ../../components/schemas/transactions/transaction-type.yaml
              included:
                $ref: ../../components/schemas/shared/empty-included-objects.yaml

Since x-remove-from-github: true is on this file, this path is not included in the bundled OpenAPI spec. However, we are now seeing that the bundled spec contains:

...
components:
  examples:
    transaction-type:
      [contents of this example]

whereas previously, this example was not included in the bundled spec, as the only schemas referencing it are marked as x-remove-from-github: true. I have confirmed that all of the schemas that reference this example on our OpenAPI files are marked as x-remove-from-github: true.

Let me know if I can provide any other info. Thank you!

[tatomyr]

However, we noticed that there are now some unexpected additions to our bundled OpenAPI spec.

This what I was afraid of 😅. Let's try a couple of things here. First, you can try running the remove-unused-components decorator and see if it strips the redundant examples.

If that doesn't help, you can check another implementation of the decorator:

const RemoveXInternal: Oas3Decorator | Oas2Decorator = ({
  internalFlagProperty = DEFAULT_INTERNAL_PROPERTY_NAME,
}) => {
  function removeInternal(node: unknown, ctx: UserContext, originalMapping: any) {
    const { parent, key } = ctx;
    let didDelete = false;
    if (Array.isArray(node)) {
      for (let i = 0; i < node.length; i++) {
        if (isRef(node[i])) {
          const resolved = ctx.resolve(node[i]);
          if (resolved.node?.[internalFlagProperty]) {
            // First, remove the reference in the discriminator mapping, if it exists:
            if (parent.discriminator?.mapping) {
              for (const mapping in parent.discriminator.mapping) {
                if (originalMapping[mapping] === node[i].$ref) {
                  delete parent.discriminator.mapping[mapping];
                }
              }
            }
            node.splice(i, 1);
            didDelete = true;
            i--;
          }
        }
        if (node[i]?.[internalFlagProperty]) {
          node.splice(i, 1);
          didDelete = true;
          i--;
        }
      }
    } else if (isPlainObject(node)) {
      for (const key of Object.keys(node)) {
        if (isRef(node[key])) {
          const resolved = ctx.resolve(node[key]);
          if (isPlainObject(resolved.node) && resolved.node?.[internalFlagProperty]) {
            delete node[key];
            didDelete = true;
          }
        }
        if (isPlainObject(node[key]) && node[key]?.[internalFlagProperty]) {
          delete node[key];
          didDelete = true;
        }
      }
    }

    if (didDelete && (isEmptyObject(node) || isEmptyArray(node))) {
      delete parent[key];
    }
  }

  let originalMapping: any = {};
  return {
    DiscriminatorMapping: {
      enter: (node: any) => {
        originalMapping = structuredClone(node);
      },
    },
    any: {
      enter: (node, ctx) => {
        removeInternal(node, ctx, originalMapping);
      },
    },
  };
};

I like this implementation less, as it creates an additional originalMapping entity that could in theory be an artefact from another discriminator. Please note that I haven't had a chance to test it properly yet.

[rubypersona]

Thanks! The remove-unused-components decorator didn't change the bundled spec. For the new implementation, could you clarify where the function definitions should be imported from (e.g. isRef, isPlainObject, isEmptyObject, isEmptyArray)

[tatomyr]

For the new implementation, could you clarify where the function definitions should be imported from

Same as in the original decorator: https://github.com/Redocly/redocly-cli/pull/1694/files#diff-a1ba985b0b0ab2998bb2679b71cce4267f2103867106142cc73b5c263fd3ab9fR1

You should be able to import them from @redocly/openapi-core or @redocly/openapi-core/lib/utils.

[rubypersona]

Thanks, your new implementation is working for us!