feat(node/cluster): cluster module for Node compat

[cmorten]

Implements majority cluster module for Node compat.

Tests and full functionality not implemented owing to missing:

• process.send() - https://nodejs.org/api/process.html#processsendmessage-sendhandle-options-callback
• process.connected - https://nodejs.org/api/process.html#processconnected
• process.on("message") - https://nodejs.org/api/process.html#event-message
• process.on("disconnect") - https://nodejs.org/api/process.html#event-disconnect
• ChildProcess.prototype.send() - https://nodejs.org/api/child_process.html#subprocesssendmessage-sendhandle-options-callback
• ChildProcess.prototype.connected - https://nodejs.org/api/child_process.html#subprocessconnected
• domain.create - https://nodejs.org/api/domain.html#domaincreate

Which are likely blocked by denoland/deno#13566 so can be implemented in a follow-up.

[bartlomieju]

@cmorten are there any blockers for this PR?

[cmorten]

@cmorten are there any blockers for this PR?

The cluster module is basically there but it relies on process.send() (REF: https://nodejs.org/api/process.html#processsendmessage-sendhandle-options-callback) and a few other bits on process that haven’t been implemented yet, so just need to find some time to explore and port that across. Involves some IPC channel stuff and not sure yet if that’ll block it?

Guess similar to denoland/deno#12879

[cmorten]

Hmm maybe https://github.com/denoland/deno/pull/13566… worst case can tidy up with some notImplemented() placeholders and is at least a step closer

[cmorten]

Required to avoid errors with forked processes which start complaining because of the .ts extension - would need the require helper file to be .mjs for it to work... or just use --compat here (which opted for).

[kt3k]

I prefer .mjs here. It's more important to check that non-compat deno can consume std/node correctly in my view

[kt3k]

@cmorten Can you somehow get rid of circular dependency of net.ts -> cluster.ts -> internal/cluster/child.ts -> net.ts?

(circular dependency can be checked with the command ./_tools/check_circular_deps.ts

[cmorten]

@cmorten Can you somehow get rid of circular dependency of net.ts -> cluster.ts -> internal/cluster/child.ts -> net.ts?

(circular dependency can be checked with the command ./_tools/check_circular_deps.ts

Yeah aware 😅 Had tried but it’s not a simple one to fix… suspect might have to deviate somewhat from the Node file structure, but haven’t had time recently to try and attack!

Will put this back in draft in the meantime as it’s not ready without this solved.

[iuioiua]

Hi @cmorten, is there anything we can do to progress this PR? If you cannot do so, I'm sure someone will be happy to take it on.

[cmorten]

Hi @cmorten, is there anything we can do to progress this PR? If you cannot do so, I'm sure someone will be happy to take it on.

Hey @iuioiua 👋 Sorry for slow reply! Unfortunately been a matter of time (and then forgetting) that haven’t got around to finishing this off 😅. Unfortunately I don’t expect this will change in the coming weeks.

If anyone is wanting to progress or throw away these changes then please do (I seem to be making a habit with this, same happened when I implement net 🤣). I’ll see if I can get the merge conflicts sorted on my commute to make it more appetising.

IIRC the only blocker when I left it was circular deps as a result of being largely uninventive and mirroring the node implementation.

[iuioiua]

@cmorten, are you able to enable some native cluster-related Node tests? I'm curious whether they're affected while circular dependencies are present.

The contribution guide is in node/README.md.

[iuioiua]

Hi @cmorten, is this PR ready for review? CI is green 👍🏾

[cmorten]

Hi @cmorten, is this PR ready for review? CI is green 👍🏾

Yep all good to go

[kt3k]

Great progress in node compatibility! Thank you for your contribution! @cmorten

[lino-levan]

Wow, this was a lot of work. Great to see that it finally landed. Incredible job @cmorten!