Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nodejs #61

Merged
merged 6 commits into from
Sep 9, 2015
Merged

Nodejs #61

merged 6 commits into from
Sep 9, 2015

Conversation

abudaan
Copy link
Contributor

@abudaan abudaan commented May 7, 2015

  • made shim more platform agnostic: removed global window in both web and nodejs version
  • fixed bug in nodejs example, simplified it and improved comments
  • added a test for npm test
  • updated README

@jazz-soft
Copy link
Contributor

Daniel, I just wonder if it's possible to browserify the performance-now module into WebMIDIAPI.js to minimize the number of end user dependencies?

@abudaan
Copy link
Contributor Author

abudaan commented May 15, 2015

Not sure if I understand you correctly. The performance-now package is only needed in the nodejs version; for the browser version we have the performance polyfill in src/util.js or the native implementation of performance.now() when available

@jazz-soft
Copy link
Contributor

Ok, I'll try to do it myself after your check-in gets merged. If that will not work, I'll leave everything as it is :)

@abudaan
Copy link
Contributor Author

abudaan commented May 16, 2015

Okay, but browserify is meant for adding functionality of nodejs packages to a web project. So it is a bit odd to browserify performance-now while performance.now() has been implemented in most modern browsers.

Also I don't see a problem in having dependencies; npm is made for managing dependencies and it encourages the use of smaller interdependent modules for the sake of maintainability and scalability.

The way the WebMIDIAPIShim is currently set up only needs and uses the performance-now package for nodejs projects. Browser projects only need the transpiled .js file in the /lib folder.

If you insist on limiting the number of dependencies, you could remove the performance-now package all together: in that case the polyfill will add a performance.now() function based on setTimer().

You can do this by replacing the lines 6 - 8 in in /lib/index.js

var performance = {
  now: require('performance-now')
};

by just:

var performance;

@jazz-soft
Copy link
Contributor

Thanks a lot for the advice. I'll try it.
By the way, the plugin itself has function Time() that returns time in ms

@abudaan
Copy link
Contributor Author

abudaan commented Jun 2, 2015

The examples routing_1 and routing_2 didn't work correctly on Windows and OSX because in Chrome 43 stable the device ids on Windows are index numbers (0, 1, 2, etc.) and therefor not unique per device. On OSX the device ids can be negative numbers which can't be used as html id attribute for the checkboxes. So I have added the prefixes 'input' and 'output'.

@jazz-soft
Copy link
Contributor

Chris, are you on vacation? :)

@cwilso
Copy link
Owner

cwilso commented Jun 11, 2015

Kind of. Swamped post-I/O, and haven't had a chance to review this (sizable!) update.

@jazz-soft
Copy link
Contributor

Hey Chris, just wonder if you are still maintaining this project?

@cwilso
Copy link
Owner

cwilso commented Sep 9, 2015

Sooooo sorry. Been very busy, and I wanted to look this over in detail.

cwilso added a commit that referenced this pull request Sep 9, 2015
@cwilso cwilso merged commit e154619 into cwilso:gh-pages Sep 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants