search

awslabs / aws-crt-nodejs

NodeJS bindings for the AWS Common Runtime.
Apache License 2.0
37 stars 24 forks source link

Add win_delay_load_hook #525

Closed xiazhvera closed 4 months ago

xiazhvera commented 5 months ago

Issue #, if available:

Description of changes: This change will fix dll load issue on Windows. It would allow us to use Electron and vercel/pkg.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

xiazhvera commented 5 months ago

I'm very uncomfortable with making a change that affects everyone and switches from C to C++. I think we should push the maintainer of cmake-js to add a build option that would cause their copy of win_delay_load_hook to be treated as C.

The primary blocker there looks to be finding a replacement for the decltype shorthand in the last line. It sure seems like it could just be replaced by the actual function signature above since it appears to be a fixed ABI contract.

We can probably add our own win_delay_load_hook implementation instead of using cmakejs version. Let me update it.

xiazhvera commented 5 months ago

Is there any way to add a CI step that makes and runs a simple electron exe on windows?

As all our samples are in js-v2 branch, the electron sample would also launched there:https://github.com/aws/aws-iot-device-sdk-js-v2/pull/398 . The Windows CI requires this PR to pass.

I also created an quick electron sample to verify the change: https://github.com/awslabs/aws-crt-nodejs/pull/526. Checkout windows and windows-vc14-x86 section Run PubSub Sample