Moddable-OpenSource / moddable

Tools for developers to create truly open IoT products using standard JavaScript on low cost microcontrollers.
http://www.moddable.com
1.35k stars 238 forks source link

Improve Instrumentation typings #1399

Closed cmidgley closed 2 months ago

cmidgley commented 2 months ago

This PR updates the .d.ts for Instrumentation. It adds the map and name methods, removes the old numeric types for all the instrumentation (which were incorrect depending on the platform/configuration), and adds a new string literal union type with the names of the instruments for use with map. It also allows for number | undefined for the index to nested operations such as Instrumentation.get(Instrumentation.map('Network Bytes Read')) which otherwise would be a type error.

phoddie commented 2 months ago

Thanks for doing this. We'll get it merged.

The list of key strings is a little ugly to manage, since it can vary by host. I guess we can keep that list up-to-date. But it will be possible to pass the TypeScript checks and still have that key be unavailable on a particular host. Not the end of the world (and no easy solution).

cmidgley commented 2 months ago

Might some hosts offer something unique or in addition to this list, or is this list the union of all possible names? It would be a type error if a different name were tried. I only added the list because the prior version had a list of (incorrect) name mappings, so perhaps instead it should just be a string (and not a union of string literal)?

phoddie commented 2 months ago

The current list of strings / names is not strictly the union of all possible names, since hosts can change that. But, I think for current purposes the list is fine as-is. Hosts don't introduce new instrumentation values often. The benefit of having a compile-time validity check on the string literals seems more important.