Skip to content

Conversation

@moander
Copy link
Contributor

@moander moander commented Feb 13, 2021

type must be optional to support schemas using the typeKey option and nested paths.

This PR reverts a change that was merged a few days ago:
968bd8a#diff-7aa4473ede4abd9ec099e87fec67fd57afafaf39e05d493ab4533acc38547eb8R1367

type must be optional to support schemas using the `typeKey` option.
moander referenced this pull request Feb 13, 2021
@vkarpov15
Copy link
Collaborator

Unfortunately this change prevents us from throwing an error if the type is incorrect. We'll investigate this further, I'd like to avoid merging this PR as is because of #9857, but we'll see if we can get the best of both.

@vkarpov15 vkarpov15 added this to the 5.11.17 milestone Feb 15, 2021
@vkarpov15 vkarpov15 added the typescript Types or Types-test related issue / Pull Request label Feb 15, 2021
@vkarpov15 vkarpov15 closed this in a485c40 Feb 16, 2021
@vkarpov15
Copy link
Collaborator

I took a closer look and the issue comes down to the fact that the value String is a valid SchemaTypeOptions because SchemaTypeOptions has no required keys. That leaves us in a tough spot with supporting checking incorrect schema definitions where the interface has a path set to number but the Schema has that path set to String while still supporting typeKey, because typeKey isn't necessarily known at compile time. The approach on a485c40 will handle the case where the user uses { type: String } rather than just String, that's the best we can do for now 👍

This was referenced Mar 5, 2021
This was referenced Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

typescript Types or Types-test related issue / Pull Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants