Add support for Node.js 10 #314
Conversation
|
@Qard Thought I'd just kick of this PR now that Node 10 is out. As our preliminary testing has shown, it will fail, but feel free to push commits to this PR to fix it if you get time.
|
|
I changed the lint and code coverage tests to use the latest LTS release instead of 10, so that they don't fail just because the latest release have issues. Just so we don't waste time looking in the wrong places. |
|
The hapi failure is a result of changes to promise hooks in V8, which result in broken continuation from async_hooks when an |
|
@watson How do we want to handle landing this? Should we configure Node.js 10 to allow failures for now? |
|
In any case we should probably update the Compatibility docs to mention the missing support. How about adding this line:
|
|
It already states that async_hooks is experimental in the note below there. Perhaps we could just expand that with an additional sentence explaining that async/await in Node.js 10 has known issues? |
|
I think it's better to have it in the list so it's not buried inside a paragraph. But I agree it could be more to the point. What do you think of this instead:
|
|
Maybe replace "not be instrumented properly" with "not be supported" to make the intent clearer. |
This is a temporary solution until nodejs/node#20274 is resolved
docs/compatibility.asciidoc
Outdated
| @@ -15,8 +15,9 @@ image::https://raw.githubusercontent.com/nodejs/Release/master/schedule.png[] | |||
| Some versions of Node.js contain bugs or issues that limit our ability to instrument them correctly. | |||
| The following versions of Node.js are known to not be fully instrumented: | |||
|
|
|||
| - v8.0.x - Recommended solution: Upgrade to v8.2.0 or higher to get full support | |||
| - v10.x - Async/Await in Node.js 10 is currently buggy and can therefore not be supported (https://github.com/nodejs/node/issues/20274:[nodejs/node#20274]) | |||
There was a problem hiding this comment.
nodejs/node#20516 is probably a better issue to point to.
test/instrumentation/modules/hapi.js
Outdated
| @@ -13,6 +13,9 @@ var semver = require('semver') | |||
| // hapi 17+ requires Node.js 8.9.0 or higher | |||
| if (semver.lt(process.version, '8.9.0') && semver.gte(pkg.version, '17.0.0')) process.exit() | |||
|
|
|||
| // hapi does not work on Node.js 10 because of https://github.com/nodejs/node/issues/20274 | |||
Until nodejs/node#20516 have been resolved, we have disabled the hapi tests to make sure we still test as much as possible on Node.js 10
Todo
Closes #307