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

Schema annotations interfaces have been refactored into a namespace Annotations #2289

Merged
merged 6 commits into from
Mar 12, 2024

Conversation

gcanti
Copy link
Contributor

@gcanti gcanti commented Mar 12, 2024

No description provided.

Copy link

changeset-bot bot commented Mar 12, 2024

⚠️ No Changeset found

Latest commit: bbac33f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes changesets to release 10 packages
Name Type
@effect/schema Minor
@effect/cli Major
@effect/experimental Major
@effect/platform-browser Major
@effect/platform-bun Major
@effect/platform-node-shared Major
@effect/platform-node Major
@effect/platform Major
@effect/rpc-http Major
@effect/rpc Major

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@patroza
Copy link
Member

patroza commented Mar 12, 2024

I suppose i'm early to the party but the problem I have with this is

    const Person = S.struct({
      name: S.string,
      age: S.optional(S.number.pipe(S.positive()), { default: () => 0 }).annotations({
        encodedAnnotations: { description: "An optional age." }
      })
    })

    console.log(JSONSchema.make(S.encodedSchema(Person)))
    console.log(JSONSchema.make(Person))

we're loosing the interesting information like exclusiveMinimum:

{
  '$schema': 'http://json-schema.org/draft-07/schema#',
  type: 'object',
  required: [ 'name' ],
  properties: {
    name: { type: 'string', description: 'a string', title: 'string' },
    age: {
      type: 'number',
      description: 'An optional age.',
      title: 'number'
    }
  },
  additionalProperties: false
}
{
  '$schema': 'http://json-schema.org/draft-07/schema#',
  type: 'object',
  required: [ 'name', 'age' ],
  properties: {
    name: { type: 'string', description: 'a string', title: 'string' },
    age: {
      type: 'number',
      description: 'a positive number',
      title: 'number',
      exclusiveMinimum: 0
    }
  },
  additionalProperties: false
}

I think JSONSchema has to be about how to get from the Encoded side to the Type side, they are maybe distinct but also related. The Type rules must match to successfully decode. So it's a combination of Encoded side property descriptors, and Type side schema rules.

The description "An optional age", makes for a distraction. It's a positive number, it's optionality is a property of the property and documented via the required field, which could be sourced from the Encoded part. The same go for the key, it should come from the Encoded part.

@gcanti
Copy link
Contributor Author

gcanti commented Mar 12, 2024

yeah, this was more of an attempt focused solely on annotations, but in fact, I don't think it's worth it

@gcanti gcanti added the schema label Mar 12, 2024
@gcanti gcanti changed the title POC: add support for encodedAnnotations Schema annotations interfaces have been refactored into a namespace Annotations Mar 12, 2024
@gcanti gcanti marked this pull request as ready for review March 12, 2024 16:40
@gcanti gcanti merged commit 3e2038f into main Mar 12, 2024
12 checks passed
@gcanti gcanti deleted the feat/encodedAnnotations branch March 12, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants