search

pamelafox / ndjson-readablestream

A small JS package for reading a ReadableStream of NDJSON
MIT License
9 stars 4 forks source link

possible fix for type notations #9

Closed Lucas-Levandoski closed 5 months ago

Lucas-Levandoski commented 5 months ago

I have made this simple PR as a possible solution for typechecking on this service response.

https://github.com/pamelafox/ndjson-readablestream/issues/8

pamelafox commented 5 months ago

Thanks! Is there a tool that I can run to verify that this produces the desired result? Or tests to write? I would like some way to verify the correct behavior of the types.

Lucas-Levandoski commented 5 months ago

well, I have never written any unit tests to cover a generic type checking, I don't even know if it is possible, but I will try. Any help would be appreciated tho

Lucas-Levandoski commented 5 months ago

There are a few typescript libraries that work on runtime, but the typescript itself doesn't seem to be meant to do that anyways, it is a compile time type check, so you can only see the problem on your interface, if you know a way to build a unit test to check that, please let me know and I will apply but I didn't manage to make it work.

I have only managed to check that directly in vs code interface, so you can see the result like this.

image

but thanks to your comment I noticed a problem with my type there, the <T | any> notation doesn't work properly for any, so I have fixed it now.

Lucas-Levandoski commented 5 months ago

May I complete this one ?

pamelafox commented 5 months ago

Yes, this looks good. I read a bit of TypeScript docs and consulted with some other more knowledgable Typescript users, and they agreed with the change. Thanks!

pamelafox commented 5 months ago

Also, btw, if you are using this for a Chat App, you may want to move to https://www.npmjs.com/package/@microsoft/ai-chat-protocol at some point. I'll be updating our Microsoft AI chat app samples to that. But it's more opinionated, so up to you - just sharing as I saw the types in your code look like Chat App types.