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 definitions #191

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

add TypeScript definitions #191

wants to merge 3 commits into from

Conversation

Narazaka
Copy link

I have created TypeScript typedefs for probably almost all of the public APIs. (cf. partial #105 )

I've typed them tightly enough that type completion should work in many places.

image

@colinbdclark
Copy link
Owner

Thanks @Narazaka, it is really kind of you to share a PR for these type definitions. I'll try to take a closer look this weekend and get it merged in.

@Polyterative
Copy link

Any updates on this? I really need types to work with osc in Angular/React

@colinbdclark
Copy link
Owner

Hi @Polyterative, I'm glad to hear you're excited about this PR. I work on osc.js in my spare time, and it's been very busy recently with a residency and recording, so while this is definitely the next feature I will merge in, I'm not really sure yet when I'll have enough free time to look closely at it.

But you can help! Since it sounds like this is an important feature to you, and you're actually a TypeScript user (whereas I am not), I wonder if you'd be willing to take some time to test out @Narazaka's branch and confirm that it is working as expected for you? It would be very helpful, thanks.

@JoueBien
Copy link

JoueBien commented Sep 18, 2022

I'm running with these types and my first notes would be:

Can we expose types for instances?

How these definitions are set up makes it basically impossible to set any variable type with out it being inferred.
To be able to set up a class I have to do:

function UDPPortFunc () {
  return new UDPPort({})
}

class X32 {
  _UDPPort?: ReturnType<typeof UDPPortFunc>
}

and then cast as any because the shape doesn't fit new UDPPort()

Unless I'm missing some obscure way to get the return type off the type for the UDPPort version of PortConstructor?

SS-Deploy pushed a commit to spyscape/osc.js that referenced this pull request Mar 10, 2023
@lachlansleight
Copy link

Hey just a bump here - it looks like this is ready to be merged, the diff is just a little garbled. It's literally just one line that needs to be added to package.json and everything else is new files. TypeScript support is 30 seconds of work from a contributor away ;)

@code-ape
Copy link

code-ape commented Sep 3, 2023

Voicing need here as well for type definitions. Thanks!

@cronin4392
Copy link

Been using this a lot lately. Would love to find out how we can get this merged. I can help out where possible.

@JoueBien
Copy link

JoueBien commented Jan 27, 2024

Having used these types for a year or so I can't say I'd recommend them.
The types don't follow half of the TS suggest practices.
Things that I would recommend changing:

  • the classes constructor's return a types don't match the class definitions.
    • use declare class instead of interface which should fix the above.
  • make the arguments, listener and transport types not depending on a quagmire of <T> passed to <T> (some of these are like 5 levels deep) to get the right shape. Changing this would make it easier to use the types in consuming code + make the intellisense show something actually useful.
    • args shouldn't be so complicated type wise - there's a tone of type inheritance where a much more simple set of types would do & make it much easier to use.
    • .on and .once should have a <T> specked for the returned arg[] in the response. This way you can specify what the listener have arg wise with out having to cast it or unwrap it as "all of the above".

@colinbdclark
Copy link
Owner

Thanks @JoueBien for the confirmation. At some point I hope to work on an API-breaking version of osc.js that fixes some of the issues with the design that have been bugging me for years. At that point I'll try to learn enough TypeScript to produce viable bindings for the library. But honestly, I'm not certain when that will be. In the meantime, if this is important to people in the community, I'm willing to look at pull requests by contributors if they are comprehensive.

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.

7 participants