Skip to content

src: add Object::TypeTag, Object::CheckTypeTag #1261

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 4 commits into from

Conversation

KevinEady
Copy link
Contributor

Adds support for napi_type_tag_object via Object::TypeTag and napi_check_object_type_tag via Object::CheckTypeTag.

Closes: #1260

@KevinEady
Copy link
Contributor Author

KevinEady commented Jan 2, 2023

Looks like the test has different outcomes depending on Node version, which I can reproduce locally. The test (cpp, js) is similar to the one found in node core (cpp, js).

It looks like there was an issue in type tag checking nodejs/node#43786 fixed by nodejs/node#43788 ... Was this backported to other Node versions?

EDIT: I just changed the test to not use the bugged checks if node version < 18.

v14: Failure ❌

➜  node-addon-api git:(add-type-tag-methods) ✗ nvm use v14                        
Now using node v14.21.2 (npm v6.14.17)
➜  node-addon-api git:(add-type-tag-methods) ✗ node test/object/object_type_tag.js
(node:13959) UnhandledPromiseRejectionWarning: AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

false !== true

    at test (/Users/kevineady/Documents/Projects/node-addon-api/test/object/object_type_tag.js:17:10)
    at Object.exports.runTest (/Users/kevineady/Documents/Projects/node-addon-api/test/common/index.js:122:27)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:13959) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:13959) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

v16: Failure ❌

➜  node-addon-api git:(add-type-tag-methods) ✗ nvm use v16                        
Now using node v16.15.1 (npm v8.11.0)
➜  node-addon-api git:(add-type-tag-methods) ✗ node test/object/object_type_tag.js
node:internal/process/promises:279
            triggerUncaughtException(err, true /* fromPromise */);
            ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

false !== true

    at test (/Users/kevineady/Documents/Projects/node-addon-api/test/object/object_type_tag.js:17:10)
    at Object.exports.runTest (/Users/kevineady/Documents/Projects/node-addon-api/test/common/index.js:122:27) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: 'strictEqual'
}

v18: Success ✅

➜  node-addon-api git:(add-type-tag-methods) ✗ nvm use v18                        
Now using node v18.12.1 (npm v8.19.2)
➜  node-addon-api git:(add-type-tag-methods) ✗ node test/object/object_type_tag.js

v19: Success ✅

➜  node-addon-api git:(add-type-tag-methods) ✗ nvm use v19    
Now using node v19.3.0 (npm v9.2.0)
➜  node-addon-api git:(add-type-tag-methods) ✗ node test/object/object_type_tag.js

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

mhdawson pushed a commit that referenced this pull request Jan 13, 2023
PR-URL: #1261
Reviewed-By: Michael Dawson <midawson@redhat.com
@mhdawson
Copy link
Member

Landed as b11e4de

@mhdawson mhdawson closed this Jan 13, 2023
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
PR-URL: nodejs/node-addon-api#1261
Reviewed-By: Michael Dawson <midawson@redhat.com
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing napi_type_tag_object?
2 participants