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

fix: Array type in JSONSchemaType should allow ReadonlyArray too #1473

Conversation

t7yang
Copy link
Contributor

@t7yang t7yang commented Mar 2, 2021

What issue does this pull request resolve?
Currently developers can not use ReadonlyArray in JSONSchemaType like enum, items (for tuple), required, ...

This because in TypeScript, mutable array can assign to read-only array but read-only can not assign to mutable array.

Example

What changes did you make?
Convert every mutable array types to read-only array types in lib/types/json-schema.ts.

Is there anything that requires more attention while reviewing?
No.

@epoberezkin
Copy link
Member

Just wondering - why do we need readonly arrays in schemas?

@t7yang
Copy link
Contributor Author

t7yang commented Mar 2, 2021

Said data like this:

enum Foo {
  a = 'A',
  b = 'B'
}

// FOO can be the value of `enum` in a schema, and also expose to outside world to use
// for some reasons, we freeze this tuple to prevent it from mutation
export const FOO = Object.freeze([Foo.a, Foo.b]);

interface Bar {
  foo: Foo;
}

// currently TypeScript complain about can not assign readonly array to mutable array
const schema: JSONSchemaType<Bar> = {
  type: 'object',
  properties: {
    foo: { type: 'string', enum: FOO }
  },
  required: ['foo']
}

We do have other alternatives like create a new mutable tuple for enum to bypass this issue, but kind of cumbersome work.

The main idea is to enhance the capability of JSONSchemaType to accept both mutable and read-only array.
I think this can make DX better :)

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

Successfully merging this pull request may close these issues.

2 participants