-
Notifications
You must be signed in to change notification settings - Fork 44
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
#98 - Fix native worker compatibility #109
Conversation
@@ -1,7 +1,7 @@ | |||
{ | |||
"targets": [ | |||
{ | |||
"target_name": "fs-ext", | |||
"target_name": "fs_ext", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: NODE_MODULE_INIT uses NODE_GYP_MODULE_NAME as an identifier, so this must be a valid identifier.
@@ -19,7 +19,7 @@ | |||
"node": ">= 8.0.0" | |||
}, | |||
"dependencies": { | |||
"nan": "^2.14.0" | |||
"nan": "^2.21.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Build was failing with nan 2.14.
@baudehlo hey, not to be a noodge, but have you had a chance to look at this? |
I haven't - someone else took over maintenance of this module, but maybe
they are awol now. It looks fine, and I'm happy to merge.
…On Wed, Oct 23, 2024 at 10:52 AM Bryan Elliott ***@***.***> wrote:
@baudehlo <https://github.com/baudehlo> hey, not to be a noodge, but have
you had a chance to look at this?
—
Reply to this email directly, view it on GitHub
<#109 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFBWYYVG37V76ZPW7NNHRDZ46Z4HAVCNFSM6AAAAABPXL4TFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZSGQ4TQNRUGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Been a week, the maintainer hasn't responded. Please merge. |
I merged it, but I've been trying to add Actions tests (it was originally setup using Travis), and the tests don't pass: #110 |
Haha. Mandatory, "it works on my local". I'll have a look and see if I can figure out what's going wrong. |
Ah. Run |
Fixes: #98
This PR modifies the node module initialization to give it minimal context-awareness, such that it can work within a
node:worker_threads
worker.See: https://github.com/nodejs/node/blob/main/doc/api/addons.md#context-aware-addons