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

Node API core consolidation #44071

Open
legendecas opened this issue Jul 31, 2022 · 3 comments
Open

Node API core consolidation #44071

legendecas opened this issue Jul 31, 2022 · 3 comments
Labels
node-api Issues and PRs related to the Node-API.

Comments

@legendecas
Copy link
Member

People are complaining that the node-api core implementation is deviating from node.js core conventions and has a lot of workarounds and non-conventional mechanisms, especially around the finalizers. It makes it harder to maintain the node-api code base.

I'm working with the @nodejs/node-api team on the document https://docs.google.com/document/d/1Bqm05PWQni65FVdSKiaVAkqCAOD9HIfCqG_Y4CAYTbo/edit?usp=sharing to list the problems we are facing and try to figure out what approaches we can apply.

@addaleax thanks for bringing the topic up at #36510 (comment).

Opening this issue to track the progress.

@legendecas legendecas added the node-api Issues and PRs related to the Node-API. label Jul 31, 2022
@deepakrkris
Copy link

I am interested in working on this

@legendecas
Copy link
Member Author

legendecas commented Aug 13, 2022

@deepakrkris it will be great if you can help out on this! As we discussed in the last node-api team meeting, please free feel to drop a comment on the document if you have any questions.

legendecas added a commit that referenced this issue Dec 19, 2022
Generalize the finalizer's second pass callback to make it
cancellable and simplify the code around the second pass callback.

With this change, it is determined that Reference::Finalize or
RefBase::Finalize are called once, either from the env's shutdown,
or from the env's second pass callback.

All existing node-api js tests should pass without a touch. The
js_native_api cctest is no longer applicable with this change,
just removing it.

PR-URL: #44141
Refs: #44071
Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit that referenced this issue Jan 1, 2023
Generalize the finalizer's second pass callback to make it
cancellable and simplify the code around the second pass callback.

With this change, it is determined that Reference::Finalize or
RefBase::Finalize are called once, either from the env's shutdown,
or from the env's second pass callback.

All existing node-api js tests should pass without a touch. The
js_native_api cctest is no longer applicable with this change,
just removing it.

PR-URL: #44141
Refs: #44071
Reviewed-By: Michael Dawson <[email protected]>
RafaelGSS pushed a commit that referenced this issue Jan 4, 2023
Generalize the finalizer's second pass callback to make it
cancellable and simplify the code around the second pass callback.

With this change, it is determined that Reference::Finalize or
RefBase::Finalize are called once, either from the env's shutdown,
or from the env's second pass callback.

All existing node-api js tests should pass without a touch. The
js_native_api cctest is no longer applicable with this change,
just removing it.

PR-URL: #44141
Refs: #44071
Reviewed-By: Michael Dawson <[email protected]>
RafaelGSS pushed a commit that referenced this issue Jan 5, 2023
Generalize the finalizer's second pass callback to make it
cancellable and simplify the code around the second pass callback.

With this change, it is determined that Reference::Finalize or
RefBase::Finalize are called once, either from the env's shutdown,
or from the env's second pass callback.

All existing node-api js tests should pass without a touch. The
js_native_api cctest is no longer applicable with this change,
just removing it.

PR-URL: #44141
Refs: #44071
Reviewed-By: Michael Dawson <[email protected]>
juanarbol pushed a commit that referenced this issue Jan 26, 2023
Generalize the finalizer's second pass callback to make it
cancellable and simplify the code around the second pass callback.

With this change, it is determined that Reference::Finalize or
RefBase::Finalize are called once, either from the env's shutdown,
or from the env's second pass callback.

All existing node-api js tests should pass without a touch. The
js_native_api cctest is no longer applicable with this change,
just removing it.

PR-URL: #44141
Refs: #44071
Reviewed-By: Michael Dawson <[email protected]>
@mhdawson
Copy link
Member

@legendecas maybe we should go through this in the next node-api meeting to see what's been completed and what is still needed on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

No branches or pull requests

3 participants