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

feat: support JSON Schema as input for a Type #1159

Draft
wants to merge 74 commits into
base: main
Choose a base branch
from

Conversation

TizzySaurus
Copy link
Contributor

@TizzySaurus TizzySaurus commented Oct 2, 2024

Closes #729.

What

This PR adds @ark/jsonschema, which is a library enabling users to convert a valid JSON Schema schema into an ArkType type.

API

Below is a basic sample-use of @ark/jsonschema:

import { parseJsonSchema } from "@ark/jsonschema"

const type = parseJsonSchema({type: "string"})
// ^? 
// NB: `type` is correctly inferred as `Type<string>`, and can now be used as-per any other ArkType type

Remaining Work Before Merging

  • For some reason parseJsonSchema({type: "array", items: {type: "string"}}) raises TypeError: this.Schema1Apply is not a function despite working against an earlier ArkType version (probably a bug in ArkType that I've not yet delved into to properly give details).

  • Similar to above, const t = parseJsonSchema({oneOf: [{type: "string"}, {type: "number"}]}) raises TypeError: this.CompositionKeywords1Apply is not a function:
    image

  • Tests are only partially implemented.

    • I wasn't entirely confident on what should/shouldn't be tested, and where things should/shouldn't be tested, so am waiting on feedback for this before finishing the tests.
      • For example, I have tests for {type: "string", "maxLength": 5} and {type: "string", "pattern": "es"}, but not {type: "string", "maxLength": 5, "pattern": "es"}. Is it worth having tests for the various "permutations"? I think this is perhaps overly excessive?
    • All test files also give this linting errors stating it can't find the @ark/jsonschema package:
      image
  • Haven't yet properly filled out ark/jsonschema/CHANGELOG.md.

Known Limitations (things I didn't implement from JSON Schema spec)

  • No type inference (this was removed in 07a5215)
  • No support for JSON Schema dependencies keyword on objects.
  • No support for JSON Schema if/else/then.
    • Sort of not required, since you can write the same (admittedly more verbosely) by combining any/oneOf/allOf/not.
  • multipleOf on numbers only supports integer divisors, due to ArkType only supporting integer divisors.

ssalbdivad and others added 30 commits June 5, 2024 01:23
@TizzySaurus
Copy link
Contributor Author

Type inference removed in 07a5215. Maybe at some point in the future it can be re-added, but agree that for now we should focus on getting a version without inference merged.

I still need to address the rest of your comments but I've run out of time for now so will have to wait. Work's been super hectic recently so haven't been able to work on this as much as I would've liked 😅


- No `dependencies` support
- No `if`/`else`/`then` support
- `multipleOf` only supports integers
Copy link
Contributor

Choose a reason for hiding this comment

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

@ssalbdivad are you open to lifting ArkType's restriction on divisors being non-integers? Or, if not/as an interim solution, it seems this could be added as a custom narrow constraint?

Copy link
Member

@ssalbdivad ssalbdivad Dec 10, 2024

Choose a reason for hiding this comment

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

You can add whatever arbitrary validation you want in a narrow, but the reason we don't support this is because there isn't a straightforward way to test float divisibility without complex + potentially fallible string math.

@TizzySaurus was actually working on an implementation but it doesn't make sense to add a dependency or implement that much custom logic for such a niche case.

For rational values like 0.1, you could testing something like (n: number) => (n * 10) % 1 === 0. Outside that though, things tend to get scary so be careful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I looked into this and it's just not feasible to do floating point division in NodeJS:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

Support JSON-schema as an input format
3 participants