search

AndreasMadsen / async-hook

Inspect the life of handle objects in node
MIT License
36 stars 14 forks source link

fix: conditionally use legacy `asyncWrap` behavior for Node.js v4 #6

Closed timkendrick closed 8 years ago

timkendrick commented 8 years ago

Reinstates legacy asyncWrap behavior when run on Node.js v4. Fixes #5 and AndreasMadsen/trace/#24

AndreasMadsen commented 8 years ago

Sorry. it is more complicated than this.

Bump node.js for updating async wrap in Node 4 here: https://github.com/nodejs/LTS/issues/86 It is also an overview of all the changes between Node 4 and Node 5.

In any case I'm not going to implement backward compatibility, there are constantly made changes to async_wrap and it is a nightmare to maintain. You can use older versions of async-hook (1.2.0 for node 4) and everything should work.

AndreasMadsen commented 8 years ago

Sorry didn't see your message in: https://github.com/AndreasMadsen/trace/issues/24#issuecomment-211423221

I didn't realize you also made async-hook, impressive work! I'll submit a PR for that package, which reinstates the prior behavior on Node v4 – let me know on that thread if this is a step in the wrong direction.

Thanks for the effort, but I definitely think it is in the wrong direction. In a previous version of async-hook it did have backward compatibility but it was a nightmare to maintain. Since then the distance between Node 4 and Node 5 have only become greater.

I will refer to my original statement.

I have been thinking about removing async-hook from the dependencies list and add an install script, that will download the correct async-hook version automatically. But I'm too busy for that right now. PRs are appreciated.

You could take a look at npm-compat, but it is 3 years old the npm logic may have changed.

timkendrick commented 8 years ago

OK, thanks for the clarification – I'll see how far I can get with trying to get trace to use an older async-hook version. Thanks for the quick responses!

AndreasMadsen commented 8 years ago

Node.js v4.5.0 is now available. This has the latest AsyncWrap API and thus trace should work out of the box.