Skip to content

Conversation

@amir-arad
Copy link

also :
update yarn lock
(git) ignore intellij ide environment config files

update yarn lock
(git) ignore intellij ide environment config files
@colinbdclark
Copy link
Owner

Hi @amir-arad, thanks very much for your contribution! I'm always excited when I see new pull requests for osc.js.

I'm not a TypeScript user myself, so without a description or related issue filed, I feel like I'm missing some background information on what this PR provides for users of osc.js. Do TypeScript type definitions have to be part of the repository of the library they annotate, or are there examples of TypeScript definitions being maintained within separate repositories? For example, is it possible to create a new typescript-osc module that depends on osc.js via npm, and contains only the TypeScript-specific information that you've got here? One of my goals for osc.js is to have it be as universally-useful to web developers regardless of the environment or tools they use, but I worry a bit about adding new artefacts that I don't have the knowledge, experience, or automated tests maintain and support, and I wonder if this is perhaps a specialized function that would fit best in its own module somehow?

I also noticed that the type definitions you provided don't seem comprehensive yet. You've covered osc.UDPPort and osc.SerialPort on Node.js, but other Node.js-based Ports such as the WebSocket and TCP-based ports are not defined, nor are any of the Ports defined for other platforms. Are they something you were planning on adding to this pull request at some point?

Again, thank you so much for this pull request and your time preparing it. I'd just love to hear more about the functionality before we decide how best to factor it.

@amir-arad
Copy link
Author

Hi @colinbdclark and thanks for the reply

I admit that this PR was an afterthought. please allow me to provide context:

I've used osc.js in CommaSword/Daedalus for a service which i am now extracting to CommaSword/open-epsilon. When I need to use js libraries that don't offer type descriptions i usually create partial descriptors for them that describes the parts of the API that are relevant to me, as part of my own project files. That's what I did for osc.js, and now I'd like to offer the next user the chance to use these descriptors. I figured it's a nice starter, that can later be brought to cover the entire API if it proves useful to anyone else.

There are many examples of type definitions being maintained on dedicated projects, but also many examples of them being offered as part of an otherwise pure js project. for library consumer it's easiest if they get the types as part of the original project, both because it saves the need to look for them elsewhere, and because there are no version synchronization headaches between the library and its outside descriptor.

The problem of making an outside types definition easily available to the consumers is solved by the
@types namespace in npm (and lately, it seems, a github organization). It used to require cloning a gigantic git repo and writing tests which to me feel a bit pointless as they only have (had?) a very limited potential of catching errors, so I kinda gave up on trying that a year ago (with other type definitions).

So, I totally get it if maintaining a file you're not familiar with is something you wish to avoid, that's what I would feel in your place.

Thank you for this nice and helpful library, btw :)
I'm new to OSC myself, and I found it easy and simple to use.

@amir-arad
Copy link
Author

BTW
I've started awesome-osc because I couldn't find one. I'd appreciate PRs for good stuff that I've missed.

@oveddan
Copy link

oveddan commented Jan 9, 2020

What's the status of this? It would be great to have typescript support!

@colinbdclark
Copy link
Owner

Hi @oveddan,

I'm not a TypeScript user, so not really equipped to test and review this pull request. My understanding is that it covers some of osc.js' functionality but not all of it. I'd happy take pull requests from anyone who wants to polish this work off, update it to the latest version of osc.js, and test it.

Thanks!

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.

3 participants