Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: dereference.preservedProperties for preserving data during dereferencing #369

Merged

Conversation

erunion
Copy link
Contributor

@erunion erunion commented Jan 28, 2025

This introduces a new dereference.preservedProperties configuration that allows for custom $ref dereferencing behavior to happen where if you want to preserve, and prioritize an existing, property you can.

The use case for this is theOpenAPI 3.1 specification where because it allows you to define a description and/or summary alongside a $ref pointer, and require that that description and summary take precedence over any that may be inside of the referenced schema there is currently no way to handle this within the library currently.

For example, the following schema:

{
  required: ['name'],
  type: 'object',
  definitions: {
    name: {
      type: 'string',
      description: "Someone's name",
    },
  },
  properties: {
    name: {
      $ref: '#/definitions/name',
    },
    secretName: {
      $ref: '#/definitions/name',
      description: "Someone's secret name",
    },
  },
  title: 'Person',
}

With the existing dereferencing behavior this schema is dereferenced into the following:

{
  required: [ 'name' ],
  type: 'object',
  definitions: { name: { type: 'string', description: "Someone's name" } },
  properties: {
    name: { type: 'string', description: "Someone's name" },
    secretName: { type: 'string', description: "Someone's name" }
  },
  title: 'Person'
}

However, because in an OpenAPI 3.1 definition I would want to use "Someone's secret name" instead of "Someone's name" on the secretName property, using this new configuration I can do that:

await $RefParser.dereference(schema, {
  dereference: {
    preservedProperties: ['description'],
  }
});
{
  required: [ 'name' ],
  type: 'object',
  definitions: { name: { type: 'string', description: "Someone's name" } },
  properties: {
    name: { type: 'string', description: "Someone's name" },
    secretName: { type: 'string', description: "Someone's secret name" }
  },
  title: 'Person'
}
  1. Why make this configurable instead of the default behavior? Because this seems to be specific to OpenAPI 3.1 and it could potentially be a big breaking change to how people are currently handling these sorts of cases.
  2. This config is generic and has nothing specific to OpenAPI 3.1 though. Correct. This behavior would be handled upstream in consumers of the library (like swagger-parser) and conditionally feed in this config for OpenAPI 3.1 definitions.

Closes #365

lib/dereference.ts Outdated Show resolved Hide resolved
@@ -123,7 +123,27 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
circular = dereferenced.circular;
// Avoid pointless mutations; breaks frozen objects to no profit
if (obj[key] !== dereferenced.value) {
// If we have properties we want to preserve from our derefernced schema then we need
// to copy them over to our new object.
const preserved: Map<string, unknown> = new Map();
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can we wrap this logic and this object allocation inside a check for if derefOptions?.preservedProperties is set? if its not set we can skip a lot of this logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely!

@coveralls
Copy link

coveralls commented Jan 28, 2025

Pull Request Test Coverage Report for Build 13020401245

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 17 of 17 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.6%) to 92.739%

Totals Coverage Status
Change from base Build 13011635299: 0.6%
Covered Lines: 1624
Relevant Lines: 1722

💛 - Coveralls

@jonluca
Copy link
Collaborator

jonluca commented Jan 28, 2025

This lgtm - if you can address the minor comment then I'll merge it in. Thanks!

@erunion
Copy link
Contributor Author

erunion commented Jan 28, 2025

@jonluca I've just pushed up your recs. Thanks for the speedy review! 💨

@jonluca jonluca enabled auto-merge (squash) January 28, 2025 22:06
@jonluca jonluca merged commit ae7d95b into APIDevTools:main Jan 28, 2025
13 checks passed
Copy link

🎉 This PR is included in version 11.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dereferencing behavior on OpenAPI 3.1 schemas
3 participants