jscs-dev / node-jscs

:arrow_heading_up: JavaScript Code Style checker (unmaintained)
https://jscs-dev.github.io
MIT License
4.96k stars 513 forks source link

Issues with harmony-reflect and disallowDanglingUnderscores #2026

Closed timkelty closed 8 years ago

timkelty commented 8 years ago

A dependency I was using started using harmony-reflect, which started throwing errors when required in tandem with jscs:

var Reflect = require('harmony-reflect');
var Checker = require('jscs');
var checker = new Checker();
checker.registerDefaultRules();
checker.configure({
    disallowDanglingUnderscores: true
});
❯ node test.js
/Users/timkelty/Repos/fusionary-fed/templates/node_modules/harmony-reflect/reflect.js:1655
      return prim_setPrototypeOf(target, newProto);
             ^

TypeError: Object prototype may only be an Object or null: true
    at setPrototypeOf (native)
    at Function.Object.setPrototypeOf (/Users/timkelty/Repos/fusionary-fed/templates/node_modules/harmony-reflect/reflect.js:1655:14)
    at Object.prim_defineProperty.set (/Users/timkelty/Repos/fusionary-fed/templates/node_modules/harmony-reflect/reflect.js:1634:21)
    at Object.module.exports.configure (/Users/timkelty/Repos/fusionary-fed/templates/node_modules/jscs/lib/rules/disallow-dangling-underscores.js:88:51)
    at NodeConfiguration.<anonymous> (/Users/timkelty/Repos/fusionary-fed/templates/node_modules/jscs/lib/config/configuration.js:847:14)
    at Array.forEach (native)
    at NodeConfiguration.Configuration._useRules (/Users/timkelty/Repos/fusionary-fed/templates/node_modules/jscs/lib/config/configuration.js:845:37)
    at NodeConfiguration.Configuration.load (/Users/timkelty/Repos/fusionary-fed/templates/node_modules/jscs/lib/config/configuration.js:241:10)
    at StringChecker.configure (/Users/timkelty/Repos/fusionary-fed/templates/node_modules/jscs/lib/string-checker.js:67:29)
    at Checker.configure (/Users/timkelty/Repos/fusionary-fed/templates/node_modules/jscs/lib/checker.js:28:39)

If I set disallowDanglingUnderscores to false, the error goes away.

https://github.com/jscs-dev/node-jscs/blob/c6a87c1637b27c1c99d01b9986fd164eed186855/lib/rules/disallow-dangling-underscores.js#L72-L90

kswedberg commented 8 years ago

It looks like the disallow-dangling-underscores.js file is setting __proto__ to true, which is throwing an error because it can only be set to null or an object. Not sure why this is only happening when harmony-reflect is required. Maybe harmony-reflect is polyfilling something that node's es6 implementation doesn't have yet?

markelog commented 8 years ago

Hey Karl, fancy meeting you here :-).

So there is our culprit - https://github.com/tvcutsem/harmony-reflect/blob/e66cbac79158b6e32c65e67b53a79d094d24dced/reflect.js#L1632-L1636, i guess now it is pretty much self-explanatory.

We shouldn't use an object there to identify our exception anyway.

kswedberg commented 8 years ago

Hey Oleg!! It gives me great confidence in this project to see you as a major contributor. :) Thanks so much for the quick turnaround. Hope to see you IRL sometime in the next year.

markelog commented 8 years ago

Thanks, hope to see you too :-).

timkelty commented 8 years ago

Thanks @markelog!