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

Update type definitions #156

Merged
merged 1 commit into from
Jun 15, 2020
Merged

Conversation

shawnmcknight
Copy link
Contributor

This PR makes a few changes to the TS type definitions:

  1. I found that a few type definitions were not being exported and thus were inaccessible. I believe the existing format of how things were being exported was a contributing factor, so I adjusted every type and interface to export at the location of the definition and eliminated the exports at the end of the declaration file. Now everything should be accessible and any new definitions added, provided they follow this pattern, will end up exported without having to add them to the follow-up exported object.
  2. ValidatorConstructorOptions was declared as a type, but it would be better declared as an interface. I have adjusted this type.
  3. The third change I am not 100% sure on, so would love some feedback. In the CheckerFunctionV1 and CheckerFunctionV2 function types, there was a schema parameter which was set as ValidationSchema. When inspecting the parameters passed to the custom function, it did not appear to line up with ValidationSchema and instead was the rule's schema. This seemed to represent what the code was actually doing as it passes rule.schema to these functions. While I definitely think ValidationSchema is incorrect, I'm unsure if what should be passed in this parameter is ValidationRule (which is union ValidationRuleObject | ValidationRuleObject[] | ValidationRuleName) or a subset of that union. In my testing, I only ever saw ValidationRuleObject (single, not array) being passed in, but I'll admit I'm quite unsure of what can actually get passed into the custom function.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 58b37fc on shawnmcknight:improve-types into c6b040a on icebob:master.

@icebob icebob requested a review from erfanium June 15, 2020 13:37
@icebob
Copy link
Owner

icebob commented Jun 15, 2020

@erfanium Could you help with it? I'm not a TS guy.

index.d.ts Show resolved Hide resolved
index.d.ts Show resolved Hide resolved
@erfanium erfanium self-requested a review June 15, 2020 21:06
@erfanium
Copy link
Collaborator

@shawnmcknight Thanks! Change are OK to me!

@erfanium erfanium merged commit 27deb21 into icebob:master Jun 15, 2020
@shawnmcknight shawnmcknight deleted the improve-types branch June 15, 2020 22:50
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

Successfully merging this pull request may close these issues.

4 participants