-
Notifications
You must be signed in to change notification settings - Fork 479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prioritize resolution of .then #736
Conversation
I rebuilt |
Failed on Node 0.10: I've no idea what might be going on there. 😄 |
* @param elem {*} object to inspect | ||
* @return {Boolean} is `elem` a Thenable? | ||
*/ | ||
dust.isThenable = function(elem) { | ||
return elem && | ||
typeof elem === 'object' && | ||
return elem && /* Beware: `typeof null` is `object` */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can totally add the null check here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a think about this: When elem
is null
it'll be falsy, so it'll early-out on the elem && ...
. Testing for null
would be redundant (albeit explicit).
There look to be a few tests that ensure this e.g. "scalar null in a # section", so it seems covered.
Couple small comments, and a rebase, and 👍 from me @jimmyhchan @prashn64 @smfoote , one more? I reran the failing CI test; Rhino isn't the most reliable. Looks fine now. |
Rebased, typo in the commit message and all. 👀 Let me know if there's anything else (e.g. the explicit |
👍 looks great |
Closes #735