electron / node-abi

:turtle: :rocket: Get the Node.js and Electron ABI for a given target and runtime
https://www.npmjs.com/node-abi
MIT License
164 stars 58 forks source link

Add support to node-webkit target. #34

Closed saboya closed 6 years ago

saboya commented 6 years ago

This PR adds support to node-webkit target, necessary to build native modules when using NW.js, an Electron alternative.

Tests are included.

NW.js apparently does not follow a specific versioning pattern to switch ABI, so the getNextTarget function can't be implemented correctly for node-webkit, so I left it untouched.

ralphtheninja commented 6 years ago

LGTM!

lgeiger commented 6 years ago

@saboya Thanks for adding this :+1:

@ralphtheninja wouldn't this be a breaking change for prebuild since this PR modifies allTargets which is used to determine the default ABIs to prebuild against?

Is this behavior change in behavior desired? If so we should do a new major release, else I would recommend adding node-webkit to something like additionalTargets so we don't break earlier versions of prebuild.

ralphtheninja commented 6 years ago

Is this behavior change in behavior desired? If so we should do a new major release, else I would recommend adding node-webkit to something like additionalTargets so we don't break earlier versions of prebuild.

Moving to additionalTargets is probably a good idea.

ralphtheninja commented 6 years ago

@lgeiger Maybe should have a discussion on what is ok for default behavior of prebuild. I'll open an issue.

ralphtheninja commented 6 years ago

@lgeiger I've made PR to adjust this. Hold off with merge.

ralphtheninja commented 6 years ago

Some research below:

screenshot from 2018-01-28 16-09-42

screenshot from 2018-01-28 16-10-20

lgeiger commented 6 years ago

@saboya @ralphtheninja Thanks for your work!

This is released under 2.2.0