Windows electron

[kjvalencik]

Fixes #399
Fixes #389
Fixes #394
Fixes #194

Replaces #404

[joshuef]

This is working well on windows 10 w/ https://github.com/joshuef/safe-nodejs in the safe browser 🎉

[dherman]

This is exciting! I learned some cool techniques from the PR too, like using xvfb and spectron to test Electron apps in CI. 👍

I left a few questions inline. And a few more:

• Does this need any changes to the Electron guide?
• What were your thoughts on the whole question of an --electron flag versus trying to automatically work both inside and outside Electron?
• What were your thoughts on eliminating the need for electron-build-env? Still worth investigating but not part of this PR?

[kjvalencik]

@dherman

• Does this need any changes to the Electron guide?

No changes necessary.

• What were your thoughts on the whole question of an --electron flag versus trying to automatically work both inside and outside Electron?

I don't think adding a flag is necessary. The only difference is linking win delay. The node-gyp docs suggest that this is almost always safe (it's the default). Instead of an --electron flag, it may be better to always link it. Thoughts?

• What were your thoughts on eliminating the need for electron-build-env? Still worth investigating but not part of this PR?

I think it's worth investigating as a separate PR.

[dherman]

I think if it "almost always" works, we can experiment with just doing this by default and learning where, if anywhere, it breaks. Once we can characterize that we can think about e.g. how to maybe create an opt-out flag.

[dherman]

LGTM!

[joshuef]

Total side note, but for testing electron apps, I'd hiiighly recommend testcafe over spectron. I've found it to be wayyyy more stable and easier to use too!

[marcmo]

Hello @kjvalencik , great to hear you fixed some issues on Windows and Linux. we are experiencing some of those issues (#394). any plans when you want to include these fixes in a neon release?
I tried including all the neon crates from master but couldn't build anymore so I guess everything has to match up.

[kjvalencik]

@marcmo Yes, they all need to be inline. We are in the process of merging a fairly large PR that is the prep work for N-API.

I will cut a release later this week after that lands. Thanks for your patience!

[terryzhao]

@marcmo Yes, they all need to be inline. We are in the process of merging a fairly large PR that is the prep work for N-API.

I will cut a release later this week after that lands. Thanks for your patience!

Hello @kjvalencik Glad to hear that you will publish this release. I'm waiting for this release so eagerly!:smile: