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

Let's discuss: modernizing the codebase #337

Closed
marvinroger opened this issue Apr 10, 2017 · 10 comments
Closed

Let's discuss: modernizing the codebase #337

marvinroger opened this issue Apr 10, 2017 · 10 comments

Comments

@marvinroger
Copy link
Contributor

marvinroger commented Apr 10, 2017

Hi!

I was about to start a new Node.js project that does exactly what zetta does, so I'd be glad to contribute here.

What do you think about modernizing the codebase? We could do that by small bits: replace all var by const/let, replace prototype with the new ES6 class syntax. I am pretty sure this would be a step forward. It would allow us to have a futureproof codebase, which could be backward compatible (using Babel), but these ES6/ES2015 features are already implemented in Node.js, so Babel is completely optional for development on recent Node.js versions.

I also saw (#336) that same dependencies are not up-to-date and even insecure. What about adding a David badge on the repo? This would encourage the community to contribute by updating some of the dependencies.

Talking about dependencies, what about using Yarn? This would lock down the dependencies and ensure we're all using consistent dependencies, and this avoids a lot of problems with users not using the same dependencies versions. By the way, NPM would still work as it currently does, so no breaking changes here.

Just a few ideas. I can really see Zetta gaining a lot of traction, so thanks for the cool project!

EDIT: also, what about enforcing a coding style, like the standard today on a lot of OSS projects, JavaScript Standard Style or semistandard which is the same but with semicolons added. I am in favor of the former, but I can understand that no semicolons might be scary. I did try, and I won't go back as I finally really like the style.

@AdamMagaluk
Copy link
Collaborator

@marvinroger I'm all for modernizing the codebase unfortunately us core developers have let it slip this past year. Thanks for the interest and support glad to know other people find it useful.

Regarding ES6 features: I think they are great but we have to be aware of the node versions that ship on some of the hardware we target like Beaglebone, Raspberry Pi, and Intel Edison and if Babel would be required to deploy Zetta to factory flashed version of each. Though I think the const, let are not major issues in zetta and can slowly be ported over time as we update code in each package. I'm more concerned with all the stale dependancies.

I haven't really played with yet but from what I understand we wouldn't need to change zetta to use Yarn right? It's just a alternative client to NPM.

@marvinroger
Copy link
Contributor Author

The version published to npm would actually be the compiled code, with Babel, so the code would be compatible with the same platforms as today. Moreover, I also care about embedded boards: Node.js provides official arm binaries, so all boards are now compatible with the latest versions.

The only thing needed to actually benefit from the Yarn dependencies lockin is to commit a Yarn lockfile (which is automatically created when you yarn install). It's like an enhanced NPM shrinkwrap. Yarn is way, way faster and reliable.

@marvinroger
Copy link
Contributor Author

First step for the ecosystem to be able to use modern ES2015 classes: see zettajs/zetta-scientist#7

@kyork-cl
Copy link
Contributor

We have Zetta deployed on a few thousand devices that currently only support Node v0.12.7. We are planning to update node in the future, but that is a major QA task and I'm not sure what the timeline for it is.

Would adding support for classes break compatibility for us?

@marvinroger
Copy link
Contributor Author

marvinroger commented Apr 11, 2017

Function.prototype.bind is an ES5 feature, so it should be available on Node.js v0.12.7. I am unable to test but the code in the above PR is backward-compatible.

@kevinswiber
Copy link
Member

Function.prototype.bind should be compatible with Node going as far back as v0.10.x, which is the earliest version still supported by Zetta.

@AdamMagaluk I think the Beaglebone Black boards we were using for workshops came pre-installed with v0.10.5. It looks like the latest boards are shipping with v4.x, but I'm sure there are a bunch out there with v0.12.x.

Due to the nature of the deployment model associated with device gateways, we will likely need to hold on to v0.12.x support for quite some time. We could, perhaps, consider dropping v0.10.x support, but I don't think that buys us much.

Also, I just have one question I posted on zettajs/zetta-scientist#7.

@marvinroger
Copy link
Contributor Author

To begin:

  • Replace var with const and let. The resulting code is cleaner, and let is block-scoped whereas var is function scoped, which is a weird behavior
  • Replace prototype manipulations with ES2015 classes

If we develop on Node >= v6, the above is supported without transpiler. However, you target Node 0.10, so we can use Babel with the preset-env with the target set to Node 0.10 so that all ES2015/ES2016/ES2017 syntaxes will be transpiled to ES5 code understandable by Node 0.10. For the builtins, we cannot include babel-polyfill because it pollutes the global scope, and zetta is a library so this is not an option. We can use the transform-runtime-plugin which will map the builtins to a locally installed babel-runtime that we'll need to add as a dependency.

Therefore, we'll be able to write ES2015+ code that would still be compatible with Node 0.10 except:

Instance methods such as "foobar".includes("foo") will not work since that would require modification of existing built-ins

The version published to NPM would be the Babel-built one. Only caveat: the stack trace would not reflect the original code, but there are sourcemaps to mitigate the issue anyway.

@kevinswiber
Copy link
Member

@AdamMagaluk, @mdobson: I know you're both busy, but I might have some cycles to spare on this.

@marvinroger: Would you be willing to get the boilerplate in place for doing the Babel transpile?

If so, I'd like to propose that we merge it into an es6 branch in this repo and iterate on that. Objections?

@kevinswiber
Copy link
Member

Also, @marvinroger, welcome to Zetta! There is actually a lot to it. If you have any questions, I'm happy to hop on IRC or the Gitter channel to discuss. Or there's always the mailing list.

Some things that aren't mentioned enough in Zetta:

  • Zetta nodes can connect to each other. After peering, the receiving node can then proxy calls to the dialing node. Because of how Zetta establishes the connection, this can cross network boundaries. Because the peering request starts life as an HTTP request, we can apply security measures like OAuth and mutual TLS to the peering request itself.
  • Zetta actually uses a decent query language. This language can be used to filter devices in searches through both the Web and JavaScript APIs. This filter can also be used on multiplexed WebSocket stream subscriptions to filter the incoming data.
  • Zetta queries can cross multiple servers. Just use an asterisk (*) for the server value.
  • Zetta is meant to run everywhere Node can run. We've made an effort to keep this project cross-platform compatible, even going so far as to build our own all-JavaScript, embedded, persistent key-value store.
  • Zetta app queries are backed by RxJS and can use reactive operators for complex scenarios.

I'm sure there are 100 more Cool Things Few People Know About Zetta, but this should be a good start.

@marvinroger
Copy link
Contributor Author

@kevinswiber done: #346

Well thanks for the warm welcome. This is such a cool project, I already implemented a driver for some cheap Xiaomi devices: https://github.com/marvinroger/zetta-lumi-aqara-driver and did not really hit any barriers.. Yet? 😋
Except this ugly process.EventEmitter = require('events').EventEmitter; at the top but hey, it works!

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

No branches or pull requests

4 participants