koajs / koa-hbs

Handlebars templates for Koa.js
MIT License
160 stars 43 forks source link

Update docs, don't advise using --harmony or harmonica anymore #65

Closed mgol closed 6 years ago

mgol commented 7 years ago

Node.js 7.6.0 and newer support async/await natively and older versions have serious memory leaks so it's better to not rely on --harmony. If running Koa in an older Node.js version is desired, it's better to transpile async/await rather than rely on a buggy implementation.

shellscape commented 7 years ago

thank you for the PR, but I disagree with your assessment. some folks have no choice as to the version of node they run.

mgol commented 7 years ago

I realize some people don't have a choice of a Node version they run on. I'm just saying for those people it's usually better to transpile in such a case instead of relying on a buggy implementation. Node.js older than 7.6 has memory leaks in its async/await implementation (see https://github.com/nodejs/node/issues/9339); using Babel will avoid that.

One can use babel-register to make it quite seamless. What do you think about recommending that instead?

shellscape commented 7 years ago

well babel-register isn't recommended for production use, so I don't think that's a good recommendation. And --harmony flags are considered experimental/unstable and preview, which means not for production use. I think it's reasonable to assume that anyone dabbling with an unstable harmony feature understands that it's unstable. I'd be open to language that points out (gently) that it's recommended to upgrade, but I don't want to mislead people who are stuck on that version and want to play with the features.

mgol commented 7 years ago

I'll think about the phrasing.

mgol commented 7 years ago

One more question: should koa-hbs still use harmonica itself? Since it's tested in latest v7 it would work without it as well and there's always a slight risk in using --harmony-* flags.

I can prepare a PR once I know what's your take here.

mgol commented 6 years ago

PR updated. Does that look better?

shellscape commented 6 years ago

I'm gonna step the module in a different direction and compile down to Node 6 support, which will result in removing all of the documentation about running with harmony. In the end I think this will be more beneficial to all.