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

Different versions of V8 give different string tags for generator functions: _.isFunction #1498

Closed
Xotic750 opened this issue Sep 29, 2015 · 23 comments
Labels

Comments

@Xotic750
Copy link

I am getting different results for generator function using _.isFunction with different versions of V8.

For example

function* test() {
  yield* [1, 2, 3];
}

console.log(_.isFunction(test));

On v4.5.103.33 I get a result of false, but on v4.5.103.34 I get a result of true.

If I typeof test on both versions they return function, but with Object.prototype.toString.call(test) I get [object GeneratorFunction] and [object Function] respectively. In your source code I see you use toString to avoid issues with typeof on older browsers, but this now causes problems on newer browsers. I don't know what to suggest, haven't thought about it much yet, but I thougt you should be aware of the issue.

@jdalton
Copy link
Member

jdalton commented Sep 29, 2015

Oh bummer! And that's by spec too. It's easy enough to fix.
Hmm. Actually by spec it should be "Function" so it looks like a V8 bug.

Is there a stable version of node/io.js that relies on the buggy V8 version?
FWIW on v4.5.103.33 (the version in Node 4) I get true.

@jdalton jdalton added the bug label Sep 29, 2015
@Xotic750
Copy link
Author

I'm using v4.1.0 which has V8 v4.5.103.33 which gives [object GeneratorFunction]

@jdalton jdalton added invalid and removed bug labels Sep 29, 2015
@jdalton
Copy link
Member

jdalton commented Sep 29, 2015

That's super odd.
I'm using using v4.1.0 on OSX which has V8 v4.5.103.33 and gives [object Function].

@jdalton jdalton closed this as completed Sep 29, 2015
@Xotic750
Copy link
Author

Hmm, let me double check what is going on and I'll come back to you.
Seems that I had a set of () in one test and not in the other, dope. I do apologise, it's getting late.

@jdalton
Copy link
Member

jdalton commented Sep 29, 2015

Seems that I had a set of () in one test and not in the other, dope. I do apologise, it's getting late.

I'm not sure how to get the GeneratorFunction result even with a typo can you post an example.

@Xotic750
Copy link
Author

That's a good question, my tests are actually far more complicated and I thought I had made a simple example of the problem, but I can't actually repeat it outside of my code. I'm really not sure how I am obtaining it, and too tired to think just now. Hopefully after some sleep I can take another look at it and figure out what is happening. Otherwise all I can do is push my code and point you at the repo. Quite confused just now.

@Xotic750
Copy link
Author

On node V8 v4.5.103.33 this is the output of my test, an error was thrown because a function was unexpected.

typeof function
isFunction false
toStringTag [object GeneratorFunction]
toString.call [object GeneratorFunction]
not ok 19 Basic tests Counter then times2
  Error: expected [TypeError: If not undefined, generator must be a function] to equal undefined

But on Chromium V8 v4.5.103.34, no error is throw an the output is

typeof function
(index):1814 isFunction true
(index):1815 toStringTag [object Function]
(index):1816 toString.call [object Function]

Burp

And like I said, if I extract the code to make a simple test then all works fine.

@Xotic750
Copy link
Author

Well, I'm still at a loss as to why this is happening, but I can work around the problem by changing isFunction:

      isFunction = (function (tagFunction, typeFunction) {
        return function (subject) {
          var tag = toStringTag(subject),
            result = false;

          if (isObject(subject)) {
            tag = toStringTag(subject);
            if (tag === tagFunction) {
              result = true;
            } else if (tag === '[object GeneratorFunction]' &&
              typeof subject === typeFunction) {

              result = true;
            }
          }

          return result;
        };
      }(toStringTag(isNil), typeof isNil))

Though I'm not so happy about this without knowing/understanding the reason behind why this is happening, but I have come to the end of my knowledge and experience and don't know how to investigate it further. If I find a reason for it then I will let you know and if you have any suggestions then I would be glad to hear them.

@Xotic750
Copy link
Author

I stripped down my project into a pretty bare format, other than the isFunction functions. And when I build and test it.

v4.1.0
Running "clean:all" (clean) task
>> 2 paths cleaned.

Running "buildIndexJs:readme" (buildIndexJs) task
File "scripts/index.js" created.
readme: OK

Running "jshint:build" (jshint) task
>> 6 files lint free.

Running "replace:lib" (replace) task
>> 15 replacements in 1 file.

Running "jshint:lib" (jshint) task
>> 1 file lint free.

Running "shell:beautified" (shell) task
1..4
ok 1 Basic tests typeof
not ok 2 Basic tests Lodash isFunction
  Error: expected false to be truthy
      at Assertion.assert (node_modules/expect.js/index.js:96:13)
      at Assertion.ok (node_modules/expect.js/index.js:115:10)
      at Function.ok (node_modules/expect.js/index.js:499:17)
      at Context.<anonymous> (tests/01-isFunction.js:44:28)
# tests 2
# pass 1
# fail 1
Warning: Command failed: /bin/sh -c ./node_modules/mocha/bin/mocha --harmony --check-leaks -u bdd -t 10000 -b -R tap tests/*.js
 Use --force to continue.

Aborted due to warnings.

I have created a repo at https://github.com/Xotic750/faultFunction

The content of the file being tested is

/**
 * @file {@link http://xotic750.github.io/faultFunction/ faultFunction}
 * WTF.
 * @version 0.1.0
 * @author Xotic750 <Xotic750@gmail.com>
 * @copyright  Xotic750
 * @license {@link <http://www.gnu.org/licenses/gpl-3.0.html> GPL-3.0+}
 * @module faultFunction
 */

/*jslint maxlen:80, es6:true, this:true */

/*jshint
    bitwise:true, camelcase:true, curly:true, eqeqeq:true, forin:true,
    freeze:true, futurehostile:true, latedef:true, newcap:true, nocomma:true,
    nonbsp:true, singleGroups:true, strict:true, undef:true, unused:true,
    esnext:true, plusplus:true, maxparams:3, maxdepth:4, maxstatements:37,
    maxcomplexity:8
*/

/*global
    define, module, require
*/

/**
 * UMD (Universal Module Definition)
 *
 * @private
 * @see https://github.com/umdjs/umd/blob/master/returnExports.js
 */
(function (root, factory) {
  'use strict';

  var name;

  if (typeof define === 'function' && define.amd) {
    /*
     * AMD. Register as an anonymous module.
     */
    define([], factory);
  } else if (typeof module === 'object' && module.exports) {
    /*
     * Node. Does not work with strict CommonJS, but
     * only CommonJS-like environments that support module.exports,
     * like Node.
     */
    module.exports = factory(require('lodash'));
  } else {
    /*
     * Browser globals (root is window)
     */
    if (Object.prototype.hasOwnProperty.call(root, 'faultFunction')) {
      throw new Error('Unable to define "faultFunction"');
    }

    if (Object.defineProperty) {
      Object.defineProperty(root, 'faultFunction', {
        enumerable: false,
        writable: true,
        configurable: true,
        value: factory()
      });
    } else {
      name = 'faultFunction';
      root[name] = factory();
    }
  }
}(

  /*
   * The global this object.
   */
  this,

  /**
   * Factory function
   *
   * @private
   * @return {function} The function be exported
   */
  function (_) {
    'use strict';

    return {
      loIsFunction: _.isFunction,
      isFunction: function (subject) {
        return Object.prototype.toString.call(subject);
      }
    };
  }

));

And the actual tests, which lodash fails (not suggesting it is lodash's fault).

/*jslint maxlen:80, es6:true, this:true */
/*jshint
    bitwise:true, camelcase:true, curly:true, eqeqeq:true, forin:true,
    freeze:true, futurehostile:true, latedef:true, newcap:true, nocomma:true,
    nonbsp:true, singleGroups:true, strict:true, undef:true, unused:true,
    esnext:true, plusplus:true, maxparams:1, maxdepth:2, maxstatements:5,
    maxcomplexity:1
*/
/*global require, describe, it */

(function () {
  'use strict';

  var required = require('../scripts/'),
    expect = required.expect,
    faultFunction = required.subject;

  describe('Basic tests', function () {
    function* wtf(arg) {
      yield arg;
    }

    it('typeof', function () {
      var result;

      expect(function () {
        result = typeof wtf;
      }).to.not.throwException(function (e) {
        expect(e).to.be(undefined);
      });

      expect(result).to.be('function');
    });

    it('Lodash isFunction', function () {
      var result;

      expect(function () {
        result = faultFunction.loIsFunction(wtf);
      }).to.not.throwException(function (e) {
        expect(e).to.be(undefined);
      });

      expect(result).to.be.ok();
    });

    it('toString', function () {
      var result;

      expect(function () {
        result = Object.prototype.toString.call(wtf);
      }).to.not.throwException(function (e) {
        expect(e).to.be(undefined);
      });

      expect(result).to.be('[object Function]');
    });

    it('isFunction', function () {
      var result;

      expect(function () {
        result = faultFunction.isFunction(wtf);
      }).to.not.throwException(function (e) {
        expect(e).to.be(undefined);
      });

      expect(result).to.be.ok();
    });
  });
}());

So there is something repeatable and without all of my code in the way. Build was using grunt, but npm build and npm test do the same thing.

@jdalton
Copy link
Member

jdalton commented Sep 30, 2015

What version of lodash?

@Xotic750
Copy link
Author

  "dependencies": {
    "lodash": "^3.10.1"
  },

@jdalton
Copy link
Member

jdalton commented Sep 30, 2015

And this errors in Node?

@Xotic750
Copy link
Author

Yes, v4.1.0 on Fedora 22 which has V8 v4.5.103.33

I haven't run the test in the browser since.

@jdalton
Copy link
Member

jdalton commented Sep 30, 2015

Can you keep stripping the repro down. Like even remove unneeded tests.

@Xotic750
Copy link
Author

I'll strip it down even further, to a bare skeleton.

@jdalton
Copy link
Member

jdalton commented Sep 30, 2015

@Xotic750
Copy link
Author

Hehe, well there is not much reattaining now. 4 file a .gitignore a .npmignore a package.json and the main test file faultFuction.js

package.json

{
  "name": "faultFunction",
  "homepage": "http://xotic750.github.io/faultFunction/",
  "description": "WTF.",
  "version": "0.1.0",
  "main": "faultFunction.js",
  "dependencies": {
    "lodash": "^3.10.1"
  },
  "devDependencies": {
    "mocha": "latest"
  },
  "scripts": {
    "test": "./node_modules/mocha/bin/mocha --harmony --check-leaks -u bdd -t 10000 -b -R tap faultFunction.js"
  }
}

faultFunction.js

/*global require, describe, it */
(function () {
  'use strict';

  var _ = require('lodash');

  describe('Basic tests', function () {
    function* wtf(arg) {
      yield arg;
    }

    it('Lodash isFunction', function () {
      if (!_.isFunction(wtf)) {
        throw new Error('wtf');
      }
    });
  });
}());

So, npm install and npm test

[graham@tyr faultFunction]$ npm install
npm WARN package.json faultFunction@0.1.0 No repository field.
npm WARN package.json faultFunction@0.1.0 No README data
npm WARN package.json faultFunction@0.1.0 No license field.
lodash@3.10.1 node_modules/lodash

mocha@2.3.3 node_modules/mocha
├── escape-string-regexp@1.0.2
├── commander@2.3.0
├── diff@1.4.0
├── growl@1.8.1
├── supports-color@1.2.0
├── debug@2.0.0 (ms@0.6.2)
├── mkdirp@0.5.0 (minimist@0.0.8)
├── jade@0.26.3 (commander@0.6.1, mkdirp@0.3.0)
└── glob@3.2.3 (inherits@2.0.1, graceful-fs@2.0.3, minimatch@0.2.14)
[graham@tyr faultFunction]$ npm test

> faultFunction@0.1.0 test /home/graham/source_projects/faultFunction
> ./node_modules/mocha/bin/mocha --harmony --check-leaks -u bdd -t 10000 -b -R tap faultFunction.js

1..1
not ok 1 Basic tests Lodash isFunction
  Error: wtf
      at Context.<anonymous> (faultFunction.js:14:15)
# tests 1
# pass 0
# fail 1
npm ERR! Test failed.  See above for more details.

@Xotic750
Copy link
Author

And finally, remove mocha from the equation

{
  "name": "faultFunction",
  "homepage": "http://xotic750.github.io/faultFunction/",
  "description": "WTF.",
  "version": "0.1.0",
  "main": "faultFunction.js",
  "dependencies": {
    "lodash": "^3.10.1"
  },
  "scripts": {
    "test": "node --harmony faultFunction.js"
  }
}

and

(function () {
  'use strict';

  var _ = require('lodash');

  function* wtf(arg) {
    yield arg;
  }

  if (!_.isFunction(wtf)) {
    throw new Error('wtf');
  }
}());

and then

[graham@tyr faultFunction]$ npm test

> faultFunction@0.1.0 test /home/graham/source_projects/faultFunction
> node --harmony faultFunction.js

/home/graham/source_projects/faultFunction/faultFunction.js:11
    throw new Error('wtf');
    ^

Error: wtf
    at /home/graham/source_projects/faultFunction/faultFunction.js:11:11
    at Object.<anonymous> (/home/graham/source_projects/faultFunction/faultFunction.js:13:2)
    at Module._compile (module.js:434:26)
    at Object.Module._extensions..js (module.js:452:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:475:10)
    at startup (node.js:117:18)
    at node.js:951:3
npm ERR! Test failed.  See above for more details.

Not much left to try, remove --harmony

[graham@tyr faultFunction]$ npm test

> faultFunction@0.1.0 test /home/graham/source_projects/faultFunction
> node faultFunction.js

Voila!!

There it is, not that it helps much as I want to use ES6 generators on node, and some older version require --harmony

@jdalton
Copy link
Member

jdalton commented Sep 30, 2015

Thanks! It's the --harmony flag that's the cause of the issue. Since that's the case and because it doesn't effect stable Node I'll punt on the issue for now unless user demand picks up for it or a high priority scenario depends on it.

@Xotic750
Copy link
Author

No problem, I just glad to get to the bottom of he issue. I hate being defeated. ;)

@jdalton
Copy link
Member

jdalton commented Oct 1, 2015

Turns out returning [object GeneratorFunction] from Object#toString is correct. Patched.

@Xotic750
Copy link
Author

Xotic750 commented Oct 1, 2015

Ah right, you live a little, you learn a little. ;)

@lock
Copy link

lock bot commented Jan 19, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

2 participants