Koa: Node 6 support.

[akdor1154]

Github code search reports that there is no use of async or await in koa itself. Therefore, it should work fine in Node 6 (and anecdotally it seems to). Therefore I am struggling to understand the engine restriction on >=7.6 (sorry if I've missed something obvious). Obviously middleware authors can require Node > 7.6 if they are going to have async and await directly in npm-published code, but it would be great if Koa itself could relax this restriction for the core if it is unnecessary. My own reasoning is that I'll be damned if I put non-LTS versions of Node into production.

[PlasmaPower]

The two reasons I can see are that the tests use async/await (so no Node 6 testing), and we might use async/await in the future (e.g. koa-compose might benefit from it).

[akdor1154]

I know this would be a relatively big move, but could you just move to koa@3 when you start using async/await?

The testing is an interesting issue if you are not wanting to rely on Babel at all. However, it would only be a dev dependency... (I'd be happy to do this work if it's something that would be accepted, and would of course make sure testing on new Node doesn't invoke transpilation).

[jonathanong]

you could just ignore the engine restriction. it would be a bigger issue if someone tried to use koa@2 with async/await on node v6 not knowing that it's not available on that version of node.

we could drop it, but we're not going to bother adding babel as a dev dependency just to support v6.

[PlasmaPower]

However, it would only be a dev dependency

Using babel for tests often makes debugging harder, in addition to being a pain to setup. I don't think it would be accepted.

[danwkennedy]

If you're worried about an LTS version of Node, Node 8 is scheduled for release in about a month. You could simply wait to upgrade to Koa 2+ until then. Plus the version of V8 that will ship with Node 8 has Promise and async/await performance improvements.

[akdor1154]

What about a runtime check that only runs the ES7 tests in a supported environment? I don't think ignoring engine restrictions is a good precedent, particularly as if async/await stuff does land in Koa core then the engine restriction is a very sane way to flag it.
@danwkennedy If my reading is correct, Node 8 will not be considered LTS until October. Until then it will see development as 'Current' at the same rate of current Node 7.

[danwkennedy]

Node 6 has had just about the same rate of changes as Node 7 regardless of the status of 'current'. The only thing LTS guarantees is length of support.

[PlasmaPower]

@akdor1154 if my PR goes through, even without the node-fetch switch, pretty much every test will rely on async/await.

Edit: also, since async/await is at the syntax level, it would be hard to disable specific tests with it.

[akdor1154]

@danwkennedy my interpretation of

Once a major version enters LTS coverage, new features (semver-minor) may only be landed with consent of the CTC and the LTS Working Group. No semver-major changes other than those required for critical security fixes may be landed.

is that before LTS coverage is entered, the Node version is ripe for more invasive changes. With that said I'm not 100% sure my interpretation is correct, I find their wording a little unclear.

@PlasmaPower it certainly makes sense to test usage of Koa with native async/await, however I don't think it makes sense to stop testing it with Promises at the same time.

[PlasmaPower]

it certainly makes sense to test usage of Koa with native async/await, however I don't think it makes sense to stop testing it with Promises at the same time

I think duplicate tests are a very bad idea (maintenance problems). If you're talking about splitting them, firstly async/await tests are easier to write and secondly that would result in only half the tests running for Node v6, which IMO is as bad as no tests running since it gives the illusion of support.

[akdor1154]

Those are good points. However looking through the tests, they are not actually testing Koa with async/await, they are simply using async/await to send the test requests themselves, because, as you say, it is 'easier to write'. Even then, a lot of tests seem to have moved to async functions for no reason; they use fetch().then() internally anyway.
I'm not a member of this project so I know it's not up to me (and I don't want to derail this discussion into things that are more suited to be discussed on the PR itself), but as an onlooker, it's concerning that the "current" version of this project would be entirely unable to run on a single LTS version of Node, not because it uses features that are unavailable on LTS Node, but because its tests were 'easier to write' with software that was only released two weeks ago. I'm not talking about ancient LTS either, I'm talking about software that entered LTS only four months ago. :/ I'm all for breaking compatibility with crusty old Node versions, but this is a little bit extreme.

[bttmly]

Further, it seems weird to insist users use async/await -- Koa works perfectly fine with Promises and indeed, even without async/await I find the Koa middleware model far superior to Express/Connect.

As for the worry of someone running koa@2 in an environment that doesn't support async/await, it doesn't seem like it should be the library's responsibility for ensuring developers know the features of then own platform. I think disclaimer in the docs about Node versions that support async/await and a note that Promises can be used instead should be sufficient.

[PlasmaPower]

@nickb1080 I don't think we're insisting, but rather recommending. It's nice to have the community standardized on one form.

[akdor1154]

@PlasmaPower Koa is not insisting that users use async/await, it is however insisting that users use an incredibly recent version of Node that supports async/await, even though these are not necessary to use Koa in the first place.

[bttmly]

Sorry, by "insist" I had a specific case in mind: on Node 6.6 and below yarn add [email protected] fails with

error [email protected]: The engine "node" is incompatible with this module. Expected version ">= 7.6.0".
error Found incompatible module

You can get around this by using --ignore-engines (or npm) but that's kind of a hassle, for one package. While this does come down to how yarn chooses to deal with engine property in package.json, it seems like the use of engine should indicate where it can run rather than where the authors suggest it run.

[akdor1154]

It's also possible for npm to be configured to outright block packages where the engine parameter suggests that local Node is incompatible, so this isn't specific to yarn.

[bttmly]

Yes, but the default of yarn is to reject. And because in this case the engine is actually not correct (it runs fine on older node versions), if you want to install it with yarn you have to ignore all packages' engine fields and hope for the best.

[DominicBoettger]

Also if you use typescript the engine restriction is not needed.....

[jonathanong]

Yes, but the default of yarn is to reject.

is this true? i use shrinkwrap and all the node engine requirements are just warnings. is the same true for npm?

if so, i am inclined to lower the engine requirement in package.json

[DominicBoettger]

Yes, the default of yarn is to reject.

[danwkennedy]

If that's the case aren't middleware like koa-bodyparser going to break the check anyway since they do use async functions and legitimately require node at versions >= 7.6.0?

[jonathanong]

@danwkennedy yes, but that's a different issue.