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

.openapi() Breaking Change #356

Merged
merged 14 commits into from
Nov 10, 2024
Merged

Conversation

samchungy
Copy link
Owner

@samchungy samchungy commented Nov 5, 2024

Resolves #303

.openapi() behaves a little differently now.

It now will attempt to store any previous openapi state and use it.

Why do we need this change?

  1. Unintuitive Extension

    const FooSchema = z.string().openapi({ ref: 'string' });
    
    const BarSchema = z.object({
      a: FooSchema,
      b: FooSchema.openapi({ description: 'foo' }),
    });
    
    createSchema(BarSchema)

    This currently generates the following:

    {
      schema: {
        type: 'object',
        properties: {
          a: { '$ref': '#/components/schemas/string' },
          b: { type: 'string', description: 'foo' }
        },
        required: [ 'a', 'b' ]
      },
      components: { string: { type: 'string' } }
    }

    However, it's clear that the user simply wanted to reuse the FooSchema but describe it as something else later on.

    After:

    {
      schema: {
        type: 'object',
        properties: {
          a: { '$ref': '#/components/schemas/string' },
          b: { '$ref': '#/components/schemas/string', description: 'foo' }
        },
        required: [ 'a', 'b' ]
      },
      components: { string: { type: 'string' } }
    }
  2. Broken .describe() behaviour on registered schemas

    	const FooSchema = z.string().openapi({ ref: 'string' });
    	
    	const BarSchema = z.object({
    	  a: FooSchema,
    	  b: FooSchema.describe('foo')
    	});
    	
    	createSchema(BarSchema)

    This scenario actually causes us to crash because it attempts to register the FooSchema twice with different objects because the .describe() triggers Schema "string" is already registered

    After:

    {
      schema: {
        type: 'object',
        properties: {
          a: { '$ref': '#/components/schemas/string' },
          b: { '$ref': '#/components/schemas/string', description: 'foo' }
        },
        required: [ 'a', 'b' ]
      },
      components: { string: { type: 'string' } }
    }
  3. Unregistered Base Schemas. You would think that if you created a base schema, and used it in a schema with something like a description, we would register it? Right now? No.

    const FooSchema = z.string().openapi({ ref: 'string' });
    const BarSchema = z.object({
      a: FooSchema.describe('foo')
    });
    createSchema(BarSchema)
    {
      schema: {
        type: 'object',
        properties: { b: { type: 'string', description: 'foo' } },
        required: [ 'b' ]
      },
    }

    After:

    {
      schema: {
        type: 'object',
        properties: {
          a: { '$ref': '#/components/schemas/string', description: 'foo' }
        },
        required: [ 'a', 'b' ]
      },
      components: { string: { type: 'string' } }
    }

@samchungy samchungy changed the title .openapi() Breaking Change WIP: .openapi() Breaking Change Nov 5, 2024
@samchungy
Copy link
Owner Author

TODO: Figure out how we're going to calculate previous state

@samchungy samchungy changed the title WIP: .openapi() Breaking Change .openapi() Breaking Change Nov 6, 2024
@samchungy
Copy link
Owner Author

TODO: Add much more tests

@samchungy samchungy mentioned this pull request Nov 7, 2024
@samchungy samchungy marked this pull request as ready for review November 10, 2024 05:02
@samchungy samchungy merged commit 4884081 into master Nov 10, 2024
1 check passed
@samchungy samchungy deleted the openapi-behaviour-breaking-change branch November 10, 2024 05:02
@samchungy samchungy added the breaking change This is a breaking change label Nov 10, 2024
@github-actions github-actions bot mentioned this pull request Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This is a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merging multiple calls to openapi()
1 participant