X Tutup
The Wayback Machine - https://web.archive.org/web/20201231110228/https://github.com/nodejs/node-addon-examples/pull/161
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.

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

Add example showing ts plus js plus cpp and c #161

Merged
merged 1 commit into from Nov 16, 2020

Conversation

@helio-frota
Copy link
Contributor

@helio-frota helio-frota commented Oct 22, 2020

No description provided.

ts_js_cpp_c/cPart.h Outdated Show resolved Hide resolved
extern "C" {
#endif

void cPart();

This comment has been minimized.

@NickNaso

NickNaso Oct 23, 2020
Member

What do you think to change the funtion cPart to be a non void function? In this way the example could be more complete and demostrate how to call from ts -> js -> c++ -> c passing some arguments and back.

This comment has been minimized.

@helio-frota

helio-frota Oct 23, 2020
Author Contributor

This is a really good suggestion thank you.
What do you think a phrase like this ? :

I'm a C function and I received this $argument from C++

And then return an int back ?

And the the c++ says:

I'm a C++ function and I received this $argument from JS and this $return from C

and the same for JS and TS ...

something like that ? looks cool 😄

Since I still learning I'll probably delay a little bit but eager to implement that

This comment has been minimized.

@helio-frota

helio-frota Oct 23, 2020
Author Contributor

@NickNaso suggestions applied now it shows the following messages:

I'm a C function and I received 12 from C++ and I'm sending back 42 
I'm a C++ function I'm sending 12 to C and I received 42
I'm a Javascript function and I'm sending 11 to C++ and I received 42
I'm a TypeScript class constructor I'm sending 10 to JavaScript and I received 42
@helio-frota helio-frota force-pushed the helio-frota:polyglot-example branch 2 times, most recently from 7c1dd44 to a07ebfd Oct 23, 2020
Copy link
Member

@NickNaso NickNaso left a comment

Could you please move the example in a folder called node-addon-api like all other examples in the repo.

ts_js_cpp_c |
            |- > node-addon-api
@helio-frota helio-frota force-pushed the helio-frota:polyglot-example branch from a07ebfd to f764de9 Oct 26, 2020
@helio-frota
Copy link
Contributor Author

@helio-frota helio-frota commented Oct 26, 2020

@NickNaso example moved to the new directory, thank you for the review 👍

ts_js_cpp_c |
            |- > node-addon-api
@@ -0,0 +1,2 @@
node_modules

This comment has been minimized.

@gengjiawen

gengjiawen Oct 26, 2020
Member

This is unnecessary, as it's already declared in root folder.

This comment has been minimized.

@helio-frota

helio-frota Oct 26, 2020
Author Contributor

@gengjiawen thank you for the review, PR updated 👍

@helio-frota helio-frota force-pushed the helio-frota:polyglot-example branch from f764de9 to 33abd10 Oct 26, 2020
@helio-frota helio-frota force-pushed the helio-frota:polyglot-example branch from 33abd10 to 8d578fd Oct 26, 2020
Copy link
Member

@gengjiawen gengjiawen left a comment

LGTM

@gengjiawen
Copy link
Member

@gengjiawen gengjiawen commented Oct 26, 2020

You can format your cpp code if your have used clang-format. But this won't be a blocker for this PR.
Thanks.

@helio-frota
Copy link
Contributor Author

@helio-frota helio-frota commented Oct 26, 2020

You can format your cpp code if your have used clang-format. But this won't be a blocker for this PR.
Thanks.

Yeah I still learning not only the language (c/cpp) but also the tools involved 👍. Thanks again for the review

@gengjiawen
Copy link
Member

@gengjiawen gengjiawen commented Oct 26, 2020

CI failed on macOS, looks like you need to add set (CMAKE_CXX_STANDARD 11) in cmake config.

Log: https://github.com/nodejs/node-addon-examples/pull/161/checks?check_run_id=1308615674

@helio-frota helio-frota force-pushed the helio-frota:polyglot-example branch from 8d578fd to 0304e87 Oct 26, 2020
@helio-frota
Copy link
Contributor Author

@helio-frota helio-frota commented Oct 26, 2020

@gengjiawen PR updated 👍

@gengjiawen gengjiawen requested a review from NickNaso Nov 3, 2020
Copy link
Member

@NickNaso NickNaso left a comment

LGTM

@mhdawson
Copy link
Member

@mhdawson mhdawson commented Nov 5, 2020

I think calling this something like typescript_with_addon might make it more likely for people to find/understand what it is trying to show.

@helio-frota helio-frota force-pushed the helio-frota:polyglot-example branch from 0304e87 to 9b1c9db Nov 9, 2020
@helio-frota
Copy link
Contributor Author

@helio-frota helio-frota commented Nov 9, 2020

@mhdawson thanks for the suggestion, example renamed 👍

@gengjiawen gengjiawen requested a review from mhdawson Nov 12, 2020
@gengjiawen gengjiawen merged commit 4a1e9b5 into nodejs:master Nov 16, 2020
11 of 13 checks passed
11 of 13 checks passed
test (10.21.0, ubuntu-latest)
Details
test (10.21.0, windows-latest) test (10.21.0, windows-latest)
Details
test (10.21.0, macos-latest)
Details
test (12.18.0, ubuntu-latest)
Details
test (12.18.0, windows-latest) test (12.18.0, windows-latest)
Details
test (12.18.0, macos-latest)
Details
test (14.x, ubuntu-latest)
Details
test (14.x, windows-latest) test (14.x, windows-latest)
Details
test (14.x, macos-latest) test (14.x, macos-latest)
Details
test (15.x, ubuntu-latest)
Details
test (15.x, windows-latest) test (15.x, windows-latest)
Details
test (15.x, macos-latest) test (15.x, macos-latest)
Details
nightly-daily-test nightly-daily-test
Details
@gengjiawen
Copy link
Member

@gengjiawen gengjiawen commented Nov 16, 2020

Thanks for the PR, great job.

@helio-frota helio-frota deleted the helio-frota:polyglot-example branch Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.
X Tutup