Skip to content

N-API#2

Closed
argv-minus-one wants to merge 1 commit into
LinusU:masterfrom
argv-minus-one:napi
Closed

N-API#2
argv-minus-one wants to merge 1 commit into
LinusU:masterfrom
argv-minus-one:napi

Conversation

@argv-minus-one
Copy link
Copy Markdown

Currently, capture-window doesn't work on Node 12. This PR fixes that (and prevents any other future V8 API breakages) by converting it to use N-API.

@LinusU
Copy link
Copy Markdown
Owner

LinusU commented Nov 6, 2022

Sorry for the extremely long delay on this 🙈 I must have totally missed this one.

I would prefer to not have a dependency on node-addon-api, so I took a slightly different approach here: #4. What do you think of this?

I'm also interested in why some of the flags was added in the gyp bindings file. Would you be able to explain the background on these?

@argv-minus-one
Copy link
Copy Markdown
Author

Glad to hear from you again! Unfortunately, it's been so long I honestly remember almost nothing about this PR, so I can't tell you why I did what I did.

I do vaguely remember is that this PR was pretty quick-and-dirty. It looks like you've done a bunch of work on #4, you should probably just go with that.

@LinusU
Copy link
Copy Markdown
Owner

LinusU commented Nov 7, 2022

I understand, again, I'm really sorry that I missed this. I only noticed it now because I started working on the repo, so the notification must have gotten lost. I'll go with #4 for now, we can also add back the flags later if someone figures out what they are for ☺️

Thank you so much for taking the time to work on this 🙏

@LinusU LinusU closed this Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants