.addListeners() passed parameter in typescript

[glend1]

I am trying to add some listeners to my code. connected, disconnected, noteon and noteoff. in v2.5 i was doing the following for a noteon event.

input.addListener('noteon', (e) => {
    let data = normalizeMidi(e.note.number, sharp)
    start(data)
});

I am using Typescript and it tells me Parameter 'e' implicitly has an 'any' type. This wasn't an issue before, Typescript understood what the callback was being passed. for connected we had WebMidiEventConnected and for disconnected we had WebMidiEventDisconnected. I don't have noteon or noteoff documented in my code (I might downgrade to find out).

These are typescript errors exlusively as the code beind them seems to work.

Sidenote: Without changing any of my code (from 2.5 to 3) the noteoff listener doesn't seem to work. When I release a midi key another noteon event is triggered

EDIT: the event sent to noteon was InputEventNoteon and the event for noteoff was InputEventNoteoff

[djipco]

I am using Typescript and it tells me Parameter 'e' implicitly has an 'any' type.

With version 3, instead of maintaining a static .d.ts file, I am relying on the TypeScript compiler's new ability to generate declaration files from the jsdoc comments in the source JavaScript files. However, your issue revealed that there is a bug in the way the compiler handles custom types scoped to a class.

Given the problems so far, I am seriously thinking of going back to manually editing the declaration file. I would really prefer to generate it dynamically though... I'll wait a little to see if the TypeScript team can move soon on the issue.

[djipco]

I did try your code with v3.0.6 and it works for me without complains. Maybe you are using a stricter TypeScript mode? I'll publish v3.0.6 in a minute so you can check if it makes a difference.

[glend1]

Tried the new version and it didn't fix the issue, I am using "noImplicitAny": true, in my typescript config.

[djipco]

The "noImplicitAny": true option is definitely at play here. By default, it's set to false. That's why I was not getting any errors on my side. If you disable the option, does it work like it does for me? If it does, perhaps you could leave it disabled while I wait for an answer from the TypeScript team regarding the aforementioned bug. Should they not get back to me in a timely manner, plan B is to tweak the .d.ts by hand. How does that sound?

[glend1]

It is a combination of "strict": true, and "noImplicitAny": true,. However, I also have eslint installed and that is giving me a warning for it (i think, might still be tsc)

[djipco]

The TypeScript team got back to me yesterday but it's unclear whether this is going to be fixed or not. I guess we will have to wait a little more.

Meanwhile, if you disable noImplicitAny and strict are you able to work?

[glend1]

it still compiles with the errors, i guess i will just wait. might have more issues. ill make another thread as i find them.

[djipco]

Can you try the new declaration file that's included in v3.0.8 and tell me if it helps?

[glend1]

Kind of, the red error line is gone, but typescript are recognizing them as any which means you don't get IntelliSense style code completion (or any typescript benefits)

[djipco]

typescript are recognizing them as any which means you don't get IntelliSense style code completion

Yes, that is right. I am extending the EventEmitter class of the djipevents library for all classes that dispatch events. This library allows you to pass any type of argument when adding a listener. So, the type any actually matches reality.

Having said that, in the extending classes from WEBMIDI.js I do know the actual types used. So, I actually intend to create all those event types and assign them properly in the specific context of the library. I just haven't had the time yet. There are over 50 types of events that I need to sort through and dynamically insert into the jsdoc (because of the problem with the TypeScript compiler not supporting namespaced properties). This is on my todo list. However, it should not prevent you from using the library. You will just have to refer to the documentation more often.

[glend1]

are you aware of generics?

[djipco]

I will check it out. Thanks.

[jjeff]

I've got a Typescript app and I'm attempting to update from v2 to v3 as well. I'm hitting a similar problem. I could force the types, but I can't seem to figure those out either. Maybe someone could push me in the right direction.

input.addListener('noteon', (midi) => {
  midi = midi as Message
  // ^^ this doesn't seem to work... what should the type be?
  const note = midi.note.number
  const channel = midi.channel
  const id = midi.target.id
})

I'd love to see the Typescript compatibility come together and I'd be happy to help if I can.

[glend1]

The types aren't exported. Any is the correct way that go for now.

[jjeff]

Oh, my linter is going to be very unhappy about that. 🙂

Maybe I'll just wait 'til this gets sorted before I move to v3. I've got 20K lines of code in my project with "noImplicitAny": true and it's a comfort to know that VSCode will help me to correctly navigate complex data structures (like midi.note.number).

All that being said, I'll reiterate my interest in helping to implement the needed type exports... and/or convert the whole module to Typescript! 😉

[djipco]

Just to be clear, any is not assigned implicitly. It is assigned explicitly because this is actually how the djipevents library works. In other words, the "noImplicitAny": true configuration does not apply here. The problems is simply that you do not have event types to cast the incoming event to.

I am committed to supporting TypeScript but I haven't worked on the event types just yet. Since v3, the .d.ts file is being generated dynamically and many changes had to be done to get everything working smoothly.

If I can count on your help, it shouldn't be too hard to get it done right. Basically, if you had a little time to test the types before I release them, that would be awesome. I did not have any TypeScript beta testers. This may explain why early TypeScript support has been a bit wonky.

I'll keep you posted.

[djipco]

Question: is it better for TypeScript developers to have one separate type for each dispatched event object or can they be combined? For instance, both Input and Output dispatch connected events. These events have exactly the same properties. Do you need both OutputConnectedEvent and InputConnectedEvent ?

In fact, a generic Event type would suffice for many events that only contain timestamp, type and target properties (e.g. opened, closed, disconnected, midiaccessgranted, enabled, disabled, portschanged, connected). If I only created an Event type for all those, would that be problem? I guess you wouldn't know which event matches which type.

A few events contain their own particular properties (controlchange, keyaftertouch, rpn, nrpn, etc.). Is it better to create a specific event for all those or to add optional properties to a more generic type of event which would be shared?

Do you have examples of how it's done in well-known libraries? I want to do it right.

Cheers!

[glend1]

I am relatively new to typscript myself. But i can certainly share my opinions.

Do you need both OutputConnectedEvent and InputConnectedEvent ?

No, I think typescript doesn't complain if you were to pass something that wanted an input an output as long as the types match. I think it does a deep comparison to see if it can be cast then says gives it the "ok". (I might actually test this, since this seems broken)

Is it better to create a specific event for all those or to add optional properties to a more generic type of event which would be shared?

I think I have seen some libraries that do this both ways. Im not sure there is a "best practice" in the "community".
personally, I don't like doing nullchecks, I probably should do them more. Although, in my mind, one of the benefits of typescript is that you don't have to do nullchecks.
If your going to return a type I would prefer it is fixed. so i don't have to check which i've got back.

[djipco]

No, I think typescript doesn't complain if you were to pass something that wanted an input an output as long as the types match.

That was my understanding too. However, from the developers perspective, it might get confusing. For instance, I could create a generic Event type (closed, disconnected, etc.), a more narrow MessageEvent with extra properties (noteon, noteoff, etc.) and an even narrower ControlChangeMessageEvent with, again, extra properties. Would it be clear for developers which type they should cast the objects to?

[glend1]

Idealy we don't explicitly cast. Idealy we use what is returned.

If that type has a .controlChange type that should be accessesable by default.

I will test passing identically typed objects tomorrow and see if typescript wants to give me an error. functionally if typescript doesn't complain i don't see the need to add complexity.

[djipco]

Idealy we don't explicitly cast. Idealy we use what is returned.

I'm not sure I understand. When you use addListener(), you can listen to a variety of events. The object passed to the callback function may be of various types depending on the event you are listening for (this is why I must use any here). For instance, when you are listening for a controlchange event, you will not get the same type of object in the callback as when you are listening for a disconnected event.

So, if you do not explicitly cast, how can TypeScript know ahead of time which type to expect?

[glend1]

It doesn't. It infers the type based on usage.

Besides, when we add a listener we aren't saving a reference to that listener (and even if we were typescript would infer it's type.

In other words we don't need to import an onclick type when using onclick events.

[djipco]

There are 4 classes that dispatch events: Input, InputChannel, Output and WebMidi. I went through them all and came up with 6 different event types.

Below, you will find, what I plan to add to the .d.ts file. Does that make sense to you? Perhaps you could append it to your own .d.ts and tell me if it solves your issues? Comments welcome. Thanks.

P.S. I put in comments which classes dispatch what.

/**
 * Input:         closed, disconnected, opened,
 * InputChannel:  -
 * Output:        closed, disconnected, opened
 * WebMidi:       connected, disabled, disconnected, enabled, midiaccessgranted, portschanged
 */
export interface Event {
  timestamp: DOMHighResTimeStamp;
  type: string;
  target: any;
}

/**
 * Input:         -
 * InputChannel:  -
 * Output:        -
 * WebMidi:       error
 */
export interface ErrorEvent extends Event {
  error: any;
}

/**
 * Input:         activesensing, clock, continue, midimessage, reset, songposition, songselect,
 *                start, stop, sysex, timecode, tunerequest, unknownmessage
 * InputChannel:  allnotesoff, allsoundoff, midimessage, resetallcontrollers, channelaftertouch,
 *                localcontrol, monomode, omnimode, pitchbend, programchange
 * Output:        -
 * WebMidi:       -
 */
export interface MessageEvent extends Event {
  message: Message;
  value?: number | boolean;
  rawValue?: number;
}

/**
 * Input:         -
 * InputChannel:  controlchange, controlchange-controllerxxx
 * Output:        -
 * WebMidi:       -
 */
export interface ControlChangeMessageEvent extends MessageEvent {
  controller: {
    number: number;
    name: string;
  };
  subtype?: string
}

/**
 * Input:         -
 * InputChannel:  noteoff, noteon, keyaftertouch
 * Output:        -
 * WebMidi:       -
 */
export interface NoteMessageEvent extends MessageEvent {
  note: Note;
}

/**
 * Input:         -
 * InputChannel:  nrpn, nrpn-databuttondecrement, nrpn-databuttonincrement, nrpn-dataentrycoarse,
 *                nrpn-dataentryfine, rpn, rpn-databuttondecrement, rpn-databuttonincrement,
 *                rpn-dataentrycoarse, rpn-dataentryfine
 * Output:        -
 * WebMidi:       -
 */
export interface ParameterNumberMessageEvent extends MessageEvent {
  parameter: string;
  parameterMsb: number;
  parameterLsb: number;
}

[glend1]

Not sure if i updated the files correctly. I added them to both the cjs and esm files at the top and nothing appeared to change. they seem to make sense though, i don't know.

maybe the reason why just appending the file isn't fixing it is because those types aren't defined at the implimentation?

did you drop event.port in v3? its a small code change that i will have to make if these work.

EDIT: also this is valid typescript (apparently).

type Input = { value: string }
type Output = { value: string }
function test(value: Input) {console.log("print")}
const pass: Output = { value: "hello world"}
test(pass)

so to answer your earlier question about if everything should have its own type im gonna say no. as long as the types definitions are the same it doesn't matter what they are called.

[djipco]

I added them to both the cjs and esm files at the top and nothing appeared to change.

Nothing was supposed to change but you should now be able you to use the following:

input.addListener('controlchange', e => {
  e = e as ControlChangeMessageEvent;
})

[jjeff]

I'm just riffing here. But I've used a method similar to this article to create a Typescript interface that acts as a map between event types and the listener function. Then we use variable types to switch the second argument of addListener(firstArg, secondArg) based on the first. I'm not quite sure if/how you'd convert this to Javascript comment syntax, but that would be the next step.

import { Message, Note } from "../dist/cjs/webmidi.cjs";

/**
 * Input:         closed, disconnected, opened,
 * InputChannel:  -
 * Output:        closed, disconnected, opened
 * WebMidi:       connected, disabled, disconnected, enabled, midiaccessgranted, portschanged
 */
 export interface Event {
  timestamp: DOMHighResTimeStamp;
  type: string;
  target: any;
}

/**
 * Input:         -
 * InputChannel:  -
 * Output:        -
 * WebMidi:       error
 */
export interface ErrorEvent extends Event {
  error: any;
}

/**
 * Input:         activesensing, clock, continue, midimessage, reset, songposition, songselect,
 *                start, stop, sysex, timecode, tunerequest, unknownmessage
 * InputChannel:  allnotesoff, allsoundoff, midimessage, resetallcontrollers, channelaftertouch,
 *                localcontrol, monomode, omnimode, pitchbend, programchange
 * Output:        -
 * WebMidi:       -
 */
export interface MessageEvent extends Event {
  message: Message;
  value?: number | boolean;
  rawValue?: number;
}

/**
 * Input:         -
 * InputChannel:  controlchange, controlchange-controllerxxx
 * Output:        -
 * WebMidi:       -
 */
export interface ControlChangeMessageEvent extends MessageEvent {
  controller: {
    number: number;
    name: string;
  };
  subtype?: string
}

/**
 * Input:         -
 * InputChannel:  noteoff, noteon, keyaftertouch
 * Output:        -
 * WebMidi:       -
 */
export interface NoteMessageEvent extends MessageEvent {
  note: Note;
}

/**
 * Input:         -
 * InputChannel:  nrpn, nrpn-databuttondecrement, nrpn-databuttonincrement, nrpn-dataentrycoarse,
 *                nrpn-dataentryfine, rpn, rpn-databuttondecrement, rpn-databuttonincrement,
 *                rpn-dataentrycoarse, rpn-dataentryfine
 * Output:        -
 * WebMidi:       -
 */
export interface ParameterNumberMessageEvent extends MessageEvent {
  parameter: string;
  parameterMsb: number;
  parameterLsb: number;
}

export interface InputEventMap {
  'songposition': (event: MessageEvent) => void;
  'songselect': (event: MessageEvent) => void;
  'start': (event: MessageEvent) => void;
  // etc
  'clock': (event: MessageEvent) => void;
  // etc
  'noteon': (event: NoteMessageEvent) => void;
  'noteoff': (event: NoteMessageEvent) => void;
  'controlchange': (event: ControlChangeMessageEvent) => void;
}

class Input {
  constructor() {}
  addListener<T extends keyof InputEventMap>(event: T, listener: InputEventMap[T]): void {
    // ...
  }
}

const input = new Input()
input.addListener('noteon', (message) => {
  message.note.number; // <-- all autocompleting and types are correct
})

[djipco]

Thank you so much Jeff. This looks like the right way to do it but I'm not sure if this can be done directly in jsdoc. I'm going to try it out and I'll keep you guys posted.

[jjeff]

The more I think about it, the more manually maintaining some .d.ts files feels like it's probably the way to go. I have no idea how you'd get these dynamic types into JSDoc. It's worth a try, but you don't get much/any syntax/autocomplete help when typing JSDocs... but if you put all of this into .ts files, I would think it'd be comparatively easy to maintain. Also, once you've got the .d.ts files, you should be able to reference those types in your JSDoc comments.

IIRC, you can actually do JSDoc comments with import statements like this:

/**
  * @param event {import('./Interfaces').NoteMessageEvent} The event type to listen to.
  */
addListener(event) {
  // ...
}

I think that would work. I haven't fully tested it. But I know I've done stuff like that before.

[djipco]

I'm probably going to simply inject them in the auto-generated .d.ts file. This way I can keep my jsdoc in sync and TypeScript-specific stuff will be isolated.

[jjeff]

The way I'm describing it, you kind of get the best of both worlds. You can define the custom interfaces in some files in your /src directory. Then you reference them in the JSDoc. Then those interfaces should be output in the auto-generated .d.ts. So you won't need to edit the auto-generated files or worry about your code getting overwritten when the files get generated again. But ultimately you should do whatever works! If it works, it's certainly good enough for me. 😉

[djipco]

Okay... I got fed up of fighting and I decided to manually put together a new .d.ts file. Could you give it a try before I release it. You can get it from here.

[glend1]

Not sure it worked. I put it in /node_modules/webmidi/

[djipco]

Sorry. I should have explained it better.

You need to replace the content of /dist/esm/webmidi.esm.d.ts and/or /dist/cjs/webmidi.cjs.d.ts (depending which flavour you use) with the content of /typescript/webmidi.d.ts.

[jjeff]

Ah ha! Once, I copied and renamed it to /dist/cjs/webmidi.cjs.d.ts it is now working! Or I guess I could have copy/pasted the content like you suggested. 🙂

There is still not a type for WebMidi.addListener()... but technically that's a different issue.

Also, NoteMessageEvent no longer seems to contain the event.channel value... It now appears to be at event.message.channel. But again... different issue... and also, I know about this because Typescript told me about it. So it's working!

[djipco]

Awesome. As long as we're making progress, we'll get there.

What do you mean when you say there is no type for WebMidi.addListener() ?

I will look into event.channel. Might have been deprecated, I can't remember.

[jjeff]

You've added the types for Input.addListener() and Output.addListener(). But I didn't see types for WebMidi.addListener('connected', someFunction) or WebMidi.addListener('connected', someFunction). However, now that I'm looking again, I do see them. Great!

[djipco]

I just released v3.0.9 which incorporates the new type declarations. I improved a few things such as refining the types for the target property and adding back the channel property.

Let me know how it works for you guys.

[glend1]

It didn't work for me. I can't find a way to access .name with intellisense.

[djipco]

Hmm... that's odd. It works for me for both the input's name and the note's name. Which .name isn't working for you?

[glend1]

from connected and disconnected events.

[djipco]

The generic Event class (which is what you get when listening to connected and disconnected) does not have a name property. It does have a type property, though.

[glend1]

I used to use e.port.name this returned the name of the device. I no longer have any information about what I'm connecting.

EDIT: well, its still there, but typescript doesn't know.

[djipco]

TypeScript does not know about it because it has been deprecated and is not documented in the .d.ts file. However, for backwards compatibility, I decided to leave the port property in v3.

To make it uniform, from v3 onward, all events are using the target property to identify the object that dispatched the event. So, you can use e.target.name.

Hope this helps.

[glend1]

when using target i get this;

Property 'name' does not exist on type 'Input | WebMidi | InputChannel | Output'.
Property 'name' does not exist on type 'WebMidi

However, if your concerned about backwards compatability when you assign a listener to a press (noteon/noteoff) you no longer have to pass in a channel as an argument. I assume this goes into the optional arguments at the end which I haven't looked into since i don't care about channels.

[djipco]

Check out: #226 (comment)