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

How can I detect for GeneratorFunctions in code? #48

Closed
TooTallNate opened this issue Oct 31, 2013 · 9 comments · Fixed by #55
Closed

How can I detect for GeneratorFunctions in code? #48

TooTallNate opened this issue Oct 31, 2013 · 9 comments · Fixed by #55

Comments

@TooTallNate
Copy link

So existing code out there that I've seen checks the constructor.name property of the object to see if it's a GeneratorFunction:

function isGeneratorFunction(obj) {
  return obj && obj.constructor && 'GeneratorFunction' == obj.constructor.name;
}

Currently, regenerator-transpiled code fails this test, since the constructor property is still set to the Function object.

Can we rectify this somehow? Ideally making the above test function pass as-is, so that existing generator code out in the wild doesn't need to be tweaked.

@benjamn
Copy link
Collaborator

benjamn commented Oct 31, 2013

Unfortunately it doesn't seem possible to change the .constructor property of a function or the .name property of a constructor, so we might be out of luck here. Where have you seen this test in the wild?

@TooTallNate
Copy link
Author

Unfortunately it doesn't seem possible to change the .constructor property of a function or the .name property of a constructor, so we might be out of luck here.

While we can't update name on a function, we can luckily definitely set the constructor on a function object. To wit:

$ node
> function test () {}

// original constructor is Function
> test.constructor
[Function: Function]

// create a polyfill faux GeneratorFunction constructor
> function GeneratorFunction () {}

// set constructor to polyfill
> test.constructor = GeneratorFunction
[Function: GeneratorFunction]

// test.constructor.name is now the expected value, and will pass isGeneratorFunction()
> test.constructor.name
'GeneratorFunction'

Where have you seen this test in the wild?

The generator flow control library co: https://github.com/visionmedia/co/blob/c247a120e34c147b28119c732e550836e3477660/index.js#L201-L211

You can see above that the same technique is used to detect Generator instances, and that works great with regenerator at the moment. The fact that the isGeneratorFunction is failing is causing 4 of the co tests to fail, whereas they pass using the --harmony_generators flag in node.

@TooTallNate
Copy link
Author

That said, my proposal (and I know the extra boilerplate here kinda sucks), is to wrap the function in an immediately invoked function, so that we can have a closure around the transpiled function, and then set the constructor property before returning.

So some ES6 code like this:

co(function *() {
})();

Would turn into this (extra whitespace added for readability):

co( (function() {
  var fg = function() {
    return wrapGenerator(function($ctx) {
      while (1) switch ($ctx.next) {
      case 0:
        return $ctx.stop();
      }
    }, this);
  };

  // here's the crucial part: we overwrite the original `constructor` property with
  // one defined in the runtime, perhaps exposed as `wrapGenerator.GeneratorFunction`…
  fg.constructor = wrapGenerator.GeneratorFunction;

  return fg;
})() )();

Thoughts?

edit: fixed syntax errors in example output

@benjamn
Copy link
Collaborator

benjamn commented Nov 1, 2013

I agree that the boilerplate is less than ideal, but I also think it would be worth it.

The part I'm struggling with right now is what to do about named function declarations. Right now, their behavior is pretty simple because a generator function declaration just becomes a normal function declaration with the same name. That seems more tricky when the function is created inside a closure, because you'd have to assign the function to a variable of the same name in the outer scope before any other statements, in case those earlier statements refer to the declared function by name.

I'm going to keep thinking about it. Maybe there's a way to implement isGeneratorFunction that's easier to cooperate with, and we could persuade @visionmedia to adopt that instead.

@TooTallNate
Copy link
Author

I have come up with a hacky, regenerator-aware isGeneratorFunction(), but I can almost promise that TJ would never merge it upstream ;) Essentially it's toString()ing the argument and checking the function source for the "wrapGenerator" function.

function isGeneratorFunction(obj) {
  // the "function" typeof test is required to filter out false-positives from
  // the toString() hack below, when obj is an Array
  if ('function' != typeof obj) return false;

  // "offically" sanctioned check...
  if (obj.constructor && 'GeneratorFunction' == obj.constructor.name) return true;

  // hacky check for facebook/regenerator
  if ((String(obj).indexOf('wrapGenerator(') != -1) return true;

  return false;
}

This makes the co test cases pass 100% for me using node v0.10.21 which is using regenerator under the hood.

@benjamn
Copy link
Collaborator

benjamn commented Nov 1, 2013

What if the runtime added

Function.prototype.markGenerator = function() {
  this.constructor = GeneratorFunction;
  return this;
};

And then generator function expressions like function *gen() {} became the expression function gen() { ... }.markGenerator() and function declarations of the same form became

gen.markGenerator(); // At beginning of scope where gen is defined.
...
function gen() { ... }

That saves some boilerplate, at least (and we could use an even shorter name than markGenerator).

@TooTallNate
Copy link
Author

If that's possible then I think it would be awesome!

I'd recommend prefixing the name of the function though, since augmenting the Function.prototype could upset some people. Perhaps _regenMark() or something less likely to ever collide.

I would love to see this happen!

@Raynos
Copy link
Contributor

Raynos commented Nov 2, 2013

Please don't add stuff to Function.prototype for making generated code pretty. Just use wrapGenerator.markGenerator(gen)

// function declarations
wrapGenerator.markGenerator(gen); // At beginning of scope where gen is defined.
...
function gen() { ... }

// function expressions
wrapGenerator.markGenerator(function gen() { ... })

@TooTallNate
Copy link
Author

@Raynos +1

benjamn added a commit that referenced this issue Nov 2, 2013
This hook will likely prove useful in more ways in the future, but for now
it's primarily useful for setting the .constructor property of the
generator function to the GeneratorFunction constructor in order to
support wrapGenerator.isGeneratorFunction.

Closes #48.
benjamn added a commit that referenced this issue Nov 2, 2013
This hook will likely prove useful in more ways in the future, but for now
it's primarily useful for setting the .constructor property of the
generator function to the GeneratorFunction constructor in order to
support wrapGenerator.isGeneratorFunction.

Closes #48.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants