Skip to content

Conversation

@mattoni
Copy link

@mattoni mattoni commented Apr 8, 2024

Resolves #23 - I ended up finding some time this weekend to work on this.

This change allows for the conversion of anyOf/oneOf arrays with $ref/obj and a type: "null", which is a common way to do a type union in 3.1.0 to allow for a type to be a $ref, but also nullable. I've tested it on my own rather beefy spec (250+ endpoints and hundreds of types) with zero changes and it passed typecheck. I recently upgraded my spec to 3.1.0 and the conversion tools used by this project were not able to accurately convert the 3.1.0 spec.

Hopefully this is a good enough addition to add, it certainly suits my purposes. I can understand if it should be behind a flag though. Let me know if there are any other situations I may have missed.

Thanks!

*/
export function getRefSchema(node: object, ref: RefObject) {
const propertyName = ref.$ref.split('/').reverse()[0];
if (node.hasOwnProperty('components')) {
Copy link
Member

Choose a reason for hiding this comment

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

this is not a safe assumption; i.e. the $ref could be a reference to a schema in another file, such as
$ref: '../components.yaml/#/components/schemas/foo' in which case fetching schemas[propertyName] may fail or may fetch the wrong schema.
This tool does not resolve external references; see the README which says
"The tool only supports self-contained documents. It does not follow or resolve external $ref documents embedded in the source document."
See https://github.com/apiture/api-ref-resolver for tooling to supports resolving external $ref references before down converting. (However this tool should not assume that all external $ref have been resolved)

So instead, I suggest either checking that the $ref starts with '#/components/schemasor after splitting on '/' thatcomponents[0] === '#' && components[1] === 'components' && components[2] === 'schemas'`

return;
}

const typeNullIndex = entries.findIndex((v) => {
Copy link
Member

Choose a reason for hiding this comment

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

this would be cleaner with

const isNullable = entries.find( (v) => .... );

a: {
oneOf: [
{
$ref: '#/components/b',
Copy link
Member

Choose a reason for hiding this comment

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

should be '#/components/schemas/b'

$ref: '#/components/b',
},
{
$ref: '#/components/c',
Copy link
Member

Choose a reason for hiding this comment

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

should be '#/components/schemas/c'

a: {
oneOf: [
{
$ref: '#/components/b',
Copy link
Member

Choose a reason for hiding this comment

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

should be '#/components/schemas/b'

$ref: '#/components/b',
},
{
$ref: '#/components/c',
Copy link
Member

Choose a reason for hiding this comment

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

should be '#/components/schemas/c'

},
},
};

Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see a test where the schema d has an allOf referencing a , so that d is also tagged as nullable,
or a schema e that has an allOf with type f where f has type: [string, 'null'] so that both f and e are marked nullable

mattoni added 2 commits July 18, 2024 17:31
Lock package versions
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.

$ref with type: null not properly converted

2 participants