Would this project be open to using TypeScript?

[SamuelTrew]

I work somewhere that has been using a 0.0.5 version (I think) of this library, but without use of const, and we are planning on swapping over to using this package full on.

And as much as the JSDoc comments are super useful, having either a declaration file or full TS support in the files would be super useful.

Would this project be open to me forking and converting it to TS?

Thanks!

[fwextensions]

As it happens, I'm actually converting the QuickScore demo project to TypeScript right now, so I'd have a TS app to test the typings: https://github.com/fwextensions/quick-score-demo/tree/feature/2022/typescript

I've got a set of auto-generated typings that seems to be working, but I haven't pushed a branch for that yet. I'm still new to adding TS to a library, so I'm trying to figure out the best approach. I was thinking of just adding an index.d.ts to the existing JS code, but I'm open to suggestions.

[SamuelTrew]

I'd say that adding an index.d.ts is probably the simplest way of doing it, that would still be very useful for users of this library.
However, for further maintainability for future changes and bug catching, changing the files in general to TS would be most ideal. But I do understand that might not be the easiest.

I'd suggest starting with the index.d.ts and then maybe converting some of the simpler files (e.g. Range) at some later point and go from there.

[SamuelTrew]

I haven't seen the code, but if I could make one request, could it be that the QuickScore class is a generic?

So we could have something like:

class QuickScore<T> {
  // @param {Array<string> | Array<T>} [items]
  items: string[] | T[]
}

This would enable getting the items and using them in a useful way without having to do a cast.
Other places that use object might also benefit from this as well.

Thanks!

[fwextensions]

could it be that the QuickScore class is a generic?

Yup, that makes sense. I'll need to pass along the generic type to the results, so that results[0].item is the expected type. Maybe that'll get automatically inferred, though...

[fwextensions]

I've got a first cut of the declaration file in a branch: https://github.com/fwextensions/quick-score/blob/feature/2022/typescript-declaration-file/src/index.d.ts

If you have any time to play with it, the package from that version can be installed like npm install --save fwextensions/quick-score#feature/2022/typescript-declaration-file. The type of the items array that's used to initialize the object should be passed into the results array as the type for the item key on each object. I believe that should avoid having to cast anything while working with the results, but let me know if I've missed anything.

[SamuelTrew]

Thank you so much!
I've found a few things:

• items doesn't appear to have a type definition in the QuickScore class
• setting the default type to T = string seems to mean that all return types are of type XX | string instead of just XX if a type is given
• When a union type is given (e.g. quickScore: QuickScore<Option | Node>) the return type from searches becomes Option[] | Node[] instead of (Option | Node)[]. I don't know if this is a TS thing or not though as the types seem fine?

[fwextensions]

Thanks for the quick feedback! It's super helpful, as this is the first time I'm trying to create a library with types.

items doesn't appear to have a type definition in the QuickScore class

You mean the items property on QuickScore instances? I didn't include that in the declaration file, as it's intended to be private. Changing it directly instead of calling setItems() won't work, for instance. But I can add it if it's useful for typing.

setting the default type to T = string seems to mean that all return types are of type XX | string instead of just XX if a type is given

Hmm, I would've thought the string default would fall away if the the type is specified as something else. I'm not sure what would happen with just doing new QuickScore() if no items or type is given. Maybe it'll get a type from a subsequent setItems() call? I'll have to play around with it.

When a union type is given (e.g. quickScore: QuickScore<Option | Node>) the return type from searches becomes Option[] | Node[] instead of (Option | Node)[]. I don't know if this is a TS thing or not though as the types seem fine?

Is the goal to have the items be a mix of either of those types, rather than a pure array of one or the other? Maybe QuickScore<(Option | Node)> would work?

[SamuelTrew]

as it's intended to be private

Oh interesting, we use it to just get the items that the quickscore currently has and don't modify it.
If it's meant to be private then you could add a getItems() method maybe? Idk if that's wanted though?

I would've thought the string default would fall away if the the type is specified as something else

same tbh, not sure why it doesn't.

I'm not sure what would happen with just doing new QuickScore() if no items or type is given

In that case I'm pretty sure it just infers the type based on the array given (however, that is with me removing the T = string and changing it to T)

items be a mix of either of those types

This, but for some reason it's returning a union of 2 pure arrays. This might be a TS thing though so not sure

[fwextensions]

Oh interesting, we use it to just get the items that the quickscore currently has and don't modify it.

Why do you need to see which items the instance currently has? I've been assuming most apps would know which items were passed to it, but maybe there's a use case I'm missing.

Reading the items property is fine. I could make it clear in the docs that it should only be modified through setItems(). There are a few other instance variables I removed from the declaration file, but maybe should put back.

This, but for some reason it's returning a union of 2 pure arrays.

According to this article, arbitrary-length arrays have to have a single type:

Both array type literals and Array require all elements to have the same type. That’s how we know that they are for Arrays-as-lists.

So maybe you'd have to create a new type that specifies what's common between those two types? Not really sure, as I'm a relative TS< noob >.

[youssefm]

Just want to mention that I just discovered this package today, really liked it, and went looking for Typescript definitions too. Thankfully, I stumbled on this post, copied over the type definitions you posted, and it's working great so far!

Would really appreciate official Typescript definitions at some point down the road. Thanks for your great work with the library.

[fwextensions]

Great! Sounds like I'm on the right track, then. Hoping to get this out in a release very soon.

[fwextensions]

Here's the latest declaration file: https://github.com/fwextensions/quick-score/blob/feature/2022/typescript-declaration-file/src/index.d.ts

It's working with a TypeScriptified branch of the demo: https://github.com/fwextensions/quick-score-demo/tree/feature/2022/typescript

I think the declaration file is complete, but let me know if anyone sees any issues.

One thing I'm not entirely sure about is the return type of search(). It maps to different types depending on whether the array of items to search is just bare strings or more complex objects. Not sure if this is the best approach:

export type Result<T> = T extends string
	? StringResult
	: ObjectResult<T>;

[SamuelTrew]

Why do you need to see which items the instance currently has

We have conditional logic as to what gets put into quick-score and so we are uncertain at any given time which data is there.

Reading the items property is fine

I think if it's meant to be private then a getItems is better so that people don't think they should modify it directly? I guess documentation should reduce the need for that but you can never know how much people will read 😁

According to this article, arbitrary-length arrays have to have a single type

ah okay nvm then, just stuff on our end then 😁

One thing I'm not entirely sure about is the return type of search()

Yeah that should work well. AFAIK it works just fine and the conditional logic is correct

[SamuelTrew]

Here's the latest declaration file: https://github.com/fwextensions/quick-score/blob/feature/2022/typescript-declaration-file/src/index.d.ts

Ah thank you! I noticed that it is now missing the declare module 'quick-score', but otherwise looks good so far thank you!

[fwextensions]

Yeah, I'm not entirely clear on the declare module stuff. It seemed like it was necessary to get the separate typings file working with the demo app. But if it lives with the rest of the module, simply exporting things like a regular module seems to work.