bob icon indicating copy to clipboard operation
bob copied to clipboard

Proposal: change binding

Open Fishrock123 opened this issue 6 years ago • 8 comments

Binding is ugly and I bet it is the least comprehensible part right now.

Ideally I guess it would look something like Stream(source, transform, sink).

Fishrock123 avatar Jun 05 '19 23:06 Fishrock123

Stream(source, transform, sink, options) ?

In JS, options would be an object, in C++ it can be XOR flags.

jasnell avatar Jun 06 '19 01:06 jasnell

Needs an end callback somewhere (or, I guess, it could be a thenable)...

I am not really sure what kind of options you would want to specify at this level... most should be specified when the components are instantiated, imo.

Maybe encoding? I mean, do people actually need to change string encoding mid stream? Does that need to be at this level though?

Fishrock123 avatar Jun 06 '19 02:06 Fishrock123

Thinking about it further, you're right, options are likely not necessary here. I didn't really have any concrete suggestions in mind beyond some basic future proofing, but having it there will mean people will use it and doing so will complicate things more than strictly necessary.

jasnell avatar Jun 06 '19 15:06 jasnell

One big question I ran into yesterday while prototyping is where to put the "final bind callback" that the sink would normally have.

Stream(source, transform, sink, err => {}) // Kinda awkward

Like... I'm not sure this should return a true Promise but maybe it should be thenable? Is that too ludicrous an idea?

Fishrock123 avatar Jun 06 '19 17:06 Fishrock123

For the JavaScript side, I think returning a thenable is completely reasonable.

For the equivalent C++ API, passing in a callbacck function works just fine.

jasnell avatar Jun 06 '19 18:06 jasnell

Well, this turned out good I think: https://github.com/Fishrock123/bob/pull/31

Fishrock123 avatar Jun 06 '19 23:06 Fishrock123

Done in https://github.com/Fishrock123/bob/commit/5a460b4f1800f37f15ea78f857137bd6b276a88a although we may still want to get rid of bind altogether.

(Or possibly make it so that bindSource does not call bindSink... that would probably improve Stream()...)

Fishrock123 avatar Jun 11 '19 22:06 Fishrock123

Actually, going to keep this open.

Is class-based bind*'s useful? I.e. is there a good reason a class would do some kind of work in there?

If not... this is possible now that the exit callback is done via start():

function bob_bind(source, sink) {
  source.sink = sink
  sink.source = source
}

Edit: oh, right. That still makes implementing Stream() more complex than perhaps necessary. Will keep thinking on it.

Fishrock123 avatar Jun 11 '19 22:06 Fishrock123