sukovanej / effect-http

Declarative HTTP API library for effect-ts
https://sukovanej.github.io/effect-http
MIT License
222 stars 16 forks source link

Issues resolving OpenAPI components with subset of `Schema.optional` API #471

Closed TylorS closed 3 months ago

TylorS commented 3 months ago

Hello again 👋

I think I've encountered a small bug somewhere within the OpenAPI integration being unable to create "Components" with schemas containing identifiers when some of its property signatures are optional, but only a subset of them.

I think it has to do with the optionalToRequired transformation that takes place internally to Schema, but I'm not too positive after looking only a bit briefly.

import * as S from '@effect/schema/Schema';
import { Api, OpenApi } from 'effect-http';

const schema = S.struct({
  // Does NOT work
  // a: S.optional(S.Date, { nullable: true, exact: true, as: 'Option' }),
  // b: S.optional(S.Date, { nullable: true, default: () => new Date(), }),
  // c: S.optional(S.Date, { exact: true, default: () => new Date() }),
  // d: S.optional(S.Date, { exact: true, as: 'Option' }),
  // e: S.optional(S.Date, { nullable: true, as: 'Option' }),
  // f: S.optional(S.Date, { as: 'Option' }),
  // Does Work
  // y: S.optional(S.Date, { exact: true }),
  // z: S.optional(S.Date),
}).pipe(S.identifier('Foo'));

const api = Api.api({ title: 'Example' }).pipe(
  Api.get('getFoo', '/:foo', {
    request: {
      params: S.struct({
        foo: S.string,
      }),
    },
    response: schema,
  })
);

console.log(OpenApi.make(api));
sukovanej commented 3 months ago

Hey, thank you. Indeed, it was a problem with a transformation which was not considered for components. I didn't check the impl in effect/schema but evidently, in the non working optional cases you provided the whole struct is converted onto a transformation AST node. I have a schema-openapi fix here and I'll update it in here in a moment.

sukovanej commented 3 months ago

Please, try with the latest version.

TylorS commented 3 months ago

@sukovanej The latest build has fixed this issue, thank you!

Screenshot 2024-03-15 at 4 34 04 PM