Conversation
Co-authored-by: Jordan Harband <ljharb@gmail.com>
ljharb
left a comment
There was a problem hiding this comment.
Can you elaborate on which specific tests wouldn't have passed before this PR?
If the only effect is that class constructors are going to report as being callable, i don't think tightly adhering to the spec in this case is worth it.
| module.exports = require('is-callable'); | ||
| module.exports = function IsCallable(argument) { | ||
| // some older engines say that typeof /abc/ === 'function', | ||
| return typeof argument === 'function' |
There was a problem hiding this comment.
this isn't actually useful though, because it will report class constructors as callable.
There was a problem hiding this comment.
Well, that’s what the specification for IsCallable says to do.
ljharb
left a comment
There was a problem hiding this comment.
Something like this, but we'd still need to determine if getters are supported, in case Reflect.contruct is polyfilled in ES5.
cbdd330 to
371fc87
Compare
371fc87 to
c617543
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #93 +/- ##
==========================================
+ Coverage 90.93% 91.00% +0.07%
==========================================
Files 647 647
Lines 9041 9071 +30
Branches 2125 2137 +12
==========================================
+ Hits 8221 8255 +34
+ Misses 820 816 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c617543 to
a0dc7e0
Compare
|
I've separated out the IsConstructor fixes and pulled those into master; this PR has been rebased with just the IsCallable changes. |
ES5+: Use spec‑accurate IsCallable and IsConstructor implsES5+: Use spec‑accurate IsCallable
The
IsConstructorimplementation is based on the one in tc39/ecma262#1798 (comment)Fixes #48