Add es module build for browser installation #70
Conversation
| @@ -278,7 +278,7 @@ <h4 class="name" id="bytesProcessed"><span class="type-signature"></span>bytesPr | |||
|
|
|||
| <dt class="tag-source">Source:</dt> | |||
| <dd class="tag-source"><ul class="dummy"><li> | |||
| <a href="player.js.html">player.js</a>, <a href="player.js.html#line443">line 443</a> | |||
| <a href="player.js.html">player.js</a>, <a href="player.js.html#line447">line 447</a> | |||
There was a problem hiding this comment.
As a suggestion, it might be worth removing the docs directory from check in to avoid the noise in PRs (and maybe autodeployed from CI).
| "scripts": { | ||
| "pretest": "npm run build", | ||
| "test": "mocha", | ||
| "build": "mkdir -p build && rollup -c && npm run docs", |
There was a problem hiding this comment.
I removed the mkdir call, which causes issues on Windows. Rollup seems to work fine without it.
| "watch": "watch 'npm run build' src", | ||
| "docs": "jsdoc src README.md -d ./docs -t ./node_modules/minami", | ||
| "prepare": "npm run build", |
There was a problem hiding this comment.
This is necessary to generate the bundle before installation since the build directory is now being excluded from vcs. For example when installing via git:
"midi-player-js": "git+https://github.com/acmiyaguchi/MidiPlayerJS.git#browser",
|
Wow thanks @acmiyaguchi! This all looks great, truly appreciate your contribution. I know that Agree that docs should be handled differently so they're not always mixed up with code changes. |
|
I'm still having he issue with fs module not found because this PR was merged to master without a release. Do you know when the next release is planned? |
|
Hey @vinzentt, thanks for the reminder. I just published 2.0.14 to npm which includes this change. |
Thanks for this great package! I've had fun using it so far, and it's one of the few packages in the ecosystem that fits my needs.
I'm using a browser bundler for my application, so the current set of builds are problematic. When I use
npm install midi-player-jsfor my web app, it also includes therequire("fs")statement inloadFile. I did a couple things to resolve this:loadFilethat rollup eliminates as a dead branch when being built for the browserbuilds/index.browser.js. I add this to the package.json underbrowserso my bundler grabs the correct distribution.I also removed the build directory from check ins. I left the browser directory alone, in case the github raw link was being used to source the script.
This works as expected in a small test page I made.