fix: Input/Output Polymorphism #493
Conversation
dopry
force-pushed
the
fix/Input-Output-Polymorphism
branch
from
ad94c84 to
e7b6942
Compare
src/Port.js
Outdated
| import { WebMidi } from "./WebMidi.js"; | ||
|
|
||
| /** | ||
| * The `Input` class represents a single MIDI input port. This object is automatically instantiated |
There was a problem hiding this comment.
Documentation seems to have been copied from Input. Needs to be adjusted.
There was a problem hiding this comment.
I updated the documentation. I also hoisted common property declarations from the constructor to the class and added the private prefix, '#' to _port.
There was a problem hiding this comment.
removing the # I'd need to change the eslint config to support it and it's ES2022. I don't want to dig into the bable config to figure out if it would transpile it down, so sticking to the approach that avoids changing eslint and adds legacy compatibility risk.
|
Thanks a lot for your work on this. However, before I can merge it, the documentation needs to be carefully reviewed. The auto-generated documentation on the website relies on jsdoc being correct. From casual perusal, I can see that the Unfortunately, I do not have time right now to do this myself. |
dopry
force-pushed
the
fix/Input-Output-Polymorphism
branch
5 times, most recently
from
e4b93a8 to
ee6af46
Compare
src/Port.js
Outdated
| } | ||
|
|
||
| /** | ||
| * Destroys the `Input` by removing all listeners, emptying the [`channels`](#channels) array and |
src/Port.js
Outdated
|
|
||
| if (e.port.connection === "open") { | ||
| /** | ||
| * Event emitted when the `Input` has been opened by calling the [`open()`]{@link #open} |
|
Thanks so much for your work on this. However, a quick search still reveals many references to |
refactor with a base class of Port for common code.
dopry
force-pushed
the
fix/Input-Output-Polymorphism
branch
from
ee6af46 to
07b870f
Compare
|
done |
|
Again, thanks for your work on this. However, there is still a part that is missing: the unit tests. As far as I know, this library has been pretty stable and I do not want to introduce any forms of regression. So, we would need a new test file in |
|
Cool. Probably beyond my availability for additional work on this issue. We're actually moving away from WebMidi.js to just use the Wed MIDI API directly in our use cases. Thanks for entertaining it. Feel free to wrap it up if you're interested or close it if you're not. |
|
I dont' really have time to work on this currently. I'll leave it open and, hopefully, I can fin some time. If someone else wants to take a stab at updating the tests, that would be awesome. |
2 participants
This is a pseudo code to illustrate the concept. I'm not very good with JSDoc these days. I personally find it easier just to work with typescript so TSC and VS code do all the heavy lifting to generate my types.
This implementation refactors Input and Output with a base class of Port.
applies to #229