Skip to content

Conversation

@januwA
Copy link

@januwA januwA commented Feb 15, 2021

Added IsNullOrUndefined detection js type function 😃

napi.h Outdated
bool IsUndefined() const; ///< Tests if a value is an undefined JavaScript value.
bool IsNull() const; ///< Tests if a value is a null JavaScript value.
bool IsUndefined() const; ///< Tests if a value is an undefined JavaScript value.
bool IsNullOrUndefined() const; ///< Tests if a value is a null or undefined JavaScript value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe IsNullish would be more concise. With the definition of ?? from MDN as (emphasis mine)

The nullish coalescing operator (??) is a logical operator that returns its right-hand side operand when its left-hand side operand is null or undefined, and otherwise returns its left-hand side operand.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New changes have been submitted

Copy link
Contributor

@KevinEady KevinEady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @januwA ,

Thanks for your patience. So take a look at the comments. Also, you will need to add documentation to doc/value.md and a tests to test/basic_types/value.js. For the test, I think checking for true on null and undefined, and false on something else is a good enough test coverage.

}

inline bool Value::IsNullish() const {
return Type() == napi_undefined || Type() == napi_null;
Copy link
Contributor

@KevinEady KevinEady Feb 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler may optimize this since it is const, but let's store the Type() call in a variable or use a switch statement to ensure it isn't called twice.

@mhdawson
Copy link
Member

@januwA are you still planning to move this forward?

@januwA januwA closed this Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants