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

How to get an object from Type.Object without Symbol(TypeBox.Kind)? #309

Closed
benevbright opened this issue Jan 10, 2023 · 12 comments
Closed

Comments

@benevbright
Copy link

const T = Type.Object({
  name: Type.String(),
});
console.log(T);

result

{
  type: 'object',
  properties: { name: { type: 'string', [Symbol(TypeBox.Kind)]: 'String' } },
  required: [ 'name' ],
  [Symbol(TypeBox.Kind)]: 'Object'
}

Hi,

it's not a big problem but these [Symbol(TypeBox.Kind)] properties give type error in some case.
For example, I'm passing it to Fastify-swagger's schema.

fastify.register(fastifySwagger, {
    routePrefix: "/docs",
    openapi: {
      info: {
        title: "My Service API",
      },
      servers: [],
      components: {
        schemas: {
          User: Type.Object({
            name: Type.String(),
          }) as any               // can't pass without any
        }
    },

Is there any way to omit these properties ? Thanks.

@sinclairzx81
Copy link
Owner

@benevbright Hi.

You can use Type.Strict() to omit symbol properties, but this shouldn't be causing a type error in fastify. It would be best to submit as issue to either the Fastify or FastifySwagger repositories, but out of curiosity can you reproduce the type error at the following link?

https://www.typescriptlang.org/play

Keep me posted
S

@benevbright
Copy link
Author

benevbright commented Jan 10, 2023

@sinclairzx81 Thanks for your support!

Here you go. reproduce sample

import { Type } from "@sinclair/typebox";
import { OpenAPIV3 } from "openapi-types"

// in fastify-swagger: using openapi-types
// source here: https://github.com/kogosoftwarellc/open-api/blob/f5eb28e5a12b110d5ccdf8bcbcb8c66bb13146e8/packages/openapi-types/index.ts#L143
/*
    type SchemaObject = ArraySchemaObject | NonArraySchemaObject;
    interface ArraySchemaObject extends BaseSchemaObject {
        type: ArraySchemaObjectType;
        items: ReferenceObject | SchemaObject;
    }
    interface NonArraySchemaObject extends BaseSchemaObject {
        type?: NonArraySchemaObjectType;
    }
*/

// error
const T2: OpenAPIV3.SchemaObject = Type.Object({
  name: Type.String(),
  city: Type.Optional(Type.Union([Type.Null(), Type.String()])),
});

@benevbright
Copy link
Author

By the way, as you pointed out, the symbol properties didn't seem the original cause. Thanks.

@sinclairzx81
Copy link
Owner

@benevbright Hi,

So, had a look. It seems as though the openapi-types interfaces are a little too strict of additional properties being included on the schema. For example, the following reports a type error (although I'd expect it to support additional properties of this kind so long as they do not permit invalid known properties)

import { OpenAPIV3 } from "openapi-types"

const T2: OpenAPIV3.SchemaObject = {
    foo: 'bar' // error: not permissive of additional properties
}

You can work around this by using the as keyword.

const T2 = Type.Object({
  name: Type.String(),
  city: Type.Optional(Type.Union([Type.Null(), Type.String()])),
}) as OpenAPIV3.SchemaObject // use an `as` assertion here. 

It might be worth while submitting an issue on the openapi-types project and just asking them if this was intended. I should note that it is quite common to augment schemas with additional associative metadata, and as far as TypeBox's Kind and Modifier symbol properties are concerned, these are non serializable properties (so would never be included in some publishable OpenAPI schematic)

I'd be interested to hear their thoughts, did you want to submit an issue and link back to this issue?

@sinclairzx81
Copy link
Owner

@benevbright Actually, had a bit more of a look into this....There's something unusual happening with the way openapi-types is handling arrays. I can reproduce the error with the assignment of union type with a single variant.

import { Type } from '@sinclair/typebox'
import { OpenAPIV3 } from "openapi-types"

// Ok
const A: OpenAPIV3.SchemaObject = Type.Union([Type.String()])
// Ok
const C: OpenAPIV3.SchemaObject = Type.Union([Type.Boolean()])
// Error: Type 'TUnion<[TNumber]>' is missing the following properties from type 'ArraySchemaObject': type, items
const B: OpenAPIV3.SchemaObject = Type.Union([Type.Number()])
// Error: Type 'TUnion<[TNull]>' is missing the following properties from type 'ArraySchemaObject': type, items
const D: OpenAPIV3.SchemaObject = Type.Union([Type.Null()])

Looking through their definitions, I can't see anything obvious standing out, but the above success and failure cases are curious, especially as the error is reporting that ArraySchemaObject is being flagged for a anyOf schema.

@benevbright
Copy link
Author

benevbright commented Jan 11, 2023

// in openapi-types lib
type SchemaObject = ArraySchemaObject | NonArraySchemaObject;
interface ArraySchemaObject extends BaseSchemaObject {
        type: ArraySchemaObjectType;
        items: ReferenceObject | SchemaObject;
    }
    interface NonArraySchemaObject extends BaseSchemaObject {
        type?: NonArraySchemaObjectType;
    }
    interface BaseSchemaObject {
        title?: string;
        description?: string;
        format?: string;
        ...
        anyOf?: (ReferenceObject | SchemaObject)[];

hm... it's openapi-types code.
Here, interface ArraySchemaObject extends BaseSchemaObject This seems wrong? So ArraySchemaObject could also have anyOf property so it doesn't know which to use between ArraySchemaObject | NonArraySchemaObject

@sinclairzx81
Copy link
Owner

@benevbright Yeah, that's pretty much what I saw too :(

Unfortunately, I can't think of a solution to this issue that would be truly ideal. In openapi-types, there may be reasons why things are structured the way they are, and I got a sense reading through their code they had some technicalities differentiating Array schematics, so not sure what the issue was there....

I will say, I feel the more "user friendly" solution here would be for openapi-types to be permissive of additional properties on the schemas (which should solve any problems with these symbols); but to achieve in their codebase, it would likely require a sizeable refactor of what they have currently. On the TypeBox side, it needs these symbols for type composition, inference and conditional runtime types, so it's a little bit of a impasse....

I guess you could ask the openapi-types developers about allowing [key: string | symbol]: unknown on their existing types (presumably BaseSchemaObject) as well as this Array TS issue, but in lieu of this, you should be able use the as keyword when binding to OpenAPI properties.

const OK = Type.Union([Type.Null(), Type.String()]) as OpenAPIV3.SchemaObject

It's not much of a solution, but I do hope this helps. Will leave this issue open for a day or so. If you do want to ping them an issue though, feel free to link back to this thread.
Cheers
S

@benevbright
Copy link
Author

Ok, actually the symbol is not a problem anymore. It was my bad.

The problem is the fact that OpenApi doesn't support type: "null". And I think you've commented about it before #116 (comment)

indeed there is no null type: https://swagger.io/docs/specification/data-models/data-types/#null

// and also in openapi-types lib
    type NonArraySchemaObjectType = 'boolean' | 'object' | 'number' | 'string' | 'integer';
    type ArraySchemaObjectType = 'array';
    type SchemaObject = ArraySchemaObject | NonArraySchemaObject;

I didn't realize that openApi doesn't follow JSON schema.

@benevbright
Copy link
Author

benevbright commented Jan 11, 2023

Ah ok. "null" type seems back in OpenAPI3.1 and it's compatible with JSON schema so it should be fine soon.. they would update it I guess! (EDIT: they've done long time ago, I guess then fastify-swagger didn't..)
https://stackoverflow.com/a/48114322/949795

https://www.openapis.org/blog/2021/02/16/migrating-from-openapi-3-0-to-3-1-0

@benevbright
Copy link
Author

benevbright commented Jan 11, 2023

fastify/fastify-swagger#639
Yeah.. fastify-swagger hasn't updated it with OpenApi 3.1 yet. Thanks a lot for your support. And sorry about for having an issue here. 🙏♥️

@benevbright
Copy link
Author

import { Type } from "@sinclair/typebox";
import { OpenAPIV3, OpenAPIV3_1 } from "openapi-types"

// error
const T2: OpenAPIV3.SchemaObject = Type.Object({
  name: Type.String(),
  city: Type.Optional(Type.Union([Type.Null(), Type.String()])),
});

// ok
const T3: OpenAPIV3_1.SchemaObject = Type.Object({
  name: Type.String(),
  city: Type.Optional(Type.Union([Type.Null(), Type.String()])),
});

@benevbright
Copy link
Author

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

No branches or pull requests

2 participants