bionode / bionode-fasta

Streamable FASTA parser.
bionode.io
MIT License
14 stars 9 forks source link

Remove zlib dependency, replace with core Node api #6

Open thejmazz opened 8 years ago

thejmazz commented 8 years ago

Just reading through, checking dependency readmes, and turns out zlib (which does not come up with search on npm), is now deprecated as mentioned here. Functionality has been folded into Node core.

This brings up another topic: how to migrate to new Node versions? I think a lot of bionode is from the "v0.12 era"? I've been using the LTS Node 4 for web projects currently. But the new versions bring in some really sweet ES6 features, and I think bionode is less susceptible to "complex environments that find it cumbersome to continually upgrade" [1] and so should be OK to use the latest stable release. But I guess also, should try to stick to LTS as long as the stable features aren't super worth it. I heard that they are trying to match up v6 with V8 5.x or so for something like 95% ES6 support, so thats something nice to look out for.

bmpvieira commented 8 years ago

I would suggest to have travis CI test automatically a bunch of node versions and try write code to be as backward compatible as possible without compromising functionality. However, if the new features are really worth it, I'm OK with deprecating old 0.12.* Node and just keep 4 and 5. I agree that upgrading each module shouldn't be too difficult since they're so small and don't have a huge and complex dependency tree. In any case, any break in backward compatibility should bump the major version number as per semantic versioning guidelines.

The only thing I would worry about are Streams, since they seem have been changing a lot with each major Node release, so we need to be careful about that. Ideally we should support the best which should be the latest implementation of Streams, and this might be a good argument to deprecate old Node versions.

As for ES6 features, I don't have yet a strong opinion about them, they look cool. It may be worth opening another issue on the bionode/bionode module to discuss best practices. I'm only worried that they might brake some browser/browserify compatibility or at least while they're not mainstream,make it more difficult for developers that are unfamiliar to contribute code (similar to using things like CoffeeScript).

mafintosh commented 8 years ago

@bmpvieira re, stream you should switch all you local require('stream') to require('readable-stream') and install that from npm. readable-stream is a mirror of core streams and is tracked using semver. that way it is a lot easier to reason about how streams behave across multiple versions of node. through2, duplexify all do this already

thejmazz commented 8 years ago

@bmpvieira Sounds good.

Yeah, using ES6 in browsers is a little sketchy atm, since browser implementations are not yet complete. However, everyone just uses Babel. While ES6 is new, yes, that may make it more difficult for new devs. However, ES6 is JavaScript, so its a little different from things like CoffeeScript, TypeScript, etc. It's starting to catch on more, with const and arrow functions being used in readmes and tutorials. So far I like it a lot since it can code a lot more succinct:

const add = (a, b) => a+b
thejmazz commented 8 years ago

@mafintosh what about here? :P

mafintosh commented 8 years ago

@thejmazz yup that should be switched to use readable-stream instead :)

in general, i'm :-1: on any es6 features (or any other feature for that matter) that require compiling.

thejmazz commented 8 years ago

I'm also totally against compiling Node code. However, when discussing this, I think it's clear to make a distinction between node-side and browser-side. Since when using npm modules on the browser you will need to browserify/webpack anyways, and that can be considered a "compile" step. I'm currently writing bionode-obo with ES6 features that need no --harmony flags or anything, and works in the browser. I'll make an issue on the main repo for this, just so we don't go too off topic here.

But yeah, compiling Node side is not that nice. I think that's one of the main reasons the authors of Koa don't want call v2 stable; not until async/await works without flags/transpiling.