websocket's native dependencies

[alcuadrado]

Hi,

I'm creating this issue because at Nomic Labs and Tenderly we've been working on removing node-gyp based native dependencies form the ecosystem. We already mentioned this in other issues/pull requests here. There's more info here: ethereum/js-team-organization#18

One of the few packages that requires recompiling native dependencies on install is web3, as it uses websocket. Note that as web3 is used by lots of other tools, this project won't finish here, but we need to coordinate with many web3 consumers so they update their versions.

Intially, we tried replacing websocket with wc, but that's not feasible, so we tried to fix this upstream. There's already a PR doing it, which the author of websocket said is ok but never merged nor released.

I'm opening this issue to discuss if web3 shouldn't fork websocket and incorporate those changes instead of waiting for an upstream release. This would let us continue this effort while we wait for upstream websocket to catch up.

This decision shouldn't be taken lightly, so here's an explanation of what the fork does:

• websocket has two tiny modules that are written in C -- just utf8 validation and some Buffer helpers.
• These are compiled whenever you install websocket
• These modules were copied verbatim from wc
• wc keeps those modules as separate npm packages
• Those packages were migrated to N-API, so they don't require recompilation now
• The websocket PR removes the imported modules, and used wc's npm packages instead.
• There aren't any functional changes in the PR

What do you think?

/cc @nebojsa94

[GregTheGreek]

This makes a lot of sense. node-gyp is a parasite during build.

I think if the fork is as described I wouldn't have an opposition to using this, as long as upstream changes are always kept up to date. We'd greatly appreciate being apart of the forks updates (being tagged is fine, happy to also review) since this could have a large impact.

I haven't taken time to review the current PR, perhaps once the fork occurs then I'll dive into it?

[alcuadrado]

Thanks for your reply, Greg!

I forked into https://github.com/nomiclabs/WebSocket-Node and released @nomiclabs/websocket@1.0.31. We'll make sure that it's up to date with the upstream version. Once these changes are actually implemented, I'll change the fork to just re-export the upstream module.

The relevant diff is just this commit, which mostly deletes files, adds two dependencies, and uses those instead.

@nebojsa94, is any other change apart from websocket needed in web3 to remove node-gyp?

[GregTheGreek]

@alcuadrado sorry so is the plan that nomic will continue to maintain the fork?

Is there a reason for this?

[alcuadrado]

I think I misunderstood your last comment, @GregTheGreek.

I thought that you were willing to consider this if there was already a fork, so I went ahead and forked it. We are ok with maintaining the fork, even if web3 is the only consumer.

If you think that the fork should be managed by web3, that's even better for us. TBH this is the option I'd choose if I were maintaining web3, so feel free to do it.

For context, websocket releases 1-3 patch versions per year, so maintaining the fork is not much work, and I'm hopeful that these changes will be merged upstream soon-ish.

[GregTheGreek]

Ahh, ok I misunderstood your comment before as well.

Thanks for forking it! I gave it a test and tagged you, it's quite promising!

Id be happy to maintain it. Do you mind if I fork your changes ?

[nebojsa94]

is any other change apart from websocket needed in web3 to remove node-gyp?

Nope, that is the only one!

[alcuadrado]

Id be happy to maintain it. Do you mind if I fork your changes ?

No problem!

[theturtle32]

@alcuadrado - I've incorporated the N-API changes upstream and released a new version of websocket on npm as v1.0.32 that no longer uses node-gyp.

[alcuadrado]

Thanks a lot, @theturtle32! 🙌

[GregTheGreek]

@theturtle32 :This_is_me_jumping_for_joy: thanks!!!