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

Stream Transform with Arrow Functions #2740

Closed
hughrawlinson opened this issue Sep 8, 2015 · 6 comments
Closed

Stream Transform with Arrow Functions #2740

hughrawlinson opened this issue Sep 8, 2015 · 6 comments
Labels
question Issues that look for answers.

Comments

@hughrawlinson
Copy link

Because this in arrow functions is lexically bound, they cannot be used in providing a transform function to a stream.Transform object, in which this.push is used to return data. See the following example:

transform.prototype._transform = (data, encoding, callback)=>{
  this.push(data); // this.push is undefined because this is
                   //lexically bound and refers to the enclosing scope
  callback();
}

The callback function already accepts the output of the transform as its second argument. The following works:

transform.prototype._transform = function (data, encoding, callback) {
  callback(null,data);
}

In light of the fact that moving forward we're going to see a lot more people using the Arrow Function syntax, perhaps we should consider deprecating this.push(data), or at least changing the docs so that the callback(null,data); pattern is recommended moving forward?

I'd be very happy to submit a PR to the docs if there's any consensus on the right solution for this.

@hughrawlinson
Copy link
Author

An issue I hadn't considered: callback(null,data) should only be called once. Perhaps this.push should be passed into the _transform function as another callback for use "0 or more times"?

@Fishrock123 Fishrock123 added the question Issues that look for answers. label Sep 8, 2015
@Fishrock123
Copy link
Member

Please see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions

Arrow Functions are not a replacement for regular functions. This is expected behaviour of the language feature, in this context it will bind to the wrong scope.

You can't propagate this state to Arrow Functions, so they are unsuitable for these parts of our API. Again, this is expected, and you will need to use "regular" Functions. :)

@hughrawlinson
Copy link
Author

@Fishrock123 I understand that it's expected behaviour, I explained that the issue is a result of the expected behaviour in the first line of my issue. My point was that now that ES6 usage is going to increase by a lot, we should perhaps consider finding a way to make this part of the API easier with both styles of functions without breaking existing codebases?

@Fishrock123
Copy link
Member

My point was that now that ES6 usage is going to increase by a lot, we should perhaps consider finding a way to make this part of the API easier with both styles of functions without breaking existing codebases?

Why would we want to? Arrow Functions are not a replacement for regular Functions..

@Trott
Copy link
Member

Trott commented Sep 8, 2015

or at least changing the docs so that the callback(null,data); pattern is recommended moving forward?

A documentation PR indicating that arrow functions should not be used in this type of situation might be in order. Personally, I'd wait to see if people actually run into this problem frequently or not. But if someone felt strongly and wanted to be proactive about it, that's totally cool too.

@syntacticsolutions

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

4 participants