Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make IsObject() allow functions? #207

Closed
addaleax opened this issue Jan 2, 2018 · 4 comments
Closed

Make IsObject() allow functions? #207

addaleax opened this issue Jan 2, 2018 · 4 comments

Comments

@addaleax
Copy link
Member

addaleax commented Jan 2, 2018

Hi everyone!

I’ve just received my first PR from somebody else on a N-API project 🎉 node-ffi-napi/weak-napi#3

I’m wondering whether it makes sense to change IsObject() in the wrapper so that napi_function is also allowed, because functions are objects in the JS sense and any N-API operation that expects an object should also work on a function. What do you think?
@jasongin
Copy link
Member

jasongin commented Jan 2, 2018

I agree, it seems like a bug. IsObject() should be true for functions.

@mhdawson
Copy link
Member

mhdawson commented Jan 3, 2018

I agree as well. IsObject() should return true for a function.

@guymguym
Copy link

guymguym commented Jan 3, 2018

Hey, sounds good to me.
btw you might consider having the original IsObject meaning as IsObjectLike the same as done by lodash:
https://lodash.com/docs/4.17.4#isObject
https://lodash.com/docs/4.17.4#isObjectLike

romandev added a commit to romandev/node-addon-api that referenced this issue Jan 22, 2018
@romandev
Make IsObject() allow functions
2b4aeb1 
As discussed in the issue[1], IsObject() should return true for a
function.

[1] nodejs#207
@romandev romandev mentioned this issue Jan 22, 2018
Merged
addaleax pushed a commit that referenced this issue Jan 28, 2018
@romandev @addaleax
Make IsObject() allow functions
1376377 
As discussed in the issue[1], IsObject() should return true for a
function.

[1] #207

PR-URL: #217
Reviewed-By: Anna Henningsen <[email protected]>
@addaleax
Copy link
Member Author

This has been fixed a while ago, the PR author just didn’t add a Fixes: tag. Closing! :)

kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this issue Aug 24, 2022
@kevindavies8
Make IsObject() allow functions
15ecde0 
As discussed in the issue[1], IsObject() should return true for a
function.

[1] nodejs/node-addon-api#207

PR-URL: nodejs/node-addon-api#217
Reviewed-By: Anna Henningsen <[email protected]>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this issue Aug 26, 2022
@Marlyfleitas
Make IsObject() allow functions
9aa647a 
As discussed in the issue[1], IsObject() should return true for a
function.

[1] nodejs/node-addon-api#207

PR-URL: nodejs/node-addon-api#217
Reviewed-By: Anna Henningsen <[email protected]>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this issue Sep 19, 2022
@wroy7860
Make IsObject() allow functions
1b38121 
As discussed in the issue[1], IsObject() should return true for a
function.

[1] nodejs/node-addon-api#207

PR-URL: nodejs/node-addon-api#217
Reviewed-By: Anna Henningsen <[email protected]>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this issue Aug 11, 2023
Make IsObject() allow functions
5b639e8 
As discussed in the issue[1], IsObject() should return true for a
function.

[1] nodejs/node-addon-api#207

PR-URL: nodejs/node-addon-api#217
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@addaleax @guymguym @mhdawson @jasongin