Proposal for Standardising API that can return buffered results vs Node.js Streams vs Pull Streams

[daviddias]

tl;dr; this is a proposal to standardize the API definitions of function calls that may return buffered values, Node.js Streams or pull-streams.

I'll make this proposal and succinct as possible, with the hopes we can get around it asap.

Our current needs and requirements:

• Users want to be able to consume data coming from calls (i.e: files.{cat, get, ls}) as a single result value. It is extremely convinient.
• We know that buffering everything doesn't work for a big part of the cases.
• We like to use pull-streams everywhere because it saves us from some Node.js hiccups and makes it extremely convenient between modules.
• Users still to have access to Node.js Streams, since they are the most widely use.
• We have a strong commitment to not breaking user-space, however, if this standardization proves to be popular and more user-friendly, I believe we can open an exception as long as we don't loose functionality (only some name changing), under the condition to have a smooth transition.

Essentially we have two options on the table

a) Use option arguments to specify what we are looking for

ipfs.files.cat(cid, {
  buffer: true         // buffer all the results and return a single value
  // PStream: true // return a Pull-Stream
  // NStream: true // return a Node.js Stream
}, callback)

b) Use different method names

ipfs.files.cat(cid, callback)
// or
ipfs.files.catPStream(cid)
// or
ipfs.files.catNStream(cid)

This option has a couple of advantages:

• interpreter friendly - by having a method for each type, it will enable V8 to optimize it better
• comparing to what we have today, it will enable the functions that return streams to return streams, instead of the 'callback with a stream'. Enabling things like ipfs.files.catNStream(cid).pipe(process.stdout) without going into callback. (The reason why we had to return a stream in a callback, was because of Promises, but that stays on ipfs.files.cat only.

Once this proposal goes forward we need to:

• Update the API definitions
• Update the tests
• Update the implementations
• Update the internals that exposes pull-streams with something like getStream, to be getPStream.

[daviddias]

@ipfs/javascript-team I believe you all might want to chime on this.

[dignifiedquire]

👍 for proposal b from my side, not sure about the names and naming is hard so go ahead with these :)

[nicola]

I am for option b,

what about

ipfs.files.cat(cid, callback)
// or
ipfs.files.catPullStream(cid)
// or
ipfs.files.catStream(cid)

?

[daviddias]

@dignifiedquire @nicola thank you for the feedback!

With regards to the naming. I don't have strong feelings about 'catPStream' and 'catNStream', I kind of like @nicola suggestion. The only that feels weird to me from all the options shared was: 'catPull' without the 'Stream' keyword.

[victorb]

Yeah, @nicola's suggestion makes sense to me too. Divide the methods would be the most beneficial and least confusing (more arguments/options for a function, the less intuitive). Also, regarding the naming, using full names like catPullStream and catStream makes it easier to assume or remember what is returned, rather than using shortnames that can be confusing.

[daviddias]

We need to provide this change the sooner the better. Streams are still confusion for devs.

One thing to consider is to s/catStream/catReadableStream since we will use in fact https://www.npmjs.com/package/readable-stream and not Node.js core streams to ensure compatibility and stability across Node.js versions.

We also should consider aliases

• catReadableStream -> catRS
• catPullStream -> catPS

[dignifiedquire]

The short names are reeeeaally hard to distinguish with P and R looking almost the same, so I think we should go for the long versions.

[dignifiedquire]

Not sure ReadableStream is good though, given that they sometimes might be duplex or other kind of streams, so could be more confusing.

[dignifiedquire]

Maybe we could do

// callback
files.cat(cid, callback)
// node stream
files.cat(cid)
// pull streams
files.catPull(cid)

It's easy to detect argument count and this might make it a bit less awkward to find correct names