3.4.0 breaks babel-eslint

[keithamus]

Hey, thanks for making a great project!

Sadly, it looks like 3.4.0 broke eslint with babel-eslint:

$ npm ls escope
[email protected] /<snip>/mypackage
└── [email protected]

$ grep eslintConfig -A6 package.json
  "eslintConfig": {
    "extends": "strict",
    "parser": "babel-eslint",
    "rules": {
      "spaced-comment": 0
    }
  },

$ eslint .
 # no errors

$ npm i [email protected]

$ eslint .
TypeError: Cannot read property 'visitClass' of undefined
    at monkeypatch (/<snip>/myproject/node_modules/babel-eslint/index.js:220:40)
    at Object.exports.parse (/<snip>/myproject/node_modules/babel-eslint/index.js:362:5)
    at parse (/<snip>/myproject/node_modules/eslint/lib/eslint.js:512:27)
    at EventEmitter.module.exports.api.verify (/<snip>/myproject/node_modules/eslint/lib/eslint.js:646:19)
    at processText (/<snip>/myproject/node_modules/eslint/lib/cli-engine.js:230:27)
    at processFile (/<snip>/myproject/node_modules/eslint/lib/cli-engine.js:270:18)
    at executeOnFile (/<snip>/myproject/node_modules/eslint/lib/cli-engine.js:617:23)
    at Array.forEach (native)
    at CLIEngine.executeOnFiles (/<snip>/myproject/node_modules/eslint/lib/cli-engine.js:640:56)
    at Object.cli.execute (/<snip>/myproject/node_modules/eslint/lib/cli.js:162:95)

[keithamus]

If you'd like me to make a reduced test case, I can do so. Let me know if you'd like anything else from me too.

[michaelficarra]

Strange, there's only been one nontrivial commit since 3.3 (17f5688). @nre?

[nre]

@michaelficarra
I haven't been using babel-eslint, but I'll try and work out what's gone wrong.

[hzoo]

Just saw this now. It's the babel 6 update - Just need to update the require statements (babel/babel-eslint#244) everyone can follow babel/babel-eslint#243 so not a escope issue.

[nre]

@michaelficarra

The transpiled file lib/referencer.js looks completely different in v3.3 and v3.4. Here is visitClass in v3.3:

visitClass: {
    value: function visitClass(node) {
        if (node.type === Syntax.ClassDeclaration) {
            this.currentScope().__define(node.id, new Definition(Variable.ClassName, node.id, node, null, null, null));
        }

        // FIXME: Maybe consider TDZ.
        this.visit(node.superClass);

        this.scopeManager.__nestClassScope(node);

        if (node.id) {
            this.currentScope().__define(node.id, new Definition(Variable.ClassName, node.id, node));
        }
        this.visit(node.body);

        this.close(node);
    }
},

And this is why it can't be found in v3.4:

}, {
    key: 'visitClass',
    value: function visitClass(node) {
        if (node.type === _estraverse.Syntax.ClassDeclaration) {
            this.currentScope().__define(node.id, new _definition.Definition(_variable2.default.ClassName, node.id, node, null, null, null));
        }

        // FIXME: Maybe consider TDZ.
        this.visit(node.superClass);

        this.scopeManager.__nestClassScope(node);

        if (node.id) {
            this.currentScope().__define(node.id, new _definition.Definition(_variable2.default.ClassName, node.id, node));
        }
        this.visit(node.body);

        this.close(node);
    }
}, {

[michaelficarra]

@nre Don't bother.

Closing, as this is certainly a problem with the hacky way babel-eslint is trying to monkeypatch escope. If they're doing that, they should be responsible and lock the version down.

edit: Crossed streams with the above two comments.

[hzoo]

@michaelficarra yeah it's definitely something that needs to be updated in babel-eslint. Also we can can't lock the escope version since babel-eslint doesn't have escope as a dependency (it's using the one in eslint).

[sebmck]

@michaelficarra The monkeypatching is only necessary because ESLint and Escope lack any extension APIs.

[nzakas]

Let's be clear for anyone else that might come across this issue: the parser configuration for ESLint was intended for modules that look like Esprima and would produce an ESTree-compatible tree. What babel-eslint is doing, reaching up into ESLint's dependencies, is far beyond the scope of what was intended and is definitely a void-your-warranty situation.

@kittens I previously asked you for feedback on how to make ESLint more pluggable so we could avoid such hacks and didn't hear anything back. In the meantime, @hzoo and I have been discussing some options here: eslint/eslint#4640.

[sebmck]

@nzakas Yeah I know, I really appreciate your cooperation. I was just giving some historical context. I don't work on babel-eslint and have stepped away from Babel core mostly so I lack the time and oversight to make any useful suggestions.

[nzakas]

@kittens no worries, @hzoo and I will be continue plugging along to see if we can figure out something useful.