-
Notifications
You must be signed in to change notification settings - Fork 460
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.
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
How about add C++20 coroutine support to Napi::Value
?
#1456
Comments
FWIW, node-addon-api is restricted to the same build restrictions as node, which is c++17. |
@KevinEady you are right, but we have two choices:
Whaty do you think about? |
I think in instances where functionality is added that is not specifically a wrapper for Node-API functionality, we defer to placing the functionality in a separate module/package owned by the original code writer (and therefore not maintained by us), eg. #1163 |
@KevinEady Is it a better choice to add {
"cflags_cc": [ "-std=c++20" ],
"xcode_settings": {
"CLANG_CXX_LANGUAGE_STANDARD":"c++20",
"OTHER_CPLUSPLUSFLAGS": [ "-std=c++20" ]
},
# https://github.com/nodejs/node-gyp/issues/1662#issuecomment-754332545
"msbuild_settings": {
"ClCompile": {
"LanguageStandard": "stdcpp20"
}
},
} for example, I changed my toy implementation and placed it in
|
Napi::Promise
?Napi::Value
?
main...toyobayashi:node-addon-api:coroutine I added changes and test in my fork. This is a very simple implementation and have not tested complex use case. |
Following up on @KevinEady's earlier comment about node-addon-api being a thin wrapper, this is documented in https://github.com/nodejs/node-addon-api/blob/main/CONTRIBUTING.md#source-changes. We discussed in the node-api team meeting today and based on our documented approach we believe this functionality is best covered in a separated module outside of node-addon-api unless that is impossible. Some team members are going to take a deeper look and we'll talk about it again next time. |
This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made. |
We discussed again today, from the discussion in the meeting today, the feeling of the team continuies to be that it can be implemented on top of node-addon-api without needing to be integrated. Can you confirm that? That along with the agreed approach of keeping node-api and node-addon-api lean as documented in https://github.com/nodejs/node-addon-api/blob/main/CONTRIBUTING.md#source-changes means that we believe it should be implemented in an external libray versus integrated into node-addon-api itself. Our suggestion is that you create a separate repo/npm package to provide the functionality. If you do that please let us know and we will consider linking to it from the node-addon-api README.md for those who would be interested in co-routine support. Let us know if that makes sense to you so we can close the issue? |
Respect team's opinion, I create this issue just inspired by embind. Thanks for your suggestion. |
embind already has coroutine implementation
https://github.com/emscripten-core/emscripten/blob/b5b7fedda835bdf8f172a700726109a4a3899909/system/include/emscripten/val.h#L703-L789
I just now tried to write a toy version, that makes it possible to
co_await
a JavaScript Promise in C++.class CoPromise : public Napi::Promise
binding.gyp
binding.cpp
index.js
node index.js
output
The text was updated successfully, but these errors were encountered: