Added Promise support with capabilities test to renderFile #333
Conversation
|
@jdalrymple @RyanZim could y'all take a look and see if this works? It's a lot of change to a fundamental function, so I'd like some other eyes on it. But it ought to be backward compatible. The only concern we might have is in environments where people wrote code previous to the existence of native Promises. But even then the basic API ought to be the same. |
lib/ejs.js
Outdated
| try { | ||
| result = handleCache(options)(data); | ||
| if (!cb) { | ||
| if (typeof Promise == 'function') { |
There was a problem hiding this comment.
typeof always returns a String: (the name of the type, or the string 'undefined'). Strict equality is only necessary, or even useful, in cases where coercion could create incorrect results. (I'm a huge fan of strict equality against zero, Booleans, empty string -- but it doesn't make sense to cargo-cult it everywhere.)
There was a problem hiding this comment.
@mde to make sense look into projects like ts2c where we are working on cross compilation of JS to C86 it helps if we do all strict as this is where we are progressing on with es7 and the new static imports. There are Global goals for the Lang so it makes sense i would propose to do strict check for undefined and function thanks.
There was a problem hiding this comment.
Thanks for the feedback. I have been writing JS as my primary language for almost 20 years, and have written a book on JS development. My experience with JS pre-dates the existence of the so-called threequal, and I'm well aware of the problems it's intended to address. Mindlessly applying it everywhere is not, in my opinion, particularly useful or helpful -- despite people parroting what they've read in "The Good Parts" or other sources. I would also assert that your goals, for the language or the goals of the people in your particular part of the ecosystem, are not, in fact, global. There is in fact far less unity in thinking about the direction of JS than in many other languages (compare with Python, and its BDFL), which of course necessitates an actual committee to steer its development. (This may be in part because so everyone is compelled to use it, and going all the way back to the early 2000s, these groups have always attempted to twist it into a shape that makes them more comfortable. The Prototype.js people wanted it to be Ruby, the MochiKit people wanted it to be Python, the Dojo people wanted it to be Java.) There are numerous constituencies involved on the committee, with competing opinions, and "JS as a compile target" is only one of them, and not one I am particularly interested in. Lastly, EJS has a very strict linting policy -- tests will not pass if linting does not pass. If I were interested in enforcing the blanket use of triple-equal in this codebase, I would have added a linting rule for it. Again, thanks very much for commenting. I really appreciate it -- but I will not be commenting further on this.
| // in the data, copy them to options | ||
| if (arguments.length === 3) { | ||
| // Do we have a callback? | ||
| if (typeof arguments[arguments.length - 1] == 'function') { |
There was a problem hiding this comment.
@niftylettuce === is totally correct == is weak and don't checks for type so === checks including type in this case number if you want to have a Javascript Training or need any other Consulting simpy contact me i am free for individual trainings
There was a problem hiding this comment.
@mde sorry saw it to late you should do === not == for same reason as above!
There was a problem hiding this comment.
Please see above. Thanks for the comments, but I will not be applying the strict-equality operator everywhere.
|
@RyanZim Good call. I was going to add tests before merging. Thanks a lot for taking a look! |
|
Tests pass. I am merging this to master. |
|
Is there some documentation on how to use it? Edit: I see, it's in |
|
Version 2.5.8 on NPM now has this feature live. |
Added native Promise support to
renderFile. Checks for the existence of thePromisector before trying to use it.