Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow object with methods to be passed as (writeable) sink #256

Closed
jkrems opened this issue Dec 19, 2014 · 2 comments · Fixed by #265
Closed

Allow object with methods to be passed as (writeable) sink #256

jkrems opened this issue Dec 19, 2014 · 2 comments · Fixed by #265
Assignees

Comments

@jkrems
Copy link

jkrems commented Dec 19, 2014

Sorry if something similar was already discussed, I searched in the issues and couldn't find anything.

One pattern node uses[1] is to attach internal fields (e.g. pointers to native structures) to objects. When methods are called on instances later on, their native implementations can access these internal fields. The problem is now that these constructs only work if the methods are actually called on the instances, e.g.:

var TTYSink = getNativeBinding('TTY'); // returns a class created via a FunctionTemplate
var sink = new TTYSink(1); // tty stream for stdout
sink.write("Hello World").then(() => { /* written */ }); // will directly call the native implementation of `write`

It would be great if it would be possible to pass such an object into the Writeable constructor. But unfortunately right now this will fail because Writeable will try to call write as a function instead of as a method, which forces ugly hacks like binding all methods explicitly:

var stdout = new Writeable({
  /* ... */
  close: sink.close.bind(sink),
  write: sink.write.bind(sink)
});

By preserving the this-context, the following would "just work":

var stdout = new Writeable(new TTYSink(1));

I'm not sure if this is a V8-specific concern or if there's a better way of solving this when implementing the sink (e.g. if V8 already has an option to attach user data to all methods of an object). Still wanted to bring it up for discussion. :)

[1] According to v8-users, this is a common pattern: https://groups.google.com/d/msg/v8-users/Axf4hF_RfZo/hA6Mvo78AqAJ

@domenic
Copy link
Member

domenic commented Dec 19, 2014

This seems pretty reasonable. It transitions us from an "options object" model to reifying the underlying sink as a first-class object, but as you've pointed out there are benefits. (And I don't think it's just for V8; you could imagine user code doing similar things with underscored properties or similar.)

This means we will no longer be able to do type-checking on the passed in functions (since the object might have getters that sometimes return functions and sometimes don't), and instead will have to defer any errors until later when we try to invoke the method. But, no big deal.

@domenic domenic modified the milestone: Week of 2015-01-12 Jan 8, 2015
@domenic domenic self-assigned this Jan 14, 2015
@domenic
Copy link
Member

domenic commented Jan 16, 2015

In progress in the first-class-source-sink-objects branch. A bit more work than expected because of the defaulting behavior.

domenic added a commit that referenced this issue Jan 17, 2015
Part of #256. This also removes the concept of the "default readable stream strategy" in favor of incorporating the defaults into the algorithms directly.
domenic added a commit that referenced this issue Jan 17, 2015
Closes #256. As with the previous commit for readable streams, removes the idea of a default writable stream queuing strategy in favor of just incorporating the fallbacks into the algorithms directly.
domenic added a commit that referenced this issue Jan 20, 2015
Part of #256. This also removes the concept of the "default readable stream strategy" in favor of incorporating the defaults into the algorithms directly.
domenic added a commit that referenced this issue Jan 20, 2015
Closes #256. As with the previous commit for readable streams, removes the idea of a default writable stream queuing strategy in favor of just incorporating the fallbacks into the algorithms directly.
domenic added a commit that referenced this issue Jan 21, 2015
Part of #256. This also removes the concept of the "default readable stream strategy" in favor of incorporating the defaults into the algorithms directly.
domenic added a commit that referenced this issue Jan 21, 2015
Closes #256. As with the previous commit for readable streams, removes the idea of a default writable stream queuing strategy in favor of just incorporating the fallbacks into the algorithms directly.
domenic added a commit that referenced this issue Jan 21, 2015
Part of #256. This also removes the concept of the "default readable stream strategy" in favor of incorporating the defaults into the algorithms directly.
domenic added a commit that referenced this issue Jan 21, 2015
Closes #256. As with the previous commit for readable streams, removes the idea of a default writable stream queuing strategy in favor of just incorporating the fallbacks into the algorithms directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants