From bb4f23166aa5dd2c31827a430c67e6321f7653fe Mon Sep 17 00:00:00 2001
From: David Herman <david.herman@gmail.com>
Date: Tue, 10 Sep 2024 12:33:29 -0700
Subject: [PATCH] avoid the double-type-check of eagerly downcasting to
 function when calling .bind(), since it's performed when calling .apply()

---
 crates/neon/src/object/mod.rs     | 25 ++++++++++++++++++-------
 crates/neon/src/types_impl/mod.rs |  7 +++++--
 test/napi/lib/functions.js        | 13 ++++++++-----
 test/napi/lib/objects.js          |  2 +-
 4 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/crates/neon/src/object/mod.rs b/crates/neon/src/object/mod.rs
index af100195d..796f03804 100644
--- a/crates/neon/src/object/mod.rs
+++ b/crates/neon/src/object/mod.rs
@@ -32,14 +32,20 @@
 //! [hierarchy]: crate::types#the-javascript-type-hierarchy
 //! [symbol]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol
 
+use smallvec::smallvec;
+
 use crate::{
     context::{internal::ContextInternal, Context, Cx},
     handle::{Handle, Root},
     result::{NeonResult, Throw},
     sys::{self, raw},
     types::{
-        build, extract::{TryFromJs, TryIntoJs}, function::{BindOptions, CallOptions}, utf8::Utf8, JsFunction, JsObject, JsUndefined, JsValue, Value,
-        private::ValueInternal
+        build,
+        extract::{TryFromJs, TryIntoJs},
+        function::{BindOptions, CallOptions},
+        private::ValueInternal,
+        utf8::Utf8,
+        JsFunction, JsObject, JsUndefined, JsValue, Value,
     },
 };
 
@@ -197,10 +203,14 @@ where
     /// May throw an exception either during accessing the property or downcasting it
     /// to a function.
     pub fn bind(&'a mut self) -> NeonResult<BindOptions<'a, 'cx>> {
-        let callee: Handle<JsFunction> = self.this.get(self.cx, self.key)?;
-        let mut bind = callee.bind(self.cx);
-        bind.this(self.this)?;
-        Ok(bind)
+        let callee: Handle<JsValue> = self.this.get(self.cx, self.key)?;
+        let this = Some(self.this.upcast());
+        Ok(BindOptions {
+            cx: self.cx,
+            callee,
+            this,
+            args: smallvec![],
+        })
     }
 }
 
@@ -212,7 +222,8 @@ pub trait Object: Value {
         cx: &'a mut Cx<'cx>,
         key: K,
     ) -> PropOptions<'a, 'cx, K> {
-        let this: Handle<'_, JsObject> = Handle::new_internal(unsafe { ValueInternal::from_local(cx.env(), self.to_local()) });
+        let this: Handle<'_, JsObject> =
+            Handle::new_internal(unsafe { ValueInternal::from_local(cx.env(), self.to_local()) });
         PropOptions { cx, this, key }
     }
 
diff --git a/crates/neon/src/types_impl/mod.rs b/crates/neon/src/types_impl/mod.rs
index 494e0eee9..658e6e10d 100644
--- a/crates/neon/src/types_impl/mod.rs
+++ b/crates/neon/src/types_impl/mod.rs
@@ -1179,8 +1179,11 @@ where
     );
 
     match status {
-        sys::Status::FunctionExpected => {
-            return cx.throw_type_error("not a function");
+        sys::Status::InvalidArg => {
+            if !sys::tag::is_function(env.to_raw(), callee) {
+                return cx.throw_error("not a function");
+            }
+            panic!("invalid argument");
         }
         sys::Status::PendingException => {
             return Err(Throw::new());
diff --git a/test/napi/lib/functions.js b/test/napi/lib/functions.js
index cee8fda6d..94bcac8b0 100644
--- a/test/napi/lib/functions.js
+++ b/test/napi/lib/functions.js
@@ -1,8 +1,11 @@
 var addon = require("..");
 var assert = require("chai").assert;
 
-const STRICT = function() { "use strict"; return this; };
-const SLOPPY = Function('return this;');
+const STRICT = function () {
+  "use strict";
+  return this;
+};
+const SLOPPY = Function("return this;");
 
 function isStrict(f) {
   try {
@@ -61,7 +64,7 @@ describe("JsFunction", function () {
     assert.equal(result, 42);
   });
 
-  it("bind a strict JsFunction to a number", function() {
+  it("bind a strict JsFunction to a number", function () {
     assert.isTrue(isStrict(STRICT));
 
     // strict mode functions are allowed to have a primitive this binding
@@ -70,7 +73,7 @@ describe("JsFunction", function () {
     assert.strictEqual(result, 42);
   });
 
-  it("bind a sloppy JsFunction to a primitive", function() {
+  it("bind a sloppy JsFunction to a primitive", function () {
     assert.isFalse(isStrict(SLOPPY));
 
     // legacy JS functions (aka "sloppy mode") replace primitive this bindings
@@ -78,7 +81,7 @@ describe("JsFunction", function () {
     const result = addon.bind_js_function_to_number(SLOPPY);
 
     assert.instanceOf(result, Number);
-    assert.strictEqual(typeof result, 'object');
+    assert.strictEqual(typeof result, "object");
     assert.strictEqual(result.valueOf(), 42);
   });
 
diff --git a/test/napi/lib/objects.js b/test/napi/lib/objects.js
index 6ef313b70..1fe346862 100644
--- a/test/napi/lib/objects.js
+++ b/test/napi/lib/objects.js
@@ -165,6 +165,6 @@ describe("JsObject", function () {
 
     assert.throws(() => {
       addon.call_non_method_with_prop(obj);
-    }, /failed to downcast/);
+    }, /not a function/);
   });
 });