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

Add typescript types to knockout validation #670

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

SomaticIT
Copy link

Since knockout 3.5 is now publishing Typescript types in its own repository, I suggest to do the same here in knockout.validation.

This types are using the knockout embedded types as a base and extend them to add knockout.validation features.

Linked issue: knockout/knockout#2353

@gregveres
Copy link

Sorry @SomaticIT. I got distracted with getting a feature done and I didn't give you all the comments. I added 4 more comments that I missed the first time I provided comments.

@SomaticIT
Copy link
Author

@gregveres, no problem!
I improved types by adding two utility types ValidationExtendType and ValidationExtendAccessType which allows to standardized the behavior of extenders.

@sevaru
Copy link

sevaru commented Jul 10, 2020

@SomaticIT @gregveres
Any news on merging this PR? :)

@sashaostr
Copy link

sashaostr commented Dec 4, 2020

@SomaticIT @gregveres
please please please merge it!

@SomaticIT
Copy link
Author

Would love to see this merge

@jeremycrippsbf
Copy link

Hi, what's the status of this pull? I took the knockout.validation.d.ts file from these commits but found I had to make some small type parameter changes to make it work with the latest knockout code

Extender now has a parameter
SubscribableFunctions does not have any anymore
ObservableExtenderOptions now has a parameter

@SomaticIT
Copy link
Author

@jeremycrippsbf, do not hesitate to create a patch file, I will integrate it in this PR and give some advices to help users waiting for this PR.

@richardlawley
Copy link
Contributor

I've seen two of the points made by @jeremycrippsbf but don't see the SubscribableFunctions not having a parameter in the current KO source. I also had to add another parent class to ObservableExtenderOptions to lose the compiler error.

Patch below:

diff --git a/dist/knockout.validation.d.ts b/dist/knockout.validation.d.ts
index 5f34394..18e6e00 100644
--- a/dist/knockout.validation.d.ts
+++ b/dist/knockout.validation.d.ts
@@ -394,7 +394,7 @@ declare module "knockout" {
         };
     }
 
-    export interface Extenders {
+    export interface Extenders<T> {
         /**
          * This is for creating custom validation logic on the fly.
          * 
@@ -444,7 +444,7 @@ declare module "knockout" {
         extend(requestedExtenders: validation.ValidationExtendOptions): this & validation.ObservableValidationExtension;
     }
 
-    export interface ObservableExtenderOptions extends validation.ValidationExtendOptions { }
+    export interface ObservableExtenderOptions<T> extends Partial<ExtendersOptions>, validation.ValidationExtendOptions { }
 }
 
 export = ko.validation;

@SomaticIT
Copy link
Author

Hello,

I fixed the knockout validation types based on knockout 3.5.1.
It seems to work well in my simple tests.

Can you confirm?

@richardlawley
Copy link
Contributor

Hi,

Looks good, works in my project when included directly (as does your related PR in knockout.mapping).

Because you don't seem to get much traction getting these PRs merged, might I suggest making equivalent PRs into DefinitelyTyped, so people can make use of these before the library authors respond? If they do merge your PR then the DT definition for that library can be removed, but until that point these become a lot more useful :)

@richardlawley
Copy link
Contributor

Found another problem (I'm currently converting a large codebase!). The message property of a rule can be a string, or a function. The function part isn't shown in the examples, but can be seen here.

I added:

export type ValidationMessageType = string | ((params: any[], observable) => string);

then used this as the type of message on ValidationRuleDefinition and ValidationAsyncRuleDefinition to fix this.

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.

None yet

6 participants