-
-
Notifications
You must be signed in to change notification settings - Fork 18
Description
Hi,
I'm currently looking at providing a Node API for c8
, a code coverage tool using V8's Profiler.
The current code uses spawn-wrap
. Since spawn-wrap
modifies the global child_process
module, it is not safe to use it in a library: concurrent code trying to spawn processes will have unexpected results. This prevents running multiple coverages in parallel, or coverage and some other consumer tasks, inside the same Node process.
The current workaround would be to run each instance of c8
in its own process (independent child_process
). This is very heavy-handed: it introduces friction and hurts composability. It means that the API must communicate with a subprocess so only serializable messages can be sent.
I propose to do something similar to graceful-fs@4
. The version 3
modified the global fs
module while version 4
only returned a wrapper.
Here is what the updated API would look like:
// Default to safe version (breaking change)
module.exports = safeWrap;
// Alias so the API can be a simple namespace
module.exports.wrap = safeWrap;
// Keep the old behavior available to ease transition, eventually mark as deprecated?
module.exports.wrapGlobal = wrap;
// Helper function to know if the global child_process is currently wrapped
module.exports.isGlobalWrapped = ...;
safeWrap(options) {
const spawn = ...;
const spawnSync = ...;
// Start with a subset of the `child_process` API,
// eventually extend it to return a full `child_process`-like object.
return {spawn, spawnSync};
}
// We could also imagine a function like "wrapify` similar to "gracefulify"
// You can pass it a `child_process`-like object and it would mutate it
// by adding the wrappers in-place.
As stated in the example, this would introduce a breaking change but I believe that it would bring better behavior. The old behavior would still be accessible so transition would be easy.
The breaking change would also be the occasion to require Node 4+: there are other comments/issues mentioning problems with older Node versions. It would also simplify the current feature-detection mechanisms. This is not my main point though, I'm just proposing it by the way
What is needed is a way to spawn concurrent process-trees that do not interfere with each other, without isolating each root in its own sub-process.
Edit: /cc @bcoe @isaacs @sindresorhus