djipco / webmidi

Tame the Web MIDI API. Send and receive MIDI messages with ease. Control instruments with user-friendly functions (playNote, sendPitchBend, etc.). React to MIDI input with simple event listeners (noteon, pitchbend, controlchange, etc.).
Apache License 2.0
1.53k stars 115 forks source link

remove input listeners not accessable on disconnected event #229

Closed glend1 closed 2 years ago

glend1 commented 2 years ago

my environment is next.js. when i attach the following listener;

WebMidi.addListener("disconnected", (e) => {
    e.port.removeListener()
})

i get this error TypeError: e.port.removeListener is not a function

i tried e.target.removeListener() i don't get the error, but i also don't think it removes the listener(s).

glend1 commented 2 years ago

since 3.0.13 the error is now properly showing up with typescript.

This expression is not callable.
  Each member of the union type '(<T extends keyof InputEventMap>(type?: Symbol | T | undefined, listener?: InputEventMap[T] | undefined, options?: { channels?: number | number[] | undefined; context?: any; remaining?: number | undefined; } | undefined) => void) | (<T extends keyof OutputEventMap>(type?: Symbol | ... 1 more ... | undefined, listene...' has signatures, but none of those signatures are compatible with each other.
djipco commented 2 years ago

You should now (v3.0.14) be able to use e.port.removeListener() on disconnected events.

glend1 commented 2 years ago

are you sure?

djipco commented 2 years ago

are you sure?

Yes.

However, note that your code adds a listener to the WebMidi object and removes all listeners from the Input or Output that was disconnected. These are two completely separate things.

When you remove a listener, you need to remove it from the object where it was added. In this case, if you want to remove all listeners from WebMidi, you should use: WebMidi.removeListener().

glend1 commented 2 years ago

yep, sorry. your right. typescript is complaining about it though.

djipco commented 2 years ago

typescript is complaining about it though.

Complaining about what? I don't seem to have a problem when I use it.

glend1 commented 2 years ago

its telling me This expression is not callable.

djipco commented 2 years ago

I thought you meant the error was triggered by WebMidi.removeListener() when you actually mean the error is triggered by e.port.removeListener(). Sorry about the confusion.

In all honesty, I have no clue why it reports an error. My (very limited) knowledge of TypeScript cannot decipher the error. I'm going to re-open the issue but we will have to find some knowledgeable TypeScript power-user to look into this.

Perhaps you could ask in the Discussions and reference this issue?

For the record, here is the generated error message:

TS2349: This expression is not callable.
Each member of the union type '(<T extends keyof InputEventMap>(type?: Symbol | T, listener?: InputEventMap[T], options?: { channels?: number | number[]; context?: any; remaining?: number; }) => void) | (<T extends keyof OutputEventMap>(type?: Symbol | T, listener?: OutputEventMap[T], options?: { ...; }) => void)' has signatures, but none of those signatures are compatible with each other.
djipco commented 2 years ago

I did some more digging today and I understand what the problem is. Basically, it goes like this:

  1. When the disconnected event is triggered by the WebMidi object, the callback function will be passed a PortEvent.
  2. The PortEvent contains a port property which can either be an Input or an Output (either of which can be disconnected).
  3. When you call removeListener() on this PortEvent.port object, the acceptable parameters are derived from either the InputEventMap type or the OutputEventMap type (depending which kind of port triggered the event).
  4. The problem is that InputEventMap and OutputEventMap do not have the same signature. This is what triggers the error.

So, to prevent the error, starting with v3.0.15, I am using a type of any for the PortEvent.port property instead of Input | Output. This is an ugly fix but I don't know how else this can be resolved since we cannot know ahead of time the type of port that triggered the disconnected event.

P.S. In v3.0.15, OutputEventMap is renamed PortEventMap since its properties are common to both input and output ports.

glend1 commented 2 years ago

fair enough. If a solution arises will you make a note to fix it? or put a note somewhere that there is a limitation in the typescript implimentation?

Maybe it is an archiectural floor? Maybe connected inputs should be a different events to conntected outputs?

djipco commented 2 years ago

I am asking for help on StackOverflow about this. Hopefully, someone will chime in...

djipco commented 2 years ago

I have also started a page with information regarding TypeScript. Hopefully this can act as a reminder of yet-to-solve problems.

braebo commented 1 year ago

I can think of a few ways to sort this out -- is it still an issue?

djipco commented 1 year ago

The issue has been "fixed" with the aforementioned ugly workaround but if you have a cleaner solution, I'd love to hear it. Thanks for offering to help!