neurosity / openbci-observable

Making OpenBCI for Node Reactive
MIT License
37 stars 7 forks source link

OpenBCIRx async start() #14

Closed JayKan closed 7 years ago

JayKan commented 7 years ago

@alexcastillo,

When running npm start, I was getting syntax errors that relates async start() locally. However, when I created a brand new node project and install openbci-rx as its dependencies and run, everything is working as expected. Please find attached screenshot for the detailed error:

screen shot 2017-04-03 at 4 25 24 pm

It would be great if you can run via the example demos locally, maybe there are additional steps that I'm not aware in order to run examples locally, please let me know!

alexcastillo commented 7 years ago

Hey Jay,

async/await is only available in node v7+ What node version are you running?

JayKan commented 7 years ago

@alexcastillo,

I was running on node 6.5. Let me get back to you once I've tested on node v7+.

JayKan commented 7 years ago

@alexcastillo,

Apparently, only node 7.6.0 >= supports async/await functionality. I've tried node 7.2 - 7.5 and I was getting the similar error. However, once I've upgraded my node to 7.6.0 >=, there isn't any issue running npm start.

Should I add a prerequisites section on README to indicate that you must use node version >= 7.6 under development, and a .nvmrc file to lock the node version for local development? I think this will avoid any issues when new community members try to contribute. Let me know what you think!

alexcastillo commented 7 years ago

Sure, feel free to submit a PR with those updates. Would also recommend adding "engines" in package.json.

https://docs.npmjs.com/files/package.json#engines

alexcastillo commented 7 years ago

See commit #577b4a29548ac7d72da0b9b746a837dc6c7ef695

JayKan commented 7 years ago

@alexcastillo,

That's good to know. With adding engines property on package.json, I don't think we will need a .nvmrc file. Anyways I will send a PR to update the README :)