The ShareStream constructor is wrong

[hdgarrood]

My intention was that you would be able to put any kind of Stream with any row of effects into the ShareStream constructor, which is what led me to write it as it is now:

data StdIOBehaviour = ShareStream (forall r eff. Stream r eff) | [...]

However, pretty much any Stream you might come across is going to have something in both of these rows, so as far as I can tell, you pretty much have to use unsafeCoerce to actually use this constructor.

Possible solutions:

• Define newtype StdIOBehaviour = StdIOBehaviour Foreign and replace the constructors with normal functions. So eg Pipe would become a normal value pipe :: StdIOBehaviour, and ShareStream would become shareStream :: forall r eff. Stream r eff -> StdIOBehaviour. You lose pattern matching by doing it this way, but it is very simple, and if a user of this library really wanted to do some kind of case analysis, they could use purescript-foreign.
• Keep the ADT and use purescript-exists so that we have something like ShareStream (exists r. exists eff. Stream r eff) (although of course we can't actually do that). I guess it would have to be something like data StdIOBehaviour = ShareStream (Exists (Exists Stream)) which is a bit weird.
• Keep the ADT and define an opaque AnyStream type for use with the ShareStream constructor. So something like this:

-- A stream where you don't know whether it's readable or writable,
-- or what the effects associated with reading/writing are.
foreign import data AnyStream :: *

toAnyStream :: forall r eff. Stream r eff -> AnyStream

-- It is the caller's responsibility to check that the `Stream` being returned
-- has the appropriate type.
unsafeFromAnyStream :: forall r eff. AnyStream -> Stream r eff

data StdIOBehaviour = ShareStream AnyStream | [...]

[paf31]

What about

data StdIOBehaviour r eff = ShareStream (Stream r eff) | [...]

and then specifying r depending on the particular stream (stdin etc.) and propagating eff to the FFI definitions which use the options object?

[hdgarrood]

I can't see how to make that work well, because different StdIOBehaviour values corresponding to different streams all have to go into an array together, and often these different streams have different values for r (in particular, stdin, stdout, and stderr often appear there together); relevant docs: https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_options_stdio

I'm not all that keen on propagating effect rows here either, because then you'd have to make sure they line up when you use spawn and that might be a pain, but also if you're creating child processes the compiler isn't going to be able to tell you what effects that might cause, so I'm not sure effect tracking is going to pay off here.

[paf31]

On the array side, you could replace it with a record, since there are always three things in the array, right?

[hdgarrood]

I don't think there necessarily are, at least node seems to happily spawn things if I specify stdio: [] or stdio: ['pipe']. Although perhaps leaving them out is the same as passing null, so I'd guess [] means the same as [null,null,null] and ['pipe'] means the same as ['pipe',null,null]. There are sometimes more than three things in the array, too.

[hdgarrood]

Effect simplifies this :)

[JordanMartinez]

This has been resolved in the new way of handling stdio via the safe ChildProcess and the unsafe UnsafeChildProcess.