"Class property and descriptor" example crashes in Electron render process on reload

[ToadKing]

PoC Project: https://github.com/ToadKing/electron-native-module-poc (The native module part is an exact copy of the example in the node-addon-api docs.)

The class example in the docs makes the constructor reference persistent using Napi::Persistent on module initialization. However, in Electron when a render process reloads a page, the native module is re-initialized without reloading the shared library itself. When a new constructor reference is loaded into the constructor variable it tries to delete the former reference, which causes a crash of the Electron render process.

[KevinEady]

Hi @ToadKing ,

Thanks for pointing this out. The example in the documentation predates some modern APIs for handling context-sensitivity, particularly SetInstanceData and GetInstanceData. You can update https://github.com/ToadKing/electron-native-module-poc/blob/76d02d8c7cb151f3a657734836012629fc2f4f8c/native/src/binding.cc#L23-L24 with:

    Napi::FunctionReference *constructor = new Napi::FunctionReference();
    *constructor = Napi::Persistent(func);
    env.SetInstanceData(constructor);

electron-reload.mp4

[NB: SetInstanceData will overwrite any existing instance data set. See #744 for some examples on how to store multiple constructors on the instance data.]

[KevinEady]

Note that SetInstanceData is only available on Node-API 6+, see Node-API version matrix.

[ToadKing]

Thank you, this change fixes my PoC app and the app I found it in. Should this issue be left open and be changed to update the documentation or is it good to close?

[KevinEady]

@ToadKing I am not sure... We will leave it open to discuss at this week's Node-API meeting. We'll need to discuss a hopefully context-sensitive approach for Node-API version <= 5. I will leave this issue open until we discuss it.

Thanks, Kevin