refactor: IPC message forwarding #27
Conversation
demurgos
force-pushed
the
refactor-proxy-messages
branch
2 times, most recently
from
ff67c87
to
96e195e
Compare
|
The CI failure seems to be an Appveyor issue (issue in their |
| @@ -4,6 +4,9 @@ if (process.platform === 'win32') { | |||
| spawn = require('cross-spawn') | |||
| } | |||
|
|
|||
| function noop() { | |||
There was a problem hiding this comment.
Because it's a no-op function. It avoids to redefine an empty function every time I need one. (in this PR it's used only once, but there are other places in the file).
| child.send(message, handle) | ||
| } | ||
|
|
||
| child.on('message', childListener) |
There was a problem hiding this comment.
Closure would have been cleaner in IMO:
child.on('message', function (message, handle) {
child.send(message, handle);
}Same for L122.
There was a problem hiding this comment.
You can't unproxy it if you do not have a reference to the function. The current code does not completely cleans up the proxies, but the end goal is to have a general proxy function with proper clean up.
demurgos
force-pushed
the
refactor-proxy-messages
branch
3 times, most recently
from
4667fc2
to
4fdf3e9
Compare
demurgos
force-pushed
the
refactor-proxy-messages
branch
from
4fdf3e9
to
7b00697
Compare
|
This looks good to me, but fails test coverage. |
demurgos
force-pushed
the
refactor-proxy-messages
branch
from
7b00697
to
b1cc170
Compare
# Why `foregroundChild` is currently hard-coded to only support forwarding with the root process. To support a general `proxy` function between any two objects behaving like processes, we can begin by refactoring the internals to be more parametrized. # What Move the start of the IPC message forwarding to its own function.
demurgos
force-pushed
the
refactor-proxy-messages
branch
from
b1cc170
to
47e2fdf
Compare
|
This should be a cross-platform feature. The failure is caused by the lack of tests for the clean-up when cancelling the proxy. I'll add some tests. |
|
@demurgos 👋 is this is still something you'd like to land feel free to reopen, it just needs some tests added and we could see it over the finish line. |
Why
foregroundChildis currently hard-coded to only support forwarding with the root process. To support a generalproxyfunction between any two objects behaving like processes, we can begin by refactoring the internals to be more parametrized.What
Move the start of the IPC message forwarding to its own function.