search

ericf / express-handlebars

A Handlebars view engine for Express which doesn't suck.
BSD 3-Clause "New" or "Revised" License
2.31k stars 383 forks source link

Support callbacks with then/nodeify module #99

Closed srquinn21 closed 9 years ago

srquinn21 commented 9 years ago

I'm not trying to be "that guy" but as a long time consumer of this module I'm not upgrading to the 1.0 branch because of the move away from standard callbacks to Promises. I understand the aesthetic and flow control benefits gained, however, the public API should follow the best practices of the community. I shouldn't have to work around your internal design choices.

Please consider providing a dual API until Promises are standardized in the Javascript/Node community. Since you are using the then/promise module, might you also consider their https://github.com/then/nodeify module to integrate callback support?

ericf commented 9 years ago

Promises are a standard part of JavaScript as of ES6, they're implemented in browsers, and will be in Node.js (and are in the IO.js fork). The switch to using Promises reduced the LOC by 1/3, made this package faster, and made the code easier to read compared to using the async module.

srquinn21 commented 9 years ago

I'm not asking to remove Promise support, simply to provide both callbacks and Promises in the public facing API. While, yes, Promises are the future of handling async code, the future is still the future. They are not currently present in Node stable and ES6 is still a working draft — thus the reason you are depending on the then/promise module. As a result, my code is becoming unreadable because there are both callbacks and Promises instead of a unified design pattern. Again, I believe that a library should not dictate internal design decisions to their consumers — the public API should support the community's standards which at the moment are still callbacks.