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

replace SetAccessor -> SetNativeDataProperty #977

Merged
merged 1 commit into from
Oct 10, 2024
Merged

replace SetAccessor -> SetNativeDataProperty #977

merged 1 commit into from
Oct 10, 2024

Conversation

kkoopa
Copy link
Collaborator

@kkoopa kkoopa commented Oct 10, 2024

This should finally close #973. I will let the tests run, expect a release tomorrow.

@bugsounet
Copy link

Hi, just tested with electron v32.2.0 and electron-rebuild with node-pty, drivelist, node-gpiod, usb, node-detector

Result:

✔ Rebuild Complete

@agracio
Copy link

agracio commented Oct 10, 2024

Works for Electron 29+ but fails for previous versions based on Node.js 18.

nan.h(2621,8): error C2665: 'v8::ObjectTemplate::SetAccessor': no overloaded function could convert all the argument types

However does work for native node modules that rely on Node.js 16 and 18 without Electron dependency.

For me personally Electron below version 29 is not relevant but is likely relevant to other projects.

UPDATE: Electron foundation only supports versions 30+ with all previous versions including Electron 29 out of support.

@kkoopa
Copy link
Collaborator Author

kkoopa commented Oct 10, 2024

Node 18 works fine: https://ci.appveyor.com/project/RodVagg/nan/builds/50770411 Electron's frankenbuilds are not Node.js.

@agracio
Copy link

agracio commented Oct 10, 2024

I am fully aware of that and that was not really a complaint since Electron versions below 30 are out of support. As per my comment native modules using Node.js 16 and 18 that do not rely on Electron build perfectly fine.

@agracio
Copy link

agracio commented Oct 10, 2024

Apologies for spamming so many comments. Tested building my native (non Electron) node modules using node.exe 23.0.0 built from main branch and this PR - success.

@kkoopa
Copy link
Collaborator Author

kkoopa commented Oct 10, 2024 via email

@kkoopa kkoopa merged commit 6bd62c9 into main Oct 10, 2024
@kkoopa kkoopa deleted the n23 branch October 10, 2024 21:12
@danielweck danielweck mentioned this pull request Oct 16, 2024
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.

support for electron 32
3 participants