Support Node.js 10

[acappella2017]

We observed some weird behaviors with node-ldapjs when upgraded to Node.js 10 LTS in our project.

• unbind not work (already reported as Unbind not working with node 10 #483)
• ldapjs UT hangs with Node.js 10

This issue is caused by some internal change in Node.js. See nodejs/node#24577.

In summary, in Node.js 10, the close event depends on an internal end listener. so when ldap client use removeAllListeners(end`) to do cleanup work, the close listener stops working.

Node.js guys suggest that we should use removeListener to explicitly remove the listener we want to remove instead of using removeAllListeners. Node.js document also suggests that we should use removeAllListeners carefully.

Note that it is bad practice to remove listeners added elsewhere in the code, 
particularly when the EventEmitter instance was created by some other component or 
module (e.g. sockets or file streams).

This PR didn't touch all of the removeAllListeners in client.js, just fixed the one who breaks close event.

@jgeurts, @melloc, could you help to review this PR?

[sunpeinju]

@melloc @pfmooney
Could you help to take a look?

@acappella2017 has found the root cause (removeAllListeners even removes TLS socket's internal listeners and breaks socket's event sequence) and discussed with Node team. And this PR contains the safest change we can find, it doesn't change current code logic.

Many projects and npm modules depending on ldapjs need it to work on Node.js V10. Hope to see this PR get reviewed.

[longuniquename]

Hi. Any progress on this?

[kspearrin]

I just forked the repo and am using my own version with this fix in place.

"ldapjs": "git+https://git@github.com/kspearrin/node-ldapjs.git",

[zachlendon]

Hi!

Any updates/plans on this? It seems multiple individuals are left to fork to get this fix into their projects, is it mergeable here or is non-review work outstanding?

[nickzelei]

@zachlendon This project is pretty much dead. None of the maintainers have commented on here in years.

[jharris4]

For what it's worth, we started having major issues with ldap using a verdaccio server with the verdaccio-ldap plugin after upgrading to Node 10. The verdaccio ldap plugin uses this package.

I tried using the fork of this project to resolve the issues without any luck, so we had no choice but to downgrade to Node 8.

[bryancuster]

Any updates or reasons to not merge this?

[jsumners]

@acappella2017 would you be able to add a unit test for this?

[acappella2017]

@acappella2017 would you be able to add a unit test for this?

@jsumners Without this pr, ldapjs UT hangs with node 10. I believe current UTs are able to cover this change.

[jsumners]

Coverage remains the same before and after this PR. So I'll assume this is covered by any unbind tests that currently exists. We need a test coverage audit in the future anyway.

[jsumners]

⚠️ This issue has been locked due to age. If you have encountered a recent
problem that seems to be covered by this issue, please open a new issue.

Please include a minimal reproducible example
when opening a new issue.