search

grimmdude / MidiWriterJS

♬ A JavaScript library which provides an API for programmatically generating and creating expressive multi-track MIDI files and JSON.
MIT License
547 stars 58 forks source link

Docs: Explicit note that it only works with Node. Not in the browser. #83

Closed y0nd0 closed 3 years ago

y0nd0 commented 3 years ago

It would be helpful if there is a prominent note in readme.md.

Only works with Node. Not in browser.

Ok, there is a tag (keynames) node set for this repository. But does this exclude all other platforms (browser)? I cannot build with with TypeScript. Because require('fs') in build/index.js. So it's Node only.

Personally, I would remove this dependency. And just return the ArrayBuffer or what ever. So that everyone on browser or Node can handle it.

grimmdude commented 3 years ago

Hi @y0nd0,

This library should work in the browser, I thought we had that fs dependency setup to be optional but perhaps it needs to be adjusted.

I think that's a good idea though, probably best to just return ArrayBuffer. I'll try to get to this when I have time, but PRs are always welcome!

-Garrett

y0nd0 commented 3 years ago

Nope, Webpack TypeScript build says "No".

Can't resolve 'fs' in '.\node_modules\midi-writer-js\build'.

Maybe #71 fixes it?

See code. But not sure if this really works. It's also weird that this pull request made so many other changes on the project setup. But anyway. The require('fs') dependency should be removed.

As quick shot we could simply remove it and just export the the buffer. And add some docs how to handle. It's pretty simple to save a buffer as file or whatever. Btw. Buffer is also Node-only. Ok there are polyfills on browser or lib (see npm package).

Whatever, I would strictly separate the midi logic from platform depending stuff. Just I/Os or plugins.

Update: I crafted my own midi file lib. So I'm out. - You can close this issue if you want. Like I've never been here. ^^

grimmdude commented 3 years ago

Hi @y0nd0,

I've just released version 2.0.0 which removes the Writer.saveMIDI() function, thus removing the dependency on fs. It was primarily legacy code I used to use for testing when this library first started, and seems to cause more trouble than it's worth. There are other methods in the Writer class that can be used to get the raw MIDI data.

Thanks for bringing it up. Hopefully this solves on your end.

-Garrett