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

napi_add_env_cleanup_hook is not implemented #16925

Closed
oscarotero opened this issue Dec 4, 2022 · 7 comments · Fixed by #17324
Closed

napi_add_env_cleanup_hook is not implemented #16925

oscarotero opened this issue Dec 4, 2022 · 7 comments · Fixed by #17324
Assignees
Labels
node native extension related to the node-api (.node)

Comments

@oscarotero
Copy link
Contributor

Hi. I have been trying to use lighningcss in Deno and it seems to work fine but I got some warnings in the terminal:

napi_add_env_cleanup_hook is currently not supported
napi_add_env_cleanup_hook is currently not supported
napi_add_env_cleanup_hook is currently not supported
napi_add_env_cleanup_hook is currently not supported
napi_add_finalizer is not yet supported.
napi_add_finalizer is not yet supported.
napi_add_finalizer is not yet supported.
napi_add_finalizer is not yet supported.
napi_add_finalizer is not yet supported.
napi_add_finalizer is not yet supported.
napi_add_finalizer is not yet supported.
napi_add_finalizer is not yet supported.

Node API support is marked as done in the NPM roadmap, but the referrer issue has some Functions unmarked, like napi_add_env_cleanup_hook and napi_add_finalizer.

Is there any plans to implement them?

@bartlomieju
Copy link
Member

CC @littledivy

@bartlomieju
Copy link
Member

These still exist, but can be silenced with --quiet flag. We will finally implement them, but can't give any time horizon for it.

@oscarotero
Copy link
Contributor Author

Ok. Is it ok to silence them? I'm not familiarized with Node API and what these functions do, but if they are not implemented, I guess the package won't work properly.

@bartlomieju
Copy link
Member

To be honest I'm unsure about it, @littledivy can you take a look and lt us know?

@janpio
Copy link

janpio commented Jan 4, 2023

Note: Prisma also is experiencing these napi_add_env_cleanup_hook is currently not supported outputs:

C:\Users\Jan\Documents\throwaway\denoRegression>deno run -A --unstable npm:prisma@4.9.0-dev.16 generate --data-proxy
  prisma:engines  binaries to download libquery-engine, migration-engine, introspection-engine, prisma-fmt +0ms
napi_add_env_cleanup_hook is currently not supported
  prisma:tryLoadEnv  Environment variables not found at null +0ms
  prisma:tryLoadEnv  Environment variables loaded from ./.env +0ms
Environment variables loaded from .env
Prisma schema loaded from prisma\schema.prisma
napi_add_env_cleanup_hook is currently not supported
  prisma:getConfig  Using getConfig Wasm +0ms
  prisma:getConfig  config data retrieved without errors in getConfig Wasm +2ms
  ...

@bartlomieju
Copy link
Member

Note to self: discussed this with Divy, there are no more blockers to implement these functions - napi_add_env_cleanup_hook should be straight-forward to implement, while napi_add_finalizer requires integration with FinalizationRegistry which was not available in rusty_v8 at the time when we added Node-API support. I will look into implementing these.

@bartlomieju
Copy link
Member

Let's track napi_add_finalizer in #17325 and keep this issue for napi_add_env_cleanup_hook

@bartlomieju bartlomieju changed the title Node support: warnings with some napi_ functions napi_add_env_cleanup_hook is not implemented Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
node native extension related to the node-api (.node)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants