promised-ldap icon indicating copy to clipboard operation
promised-ldap copied to clipboard

Published 20 hours ago • verified publisher icon sjmeverett

Export filters

Open felixfbecker opened this issue 9 years ago • 3 comments

First of all, this module is great. Using vanilla ldap with ES7 async/await was a pain in the ***. I only got one problem, which requires me to include vanilla ldap as dependency too - you don't export the filters (and probably some other things neither like the errors). I think it would also be better to export the client seperatly (because you might add server support) at some point e.g. module.exports = {Client, Server, ...filters}. I also don't quite get why you create your own Client class and not just inherit the the ldap class, but overwrite all methods to return promises. What do you think? I would create a PR if you agree.

felixfbecker avatar Oct 03 '15 12:10 felixfbecker

PS: I think it would be the best to only return a promise if no callback is passed. That way you don't have to expose _search, because with a callback the function would just behave like in vanialla ldapjs.

felixfbecker avatar Oct 03 '15 12:10 felixfbecker

Thanks for the feedback. Creating a new class to wrap the existing class is the adapter pattern and is arguably better design than duck punching the existing class.

I guess I did think about exporting it separately, I can't remember why I didn't. Seems like a good idea really.

I'm not sure what you mean about the filters. Feel free to submit a PR and we'll talk about it there!

Thanks.

sjmeverett avatar Oct 08 '15 09:10 sjmeverett

Working on it! Some questions though:

  • What coding style do you follow? It would be nice to have a .eslintrc
  • Can I use ES6 features?
  • What about tests? Is mocha okay?

felixfbecker avatar Oct 08 '15 17:10 felixfbecker