This follows the fix for Electron v32:

• Support for node version 20.17.0 #976
• replace SetAccessor -> SetNativeDataProperty #977

See: #973 (comment)

Electron 33 ships with Node 20.18.0:

https://github.com/electron/electron/releases/tag/v33.0.0

Electron 33 moved to v8 13.0 while Node.js main is currently on v8 12.9, perhaps someone can create a working fork that addresses the problem?
Unless by some miracle Node.js moves to v8 13.0 for its v23 release I do not think nan will address the issue themselves.

Great point @agracio ... pinging @toshiyuki-rs @oc-soft @kkoopa @bugsounet

You should not ping nan maintainers as they have been quick to point out that they do not support and have no intention to support Electron in the future.

Apologies, I thought you were the NAN maintainer :) 👍 (very quick response indeed, thank you)

Well, my workaround is to comment #include nan_scriptorigin.h in nan.h:

(I do this with sed from my package.json postinstall)

Closing the issue now.

It is probably this commit. 0151569 I will get around to it at some future time.

Thanks!

@danielweck could you share your package.json script with postinstall

And can someone with Electron 33 confirm that applying 0151569 resolves the issue?

Will do but give me about 30+ minutes.

@danielweck could you share your package.json script with postinstall

Not pretty, quick and dirty, but works in MacOS and Linux:

"postinstall": "sed 's/^#include .nan_scriptorigin\\.h./\\/\\/ #include nan_scriptorigin.h/' ./node_modules/nan/nan.h > ./node_modules/nan/nan.h.new && mv ./node_modules/nan/nan.h.new ./node_modules/nan/nan.h"

And can someone with Electron 33 confirm that applying 0151569 resolves the issue?

It absolutely does, tested Electron 29, 30, 31, 32 and 33 that are supported by nan 2.22.

@kkoopa So is it just a issue of merging that commit and everything will be working?

@danielweck this issue is still valid and should be open until a fix is landed

The issue is not present in nan v2.22.2 tested with electron 29-34 as well as 35 beta and latest 36 nightly.

Apologies, I thought you were the NAN maintainer :) 👍 (very quick response indeed, thank you)

Well, my workaround is to comment #include nan_scriptorigin.h in nan.h:

nan/nan.h

Line 3045 in f5921c7

#include "nan_scriptorigin.h" // NOLINT(build/include)
(I do this with sed from my package.json postinstall)

Closing the issue now.

Hi @danielweck
But this postinstall will not work fine in case using nan by subDependencies, so I think this issue should be reopened as needs a fix...
Anyone can take a look at it, please?

EDIT:
The issue is fixed on nan v2.22.2 so overrides the version to the latest version that's released 4 days ago(`v2.22.2) will fix the issue.

While the issue is solved in nan v2.22.2 postinstall overrides can be easily applied for sub dependencies.
Here is a sample script from my repo:

updatenan.js

const spawn = require('child_process').spawn
const existsSync = require('fs').existsSync
const nanPath = existsSync('./node_modules/nan/nan.h') ? './node_modules/nan/nan.h' : '../nan/nan.h'

if(existsSync(nanPath)){
    if (process.platform === 'win32') {
        spawn('powershell', ['-Command', `(Get-Content -Raw ${nanPath}) -replace '#include \"nan_scriptorigin.h\"', '// #include \"nan_scriptorigin.h\"' | Out-File -Encoding Utf8 ${nanPath}`], { stdio: 'inherit' })
    } else {
        spawn('sed', ['-i', '-e', 's/^#include .nan_scriptorigin\\.h./\\/\\/ #include nan_scriptorigin.h/', nanPath], { stdio: 'inherit' })
    }

}

While the issue is solved in nan v2.22.2 postinstall overrides can be easily applied for sub dependencies. Here is a sample script from my repo:

updatenan.js

const spawn = require('child_process').spawn
const existsSync = require('fs').existsSync
const nanPath = existsSync('./node_modules/nan/nan.h') ? './node_modules/nan/nan.h' : '../nan/nan.h'

if(existsSync(nanPath)){
if (process.platform === 'win32') {
spawn('powershell', ['-Command', (Get-Content -Raw ${nanPath}) -replace '#include \"nan_scriptorigin.h\"', '// #include \"nan_scriptorigin.h\"' | Out-File -Encoding Utf8 ${nanPath}], { stdio: 'inherit' })
} else {
spawn('sed', ['-i', '-e', 's/^#include .nan_scriptorigin\.h./\/\/ #include nan_scriptorigin.h/', nanPath], { stdio: 'inherit' })
}

}

Also just adding overrides to the package.json should fix the issue for sub dependencies instead of using scripts

,
  "overrides": {
    "nan": "2.22.2"
  }

I removed that script from my repo since the release of nan v2.22.2 just wanted to show a PoC of install/postinstall script that works for sub dependencies. Cannot even take a credit for it as it was a PR from one of the repo contributors.