Include Typescript types inside repository

[SomaticIT]

Hi wonderful knockout team !

I wrote precise TypeScript types definitions for knockout.
Definitions can be found here: https://github.com/typed-contrib/knockout/blob/master/global/index.d.ts

This project was written for the typings which is shutting down leaving me with two solutions:

1. Merge my definitions with DefinitelyTyped types.
2. Merge my definitions directly inside the source project.

I prefer the second solution because it avoids having multiple dependencies in NPM.
Indeed, TypeScript compiler automatically finds types which are included in NPM packages.

Do you agree to include these types?
If you do, I can open a PR to make the appropriate configuration for you.

Thank you very much for this library which I really make huge use!
You can find most of my knockout related open-source projects here

[brianmhunt]

@SomaticIT — Great work, thanks for posting this.

There's a related issue on tko: knockout/tko#6 — which is already transpiled with Typescript.

@mbest Is there any intention of / ability to include Typescript definitions in ko 3.x?

[SomaticIT]

Thank you for your quick response!
I think I can help if there is any TypeScript migration project for tko.
@mbest Let me know if you want me to open a PR to apply the config and merge my types.

[mbest]

Thanks for sharing your work on this! I'd like to get this into Knockout as well.

[SomaticIT]

Hi @mbest,
I just forked the repo and start working on some improvements to match new typescript capabilities.
I will open a PR soon.
Thanks

[brianmhunt]

@SomaticIT Thanks. What's a Typescript migration project? :)

[IanYates]

As a big Typescript and KO user who long ago took the DefinitelyTyped definitions and updated them (probably around the time components landed) I'd be keen to see your PR and put in our own improvements & tweaks too. I'll keep watch :)

[SomaticIT]

I just submitted a PR with an improved version of typed-contrib types.

[mbest]

I've made some updates to the code from @SomaticIT. See 70b7d7c. Please look over them and let me know if you find any problems. I'm still pretty new to TypeScript, so I may have missed something.

[Sebazzz]

I left some notes 👍

BTW: Has this already landed in latest beta (assuming the beta itself is not too unstable)? If so, I'd like to test these bindings on my real-life project.

[mbest]

@Sebazzz No release yet. That should be coming soon. Thanks for your notes. I'll make some changes based on them.

[SomaticIT]

@mbest I found that AllBindings [L246] interface is not exported but it should be since it is used in BindingHandlers interface which is exported.

It causes some issues in extensions libraries.

[mbest]

Good catch. I'll get that fixed before release.

[michaelaird]

How do you tell Typescript to use the definitions from the knockout npm package instead of types/knockout? (I have both installed because types/knockout-validation still depends on types/knockout )

[mbest]

How do you tell Typescript to use the definitions from the knockout npm package

I don't know. @SomaticIT? Anyone else?

[SomaticIT]

The knockout's package.json is declaring explicitely that it embed its own types:
https://github.com/knockout/knockout/blob/master/package.json#L9

Typescript should use embedded types in priority.
If not, it's a issue, are you using the latest Typescript release?

As a workaround, you sould be able to exclude types/knockout from your compilation in your tsconfig.json:

{
"exclude": [
    "**/@types/knockout/**/*.d.ts"
]
}

I think it should work.
Documentation: https://www.typescriptlang.org/docs/handbook/tsconfig-json.html

[michaelaird]

I believe according to this comment microsoft/TypeScript#11137 (comment) that TypeScript is going to go to @ Types first and the exclude isn't looked at for resolving global declarations...

[mbest]

I discovered a problem with the type definitions I've set up and am wondering if any of you have a solution. If you look at https://github.com/knockout/knockout/blob/master/build/types/knockout.d.ts#L23, we have:

subscribe(callback: SubscriptionCallback<Array<utils.ArrayChange<T>>>, callbackTarget: any, event: "arrayChange"): Subscription;

The problem is that for arrays, T will be actually be X[] and we need to use the X for utils.ArrayChange, but I don't know how extract the "X".

[chrisknoll]

I don't think so, people are saying the type info is only available at compile time.

https://stackoverflow.com/questions/51906554/get-class-name-of-generic-parameter-passed-typescript

They suggest to pass the constructor of the class as a paramter to the type, to help keep that information at runtime, but it feels awfully invasive.

Edit:
Possible approach here:
https://stackoverflow.com/questions/47312116/how-to-get-generic-classt-name-of-typescript

Instantiating a new instance of the provided type automatically, and get the value from the default constructor result.

[mbest]

I asked my question on SO and got an answer pretty quickly: https://stackoverflow.com/a/53973228/1287183

Here's what I did:

type Flatten<T> = T extends Array<infer U> ? U : T;

...

    subscribe<TTarget = void>(callback: SubscriptionCallback<utils.ArrayChanges<Flatten<T>>, TTarget>, callbackTarget: TTarget, event: "arrayChange"): Subscription;

https://blogs.msdn.microsoft.com/typescript/2018/03/15/announcing-typescript-2-8-rc/

[SimmeNilsson]

Hi,
Very nice to have a 3.5.0 now. :)

But when it comes to the included definition, what are the thoughts on the knockout plugins that have typings in DefinitelyTyped?
Often they build upon extending KnockoutStatic which is not present in this definition.
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/knockout.validation/index.d.ts
Also, the types have different names, KnockoutObservable => Observable for example.

Is the preferred solution to write issues on them to include typings compatible with this one in their packages?
Or update DefinitelyTyped to include the changes in 3.5.0?

[mbest]

I think the best solution is to update the Knockout definitions in DefinitelyTyped to match Knockout's own. Then the various plugins should be updated to match the new type names.

[michaelaird]

My $0.02: the name changes are going to be a massive breaking change for everyone using knockout and typescript. I agree that there should be an official d.ts file but I think the naming should more closely match the existing DefinitelyTyped naming to ease the transition.

…

On Sat, Mar 2, 2019 at 8:57 PM Michael Best ***@***.***> wrote: I think the best solution is to update the Knockout definitions in DefinitelyTyped to match Knockout's own. Then the various plugins should be updated to match the new type names. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2353 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AANX6X6WaArzW8UPNOC-GHR8Kqbm06Deks5vSywOgaJpZM4SMXbD> .

[Sebazzz]

I think type aliases might resolve this compatibility issue for now m

[SimmeNilsson]

@mbest The workaround I took was to override the definition and run on definitelytyped for 3.4.2 to have the definitions for the plugins work for now.
Don't know if it is a good long term solution. Do you foresee more features in Knockout 3.x or is it solely maintenance?
Considering DefinitelyTyped, fixing that would be a breaking change. If following semantic versioning justifying a new major version. But that would mean a version 4 while actual knockout is on 3.
Also there are 66 dependents on the definition package.
https://www.npmjs.com/package/@types/knockout

Could a better solution be for the plugins to start including definitions, compatible with 3.5?
But maybe you factored the lack of recent activity on some of those repositories into your conclusion?
We are currently using the types for punches, mapping and validation.
I suppose Knockout.punches could be pretty straightforward, small API surface and you control the repo?
Knockout.mapping a little bit bigger API, but original repo not touched in 5 years. :/
Think this might result in something?
#2457
Knockout.validation, biggest API of the 3, but some changes made to the repo about a year ago. @crissdev we seem to be using your fork for knockout mapping as well. What chances would a pull request, with typescript definition based on definitelytyped but adapted to official knockout definition, have when it comes to making it into an actual npm package in those two repos?

[SomaticIT]

@SimmeNilsson Now that knockout 3.5 is released, I will submit my type definitions for both knockout.validation and knockout.mapping.

Hope it could help for you...

[SimmeNilsson]

@SomaticIT Oh, that would certainly help. Will you target definitelytyped or the packages themselves?

[cosmoKenney]

Can anyone help me out? I just updated my package.json file (using npm-check-updates) to the latest versions of all my packages. This moved me to tyepscript 3.5.1. Knockout 3.5.0. And we rely heavily on knockout-mapping and knockout.validation. But we were using @types/knockout, @types/knockout.mapping and @types/knockout.validation. However my project no longer compiles. I have hundreds of custom validators and mapping references. So I now have hundreds of errors about 'mapping' and 'validation' not existing on 'ko'.
With all the changes to ko and typescript, how to I get typescript to stop producing errors on references to mapping and validation?

[SimmeNilsson]

@cosmoKenney If it is any help, it is not ideal, but I still use the definitelyTyped, even for knockout 3.5.0 itself. Granted I miss out on some newly added functions, but for now I decided the trade off was worth it.
Under tsconfig.json->compilerOptions->paths:
"knockout": [
"node_modules/@types/knockout/index.d.ts"
]

[cosmoKenney]

@SimmeNilsson thanks for the reply. I'm having trouble with the plugins: mapping and validation. I suppose I could include them in the paths option? And then import "knockout.validation"; and import "knockout-mapping"; ??

[SimmeNilsson]

@cosmoKenney
Hi, they both reference the knockout definition and extends its core interface KnockoutStatic. It does not exists in the official one. That is at least why I had the problems you describe.

I have paths for mapping as well. Because it mixes separating the name with dot and dash between the types package and the actual libraries. This is my setup:

npm packages:
@types/knockout
@types/knockout.mapping
@types/knockout.validation
knockout
knockout-mapping
knockout.validation

tsconfig.json paths:
"knockout": [
"node_modules/@types/knockout/index.d.ts"
],
"knockout-mapping": [
"node_modules/@types/knockout.mapping/index.d.ts"
]

imports (typescript)
import * as ko from 'knockout';
import 'knockout.validation';
import 'knockout-mapping';

[cosmoKenney]

@SimmeNilsson thank you!

[ai-fwd]

@SomaticIT / @mbest can you provide some context on why you decided on

export function observable<T = any>(): Observable<T>;

as opposed to

export function observable<T = any>(): Observable<T | undefined>;

the latter is type correct since var name = ko.observable<string>(); is undefined yet you get no warning on name().toString().

the original definitelytyped had the latter and although it's a pain to access an observable since it doesn't know when you've set a value name("ai") it is at least up to the developer to explicitly use the non-null assertion name()!.toString() which also communicates the assumption a value has been set elsewhere.

[mbest]

@ai-fwd I think this is something that will be fixed with #2458.

[roend83]

@SomaticIT do you have type definitions for knockout.mapping and knockout.validation published somewhere?

[SomaticIT]

@ai-fwd You are right. The typing should include T | undefined if no parameter is provided.

[SomaticIT]

I worked on knockout.mapping and knockout.validation types.
You can find two linked PR which includes my work.

The knockout.validation types are stable and could be integrated in knockout.validation.

The knockout.mapping types should be reviewed and tested before being integrated in knockout.mapping. I updated them to use latest TypeScript features but did not have time to full test them. You can find the original typings here: https://github.com/typed-contrib/knockout.mapping/blob/master/index.d.ts

[gregveres]

@SomaticIT First, thank you for providing the types for knockout.validation.
I just installed it and it fixed the errors I was seeing when "extend"ing an observable by adding a validation rule. This now works:
this.settings.name.extend({ required: true });

but typescript is complaining that isValid() is not a property of an observable.

I grabbed a copy of the knockout.validation typing file from the PR on that project that you created. And I have 3.5.0 of Knockout installed with it's types. Hmmm, I noticed that you imported * as ko in the typing file. Is that how you import ko in all of your code? I used to do that with pre-3.5.0 but with 3.5.0 I start to import just the classes / functions that I use.

[SomaticIT]

@gregveres

1. To allow calling isValid() you have to do: this.settings.name = ko.observable().extend({ required: true }) and type name with ko.validation.ValidationObservable type.

2. I imported * as ko because we need to access a lot a knockout features and it's simpler to write.

[gregveres]

@SomaticIT
Thank you very much. That was very helpful. I am finding the new typings to be much cleaner than pre-3.5.0. I think it was a good move forward even though it was a big search and replace through the code base.

I am finding that some of the validations aren't parsing properly. For instance:

public num: ko.validation.ValidateObservable<number>;
...
this.num = ko.observable<number>().extend({ step: 5}); 

this gives an error that it doesn't match any of the 3 overrides of extend with "number is not assignable to RegExp | ValidationRuleExtenderParams"

I get the same thing for equal when I try to say a boolean needs to equal true.

[gregveres]

@SomaticIT
I think the issue with step is a bug in the typing file. The validation rule of step comes right after pattern and it contains the same defintion of pattern, but I think it should be the same defintion of, say, min. I changed my copy to use number instead of RegExp and it now works fine. This occurs in two places in the typings file.

I am looking into another issue where I am using min and max where the values I am comparing against are actually observables. This isn't parsing correctly with the typings.

BTW, is there a better place for me to put these issues?
I am also having problems with the pattern validation rule when I am using the set of options. I haven't looked into that one yet.

and Equal isn't working when I use a fixed value. For instance, I have a boolean where I want valid to be true when the boolean is true and invalid when false. But I get an error when I use: .extend({ equal: true }) because true can't be converted to a Subscribable.

[SomaticIT]

@gregveres

Can you add a comment with a report of bugs you found in the knockout.validation typings PR: Knockout-Contrib/Knockout-Validation#670

I will make an update based on your feedback.

Thanks