node Reduce our patches for node

Currently we have about 20 commits based on upstream node's head, we should keep our patches as minimum as possible and try to get some of them into upstream.

Dec 12 '13 06:12 zcbenz

Tried to merge one patch to upstream: https://github.com/joyent/node/pull/6744.

Dec 20 '13 05:12 zcbenz

Nice. Glad you're doing this. Looks like they're open to it?

Dec 20 '13 18:12 nathansobo

Yeah they are not against it, but it's not easy to persuade them to accept it too.

Dec 21 '13 17:12 zcbenz

Any updates on this @zcbenz?

Jun 24 '14 17:06 probablycorey

No news yet, but I have added more information in that PR since atom had been publicized, hoping it could add chances to get our patches merged.

Jun 25 '14 04:06 zcbenz

A small patch merged to upstream: https://github.com/joyent/node/pull/8122.

Aug 15 '14 05:08 zcbenz

Nice work @zcbenz :sparkles:

Aug 15 '14 17:08 nathansobo

Another patch merged to io.js.

Jan 07 '15 22:01 zcbenz

Another one merged.

Jan 20 '15 23:01 zcbenz

@zcbenz is there a way we (io.js / node) can better support you in this? We'd like to help reduce the number floating patches required for embedders, if possible.

Jun 30 '15 17:06 Fishrock123

It looks like a good amount of the patches are actually on libuv. Have you thought about submitting them to https://github.com/libuv/libuv?

Jun 30 '15 18:06 Fishrock123

@Fishrock123 For simple patches I usually just create a pull request to the repo, like https://github.com/libuv/libuv/pull/395, but most of our patches are dirty hacks, it takes a lot of efforts to make them decent APIs with test cases, which I don't have time to do now. But eventually when I got time I would love to merge them to upstream.

Jul 05 '15 09:07 zcbenz

Right, but don't be afraid to open issues for some things too! We do need to know what is necessary to make embedders' lives better. :)

Jul 07 '15 18:07 Fishrock123

Hello all, If you need someone to help port stuff to mainline node, feel free to ping me.

but most of our patches are dirty hacks,

I'm quite sure that beside me there are other node contributors that would be happy to help turn hacks into APIs.

And RE: https://github.com/electron/node/commit/dc8fe9d390b1deb32ae2601357a90679cbcc757e there's a PR open to make those opt-in https://github.com/nodejs/node/pull/15454

Sep 28 '17 15:09 refack

@refack That would be great help!

Oct 02 '17 05:10 zcbenz

@refack That would be great help!

I'll take a look at the diff. If you could prioritize stuff, that would help me help you ;)

Oct 02 '17 14:10 refack

@refack I'm going to go through the patches in this issue.

Hide console window when creating process. https://github.com/electron/node/commit/e6afee64f8f214d95381f6f36616c88bad2b945f

Generally we should have an option in libuv and Node.js for this flag, but it is useless for console apps and in Electron (and other embedders) we definitely want to have it on by default.

So we can probably add a build flag in libuv to turn it on by default.

Make Module.globalPaths a reference. https://github.com/electron/node/commit/f4ad6b7fed438d1bdde11a6f3c93bdf2bd63702c

We need this for security, so desktop apps won't accidentally load a module installed on user's system.

Maybe adding adding a new flag like what we did with process._noBrowserGlobals?

Don't set wrapper class in node, otherwise heap snapshot would crash. https://github.com/electron/node/commit/6a24dd7deda25ac81e1932edab2c202ed743cc26

We commented that code out because it conflicts with blink web engine, no idea how to do with it.

Guard against NULL returned by uv_err_name. https://github.com/electron/node/commit/124a0f2e428b03da18465f657fb38e2d85cf8220

Some antivirus and firewall softwares can block socket connections from other software clients, on our side we would get an unnormal errorno which would make uv_err_name return nullptr. I think the biggest challenge is how to represent an unknown error.

Oct 03 '17 11:10 zcbenz