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

errors from node-addon-api on node 8.0.0 #760

Closed
devsnek opened this issue Jul 6, 2020 · 8 comments
Closed

errors from node-addon-api on node 8.0.0 #760

devsnek opened this issue Jul 6, 2020 · 8 comments

Comments

@devsnek
Copy link
Member

devsnek commented Jul 6, 2020 •
edited
Loading

I'm not sure what to do with this. the version matrix (https://nodejs.org/api/n-api.html#n_api_n_api_version_matrix) says that napi v1 is present but experimental in 8.0.0. I guess that's not entirely correct?

In file included from ../packages/dd-trace/src/native/metrics/main.cc:17:
node_modules/node-addon-api/napi.h:1058:24: error: unknown type name 'napi_async_context'
                       napi_async_context context = nullptr) const;
                       ^
node_modules/node-addon-api/napi.h:1061:24: error: unknown type name 'napi_async_context'
                       napi_async_context context = nullptr) const;
                       ^
node_modules/node-addon-api/napi.h:1065:24: error: unknown type name 'napi_async_context'
                       napi_async_context context = nullptr) const;
                       ^
node_modules/node-addon-api/napi.h:1058:43: error: cannot initialize a parameter of type 'int' with an rvalue of type 'nullptr_t'
                       napi_async_context context = nullptr) const;
                                          ^         ~~~~~~~
node_modules/node-addon-api/napi.h:1058:43: note: passing argument to parameter 'context' here
/Users/gus.caplan/go/src/github.com/DataDog/dd-trace-js/node_modules/node-addon-api/napi.h:1061:43: error: cannot initialize a parameter of type 'int' with an rvalue of type 'nullptr_t'
                       napi_async_context context = nullptr) const;
                                          ^         ~~~~~~~
node_modules/node-addon-api/napi.h:1061:43: note: passing argument to parameter 'context' here
node_modules/node-addon-api/napi.h:1065:43: error: cannot initialize a parameter of type 'int' with an rvalue of type 'nullptr_t'
                       napi_async_context context = nullptr) const;
                                          ^         ~~~~~~~
node_modules/node-addon-api/napi.h:1065:43: note: passing argument to parameter 'context' here
node_modules/node-addon-api/napi.h:1087:7: error: unknown type name 'napi_deferred'
      napi_deferred _deferred;
      ^
node_modules/node-addon-api/napi.h:1238:30: error: unknown type name 'napi_async_context'
                             napi_async_context context = nullptr) const;
                             ^
node_modules/node-addon-api/napi.h:1241:30: error: unknown type name 'napi_async_context'
                             napi_async_context context = nullptr) const;
                             ^
node_modules/node-addon-api/napi.h:1245:30: error: unknown type name 'napi_async_context'
                             napi_async_context context = nullptr) const;
                             ^
node_modules/node-addon-api/napi.h:1238:49: error: cannot initialize a parameter of type 'int' with an rvalue of type 'nullptr_t'
                             napi_async_context context = nullptr) const;
                                                ^         ~~~~~~~
node_modules/node-addon-api/napi.h:1238:49: note: passing argument to parameter 'context' here
node_modules/node-addon-api/napi.h:1241:49: error: cannot initialize a parameter of type 'int' with an rvalue of type 'nullptr_t'
                             napi_async_context context = nullptr) const;
                                                ^         ~~~~~~~
node_modules/node-addon-api/napi.h:1241:49: note: passing argument to parameter 'context' here
node_modules/node-addon-api/napi.h:1245:49: error: cannot initialize a parameter of type 'int' with an rvalue of type 'nullptr_t'
                             napi_async_context context = nullptr) const;
                                                ^         ~~~~~~~
node_modules/node-addon-api/napi.h:1245:49: note: passing argument to parameter 'context' here
node_modules/node-addon-api/napi.h:1359:12: error: unknown type name 'NAPI_NO_RETURN'
    static NAPI_NO_RETURN void Fatal(const char* location, const char* message);
           ^
node_modules/node-addon-api/napi.h:1966:14: error: unknown type name 'napi_async_context'
    operator napi_async_context() const;
             ^
node_modules/node-addon-api/napi.h:1972:5: error: unknown type name 'napi_async_context'
    napi_async_context _context;
    ^
node_modules/node-addon-api/napi.h:2420:20: error: unknown type name 'napi_node_version'
      static const napi_node_version* GetNodeVersion(Env env);
                   ^
In file included from ../packages/dd-trace/src/native/metrics/main.cc:17:
In file included from node_modules/node-addon-api/napi.h:2426:
node_modules/node-addon-api/napi-inl.h:320:24: error: use of undeclared identifier 'napi_run_script'
  napi_status status = napi_run_script(_env, script, &result);
                       ^
node_modules/node-addon-api/napi-inl.h:505:24: error: use of undeclared identifier 'napi_is_promise'
  napi_status status = napi_is_promise(_env, _value, &result);
                       ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
@mhdawson
Copy link
Member

mhdawson commented Jul 7, 2020

I assume you set the define to set the Napi target to version 1. Did you check if doing so with later version ex 10.x compile ok?
I want to confirm that its a problem with the API provided in 8.x being different than 10.x versus problems with the guards in node-addon-api.

@devsnek
Copy link
Member Author

devsnek commented Jul 7, 2020

@mhdawson yeah I did. the problem is that napi v1 contains a lot of API changes between 8.0.0 and 8.7.0 (missing functions, signatures changing, etc).

@mhdawson
Copy link
Member

mhdawson commented Jul 9, 2020

@devsnek I figured as much but thought I should check. I wonder if we should update the table to say that version 1 is there as of 8.7.0 as opposed to 8.0.0 with a note to explain while it was present in earlier versions it was changing as it was still experimental.

@mhdawson
Copy link
Member

@devsnek it probably does not solve your issue, but I'll open a PR to say the first version which supported the final version of 1 was 8.7.0 as that might avoid future confusion.

@devsnek
Copy link
Member Author

devsnek commented Jul 13, 2020

@mhdawson that's definitely helpful 👍. if anything, this is more motivation for us to drop node 8 support 😅

I wonder if there's anything we can do in the c++ headers to detect this sort of situation and give a better error.

@mhdawson
Copy link
Member

@devsnek it might be easiest to just do a Node.js version check and warn/error if the request is for VERSION 1 and you are using less than 8.7.0

mhdawson added a commit to mhdawson/io.js that referenced this issue Jul 13, 2020
@mhdawson
doc: clarify N-API version 1
71c7a51 
Refs: nodejs/node-addon-api#760

Clarify which version of 8.x in which N-API version 1
matches the shape in later versions like 10.x

Signed-off-by: Michael Dawson <[email protected]>
@mhdawson mhdawson mentioned this issue Jul 13, 2020
Closed
3 tasks
@devsnek
Copy link
Member Author

devsnek commented Jul 13, 2020

@mhdawson seems reasonable

@NickNaso NickNaso mentioned this issue Jul 28, 2020
Closed
mhdawson added a commit to nodejs/node that referenced this issue Jul 29, 2020
@mhdawson
doc: clarify N-API version 1
15333ad 
Refs: nodejs/node-addon-api#760

Clarify which version of 8.x in which N-API version 1
matches the shape in later versions like 10.x

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #34344
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
@gabrielschulhof
Copy link
Contributor

The documentation fix to address this issue has landed in core at nodejs/node#34344.

codebytere pushed a commit to nodejs/node that referenced this issue Aug 5, 2020
@mhdawson @codebytere
doc: clarify N-API version 1
e2bea73 
Refs: nodejs/node-addon-api#760

Clarify which version of 8.x in which N-API version 1
matches the shape in later versions like 10.x

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #34344
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gabriel Schulhof <[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
3 participants
@gabrielschulhof @devsnek @mhdawson