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

Enhancement: migrate from nan to node-addon-api (N-API) #1282

Closed
Nantris opened this issue Jun 29, 2018 · 12 comments
Closed

Enhancement: migrate from nan to node-addon-api (N-API) #1282

Nantris opened this issue Jun 29, 2018 · 12 comments
Milestone

Comments

@Nantris
Copy link

Nantris commented Jun 29, 2018

I'm trying to use this with Electron and run some tests in Node but I absolutely cannot get a working version for each runtime working simultaneously. It always puts the built module into Release\sharp.node, overwriting the previously built version.

If I build for electron, I cannot test. If I build for Node, I cannot run it in Electron.

When I build for Electron, my tests return the following:

The file it says it cannot find does exist

 FAIL  app/redux/sagas/firebase/oneTime/__tests__/helpers.test.js
  ● Test suite failed to run

    The specified module could not be found.
    \\?\C:\projects\myApp\app\node_modules\sharp\build\Release\sharp.node

      Error: The specified module could not be found.
      \\?\C:\projects\myApp\app\node_modules\sharp\build\Release\sharp.node
      at Runtime.requireModule (node_modules/jest-runtime/build/index.js:372:31)
      at Object.<anonymous> (app/node_modules/sharp/lib/constructor.js:10:15)

When I build for Node, my app's renderer throws this error:

ELECTRON_ASAR.js:172 Uncaught Error: The system cannot find message text for message number 0x%1 in the message file for %2.
\\?\C:\projects\myApp\app\node_modules\sharp\build\Release\sharp.node
    at process.module.(anonymous function) [as dlopen] (ELECTRON_ASAR.js:172:20)
    at Object.Module._extensions..node (module.js:671:18)
    at Object.module.(anonymous function) [as .node] (ELECTRON_ASAR.js:172:20)
    at Module.load (module.js:561:32)
    at tryModuleLoad (module.js:504:12)
    at Function.Module._load (module.js:496:3)
    at Module.require (module.js:586:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (C:\projects\myApp\app\node_modules\sharp\lib\constructor.js:10:15)
    at Object.<anonymous> (C:\projects\myApp\app\node_modules\sharp\lib\constructor.js:259:3)
@lovell
Copy link
Owner

lovell commented Jun 30, 2018

Hello, this is more a current limitation of native modules in Node rather than being a problem specific to sharp.

Different versions of Node and Electron each have different ABIs, hence the need to ensure compilation occurred against the relevant runtime.

Node provides a new ABI-stable N-API that aims to solve this problem. To take advantage of this we need to:

@Nantris
Copy link
Author

Nantris commented Jun 30, 2018

Understood. Thanks very much for the explanation @lovell. Is there no way to use different release directories for different binaries under the current system? I get that there's no good way, but I'd be okay with hacking things together in the meanwhile. Unfortunately I wouldn't have the slightest clue how to do it, if it can be done at all.

It seems like some modules don't have this problem, gRPC for example. @murgatroid99, can you mention how gRPC approaches this? I know whatever gRPC and some other modules are doing works currently on Node >8.9.3, though perhaps there's just no differences in the binaries between Node and Electron in those cases?

@murgatroid99
Copy link

For gRPC we distribute separate binaries for Node and Electron, and node-pre-gyp has a feature that we use that parameterizes the name of the directory the binary is installed in based on which variant of the binary is being installed. So instead of creating build/Release/grpc_node.node, it creates something like src/node/v64-linux-x64-glibc/grpc_node.node or src/node/electron-2.0-windows-ia32-unknown/grpc_node.node.

@lovell
Copy link
Owner

lovell commented Jul 1, 2018

sharp uses prebuild to create separate binaries for Node and Electron.

prebuild-install only downloads the relevant one at install-time and probably doesn't re-check at runtime, but it does have an install-time --runtime flag that might be worth experimenting with.

@Nantris
Copy link
Author

Nantris commented Jul 1, 2018

Thanks very much to you both for your insightful answers.

@lovell lovell changed the title Can't simultaneously build for Electron and Node Enhancement: migrate from nan to node-addon-api Sep 4, 2018
@lovell lovell changed the title Enhancement: migrate from nan to node-addon-api Enhancement: migrate from nan to node-addon-api (N-API) Feb 20, 2019
@metabench
Copy link

@lovell Could anything be used as an alternative to prebuild?

"Migrate sharp from nan to node-addon-api" - That sounds like something that can happen independently of waiting for other things to be ready (depending on developers' interest in doing so).

Would there be a way to get Sharp to work on either node or electron without needing to be compiled differently?

@lovell
Copy link
Owner

lovell commented Feb 21, 2019

The prebuild changes have landed (although it looks like the linked issue hasn't been updated to reflect that) so that's no longer a problem. N-API is still marked as experimental in Node 6 but the use of node-addon-api might alleviate that now. Happy to accept a PR if you're able.

@metabench
Copy link

I doubt I'll have time (or the right experience) to do this really soon. I've recently concluded that tensors provide a useful abstraction for image processing, and am going to focus on https://github.com/metabench/ta-tensor for the moment. I plan on making a C or C++ addon and also compiling to WASM for use in the browser. I'm interested in Sharp both because I've been using it, and to find out about best practises for making data-intensive addons. If it turns out that some new techniques I use would be useful to Sharp then I'd be in a better position to implement them in Sharp in the future.

@lovell
Copy link
Owner

lovell commented Jul 1, 2019

Based on my slightly painful experience in another module of migrating to N-API across various versions of Node 8, where different versions of Node, Electron and N-API all report the same ABI but don't even support the same API, upgrading to use N-API will have to wait until after we remove support for Node 8 at the end of 2019.

@lovell
Copy link
Owner

lovell commented Feb 4, 2020

An organisation that relies on sharp has very kindly offered to pay for the migration to N-API so expect to see some progress on this task over the next couple of weeks.

@lovell
Copy link
Owner

lovell commented Feb 9, 2020

The latest work-in-progress for N-API migration is now available for early/alpha testing on the yield branch.

https://github.com/lovell/sharp/tree/yield

19 changed files with 567 additions and 711 deletions

npm install lovell/sharp#yield

lovell added a commit that referenced this issue Feb 15, 2020
@lovell
Copy link
Owner

lovell commented Mar 7, 2020

v0.25.0 now available with the use of N-API. I'm sure the maintainers of the many modules that depend upon sharp will be happy with the reduced level of installation issues this will bring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants