additionalProperties shouldn't affect TypeScript types from JTDDataType about ajv HOT 10 CLOSED

Isn't this what already happens.

Here I have a JTD schema describing an object with an a prop and only that. When I use JTDDataType and try to add b prop it gets the red underline and the error is Object literal may only specify known properties, and 'b' does not exist in type.

Is this what you expect?

from ajv.

Hi @jasoniangreen Thanks for your attention.

Yes, the example and expected tsc error you describe make sense.

The issue I'm describing always involves additionalProperties: true.

I've created a little test file so that you can both build TS and exercise runtime validations. I've added comments for a couple test cases. I hope it's helpful in seeing what I'm pointing out:

https://github.com/jaketrent/demo-ajv-ts-issue

from ajv.

Hi @jaketrent, in case 4 you state:

// no 'may only specify known properties' error
const p2: Person = { name: "Alice", age: 32 }

But in case 4 you have specified that Person has additionalProperties: true so I would expect that you can add an age prop without it being considered an error.

What am I missing?

from ajv.

Hi @jasoniangreen Thanks for coming back to this. I will attempt to clarify a bit.

Case 4 is in contrast to Case 1, where we got a type error when we tried to specify an age on a Person instance.

Still in Case 4: The p1 object is valid. This is great, because additionalProperties: true is set on the schema. And I believe this is the common case (eg, That we don't care about extra fields coming through on network request JSON).

But the p2 line also doesn't TypeError (as is shown useful in Case 1). This is because in Case 4, we're creating a Person type using the JDDataType util. But this goes against the common case, where in one's code, we want stricter types.

So I argue that the JDDataType should by default return a type as if additionalProperties: false is set, whether it's set that way in the schema or not. This would match the use cases of validation of code typing better.

from ajv.

If you're interested, my current best workaround is explained here: https://jaketrent.com/post/strong-type-check-ajv-additional-properties/

from ajv.

Thanks for the very detailed response @jaketrent, I do see your point now.

I'd like to get some more opinions so will tag @erikbrinkman who has implemented a lot of the typescript functionality and of course @epoberezkin.

from ajv.

I think my thoughts here are complicated, and I don't have a consistent way to view this problem.

As @jaketrent says, typescript types are inherently subtractive, that is interface Both { a: number; b: number } can be assigned to interface One { a: number }. Adding properties, or other qualities to a type, subtracts from the space of unknown by specifying that a property must exist, and must have this type. In this sense the idea of adding Record<string, unknown> is antithetical to typescript, because all objects are a Record<string, unknown> inherently.

That being said, typescript is an inconsistent type system, and as you demonstrate they do have different behaviors. In addition, the subtractive behavior is different in once instance, and that's assigning object literals (any maybe const type inference). This is generally the case that the typescript jtd types apply to, since they try to function more like additive types in the first place.

This playground recreates some of what you point out in your examples, namely that if you don't have the record you can't assign object literals with extra properties, or access extra properties without encountering an error in typescript.

As I write this, I think it ultimately comes down to how you view the additionalProperties keyword. Is it:

1. a validation directive, saying to ignore any properties not listed when validating
2. a structural directive, telling you something about the type you're describing, in the same way properties and optionalProperties arguably are

If you're in camp 1, then it shouldn't append the record, if you're in camp 2, then it should. I think in your arguments, you make the case that more people are in camp 1. I'm not entirely sure, but don't really have a pony in that race.

tl;dr

Ultimately, I think I still stand in camp 2, that adding the record is a more faithful reconstruction of the type, and fits with how JTD is structured. I think there are arguments for camp 1, and it's ultimately up to true maintainers e.g. @jasoniangreen or @epoberezkin to decide. Changing it would be breaking though and should probably be reserved for v9 if that's the desired route. I should probably also mention that both types are assignable to one another (as shown in the playground), so it's not too difficult to to pick how you want the type to be checked after the fact. Also, I'm not aware of a nice way to achieve both, that is ti make it user configurable.

@jaketrent let me know if I misinterpreted or got anything wrong

from ajv.

Hey @erikbrinkman. I appreciate the feedback on the idea. Your interpretation seems accurate.

Yes, I think it comes down to what one thinks is the common case for most devs.

I think most devs use ajv-validator for the validation. There, additionalProperties: true is a validation directive, for sure.

But ajv-validator also has this JDDataType util for generating types. When feeding a schema into this util, out comes a typedef, which makes the schema input seem like a structural directive.

So the schema and its directives fulfill both use cases. And this is great. No duplication of the directives is the main reason I chose ajv vs. a competitor. This feature has value, at least to me, and I'm glad it's there, straddling both use cases.

Also agreed that the perspectives are nuanced and that this is a breaking change.

Thanks for your consideration.

from ajv.

Thanks both for your detailed thoughts on this, it's really very useful and interesting. I have spoken to @epoberezkin about this and we're both in camp 2 as well with no desire to change the behaviour. For me it just made sense and was intuitive that given additionalProperties: true that the type would allow them, so much so that it took me a while to understand the issue.

Speaking with EP he also suggest the exact same work around you wrote about @jaketrent in your blog post and his position is that this logic makes sense in the application/consuming codebase rather than within AJV. So on this note I will close the issue, but I am glad it will be discoverable by others asking the same questions as it was very detailed and thoughtful discussion.

Thanks!

from ajv.

That works for me. I'm glad there's a workaround. This tool's great. Thanks for the conversation.

from ajv.