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

Make default and examples schema options properties type-safe #904

Closed
Not-Jayden opened this issue Jun 17, 2024 · 2 comments
Closed

Make default and examples schema options properties type-safe #904

Not-Jayden opened this issue Jun 17, 2024 · 2 comments

Comments

@Not-Jayden
Copy link

Not-Jayden commented Jun 17, 2024

I was wondering if there's any reason the default and examples properties for schema options couldn't be made type-safe by getting the static type from the first arg.

e.g.

-export interface SchemaOptions {
-    default?: any;
-    examples?: any;
+export interface SchemaOptions<T extends TSchema = TSchema> {
+    default?: Static<T>;
+    examples?: Static<T>[];
     $schema?: string;
     $id?: string;
     title?: string;

This would obviously be a breaking change, but it seems like it should be possible and would be preferable over any.

I'm also not 100% sure on whether examples should be an array or not, that's just how I've seen it used in my experience.

@sinclairzx81
Copy link
Owner

@Not-Jayden Hi, thanks for the suggestion.

Unfortunately, this is considered to be out-of-scope as the default and examples annotations are not required to match that of the type. There are actually cases where external tooling can use these keywords to perform custom processing of a schematic (inclusive of assigning functions that return type values lazily).

There are some notes on this at the following URL

https://json-schema.org/understanding-json-schema/reference/annotations

The default keyword specifies a default value. This value is not used to fill in missing values during the validation process. Non-validation tools such as documentation generators or form generators may use this value to give hints to users about how to use a value. However, default is typically used to express that if a value is missing, then the value is semantically the same as if the value was present with the default value. The value of default should validate against the schema in which it resides, but that isn't required.

So, from the above, making these keywords strictly match the type means that users who need to assign other values there (for the purposes of tooling) would no longer be able to do so. It was generally decided that TB should let any value be assigned.

Hope this brings some insights. Will close off this issue for now, but if you have any follow up questions, feel free to post on this thread.

Cheers
S

@Not-Jayden
Copy link
Author

Not-Jayden commented Jun 17, 2024

@sinclairzx81 Awesome, thanks a lot for sharing that context and for getting back to me so quickly!

Looking at the docs, should the examples type at least be changed to any[] maybe?

The value of this keyword MUST be an array. There are no restrictions placed on the values within the array. When multiple occurrences of this keyword are applicable to a single sub-instance, implementations MUST provide a flat array of all values rather than an array of arrays.

Also skimming the docs further, they also specify for the default property

It is RECOMMENDED that a default value be valid against the associated schema.

and for the examples property

It is RECOMMENDED that these values be valid against the associated schema.

With that in mind, I wonder if it would still be reasonable for it to be type safe by default with a means of opting out of the strict types if needed (maybe via conditional types or overloads), given it should be an exception to the rule that you don't follow the recommendation.

(example playground)

That said I understand this might not be considered overly important for the amount of complexity it adds to support, I just wanted to make sure the decision is well considered.

For some additional context, I'm particularly flagging this because I was considering to start using Value.Create() for mocking purposes inside a large shared codebase, but it feels fragile that defaults are going to inevitably be defined incorrectly.

const numSchema = Type.Number({default: "Psyche this isn't a number"})
const mockNumValue = Value.Create(numSchema); // TS thinks this is a `number` but it's just the default string value

Keen to hear your thoughts :)

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