Babel-eslint: 6.0.1 breaks on es module definitions

[markfinger]

TLDR: 6.0.0 works fine, but 6.0.1 breaks and complains about import decls.

[email protected]
[email protected]

Picked up by greenkeeper in https://travis-ci.org/markfinger/cyclic-dependency-graph/builds/119923238

The relevant stack trace

ImportDeclaration should appear when the mode is ES6 and in the module context.
AssertionError: ImportDeclaration should appear when the mode is ES6 and in the module context.
    at Referencer.ImportDeclaration (/home/travis/build/markfinger/cyclic-dependency-graph/node_modules/escope/lib/referencer.js:591:34)
    at Referencer.Visitor.visit (/home/travis/build/markfinger/cyclic-dependency-graph/node_modules/esrecurse/esrecurse.js:122:34)
    at Referencer.Visitor.visitChildren (/home/travis/build/markfinger/cyclic-dependency-graph/node_modules/esrecurse/esrecurse.js:101:38)
    at Referencer.Program (/home/travis/build/markfinger/cyclic-dependency-graph/node_modules/escope/lib/referencer.js:419:18)
    at Referencer.Visitor.visit (/home/travis/build/markfinger/cyclic-dependency-graph/node_modules/esrecurse/esrecurse.js:122:34)
    at Object.analyze (/home/travis/build/markfinger/cyclic-dependency-graph/node_modules/escope/lib/index.js:153:16)
    at Object.escope.analyze (/home/travis/build/markfinger/cyclic-dependency-graph/node_modules/babel-eslint/index.js:80:27)
    at EventEmitter.module.exports.api.verify (/home/travis/build/markfinger/cyclic-dependency-graph/node_modules/eslint/lib/eslint.js:815:35)
    at processText (/home/travis/build/markfinger/cyclic-dependency-graph/node_modules/eslint/lib/cli-engine.js:181:27)
    at processFile (/home/travis/build/markfinger/cyclic-dependency-graph/node_modules/eslint/lib/cli-engine.js:221:18)

Config: https://github.com/markfinger/cyclic-dependency-graph/blob/master/.eslintrc

{
  "parser": "babel-eslint",
  "extends": "eslint:recommended",
  "rules": {
    "array-bracket-spacing": 2,
    "arrow-spacing": 2,
    "block-scoped-var": 2,
    "block-spacing": 2,
    "brace-style": 2,
    "camelcase": 2,
    "comma-style": 2,
    "complexity": [2, 10],
    "computed-property-spacing": 2,
    "dot-notation": 2,
    "eqeqeq": 2,
    "guard-for-in": 2,
    "indent": [2, 2],
    "key-spacing": [2, {"beforeColon": false, "afterColon": true}],
    "no-console": 0,
    "no-const-assign": 2,
    "no-eq-null": 2,
    "no-floating-decimal": 2,
    "no-implicit-coercion": 2,
    "no-loop-func": 2,
    "no-mixed-spaces-and-tabs": 2,
    "no-multiple-empty-lines": 2,
    "no-negated-condition": 2,
    "no-redeclare": 2,
    "no-return-assign": 2,
    "no-spaced-func": 2,
    "no-ternary": 2,
    "no-unused-vars": [2, {"argsIgnorePattern": "^_"}],
    "no-whitespace-before-property": 2,
    "one-var-declaration-per-line": 2,
    "operator-linebreak": [2, "after"],
    "prefer-const": 2,
    "quotes": [2, "single"],
    "semi": [2, "always"],
    "space-before-blocks": 2,
    "space-before-function-paren": [2, "never"],
    "space-in-parens": 2,
    "spaced-comment": [2, "always"],
    "strict": 0
  },
  "env": {
    "browser": true,
    "node": true,
    "es6": true
  },
  "globals": {
    "describe": true,
    "it": true,
    "beforeEach": true,
    "after": true,
    "Promise": true
  }
}

I'm not sure which file is triggering the failure, but it'd be one of https://github.com/markfinger/cyclic-dependency-graph/tree/master/src

[markfinger]

Greenkeeper's currently reporting failures on every project that uses babel-eslint, so this would appear to be a regression.

[hzoo]

Yep, thanks. This would be from https://github.com/babel/babel-eslint/pull/282 then. cc @josh

http://eslint.org/docs/user-guide/configuring#specifying-parser-options

Can you pass sourceType: "module" to your code? - the error mode would be correct then (I was hardcoding modules before)

[josh]

Hmm, perhaps this is going to be seen as backwards incompatible for those who never specified their sourceType. I don't think its possible to know if sourceType was explicitly set from a config or using eslint's default sourceType: "script".

[hzoo]

sourceType - set to "script" (default) or "module" if your code is in ECMAScript modules.

babel-eslint was hiding this issue by assuming it would be modules. I think if you switched to the regular parser it would error as well.

I don't think its possible to know if sourceType was explicitly set from a config or using eslint's default sourceType: "script".

Um yeah will have to investigate

[markfinger]

moduleType defaulting to script is definitely the norm for the rest of the ecosystem.

Broader conventions aside though, the overwhelming majority of babel users will be using es modules so leaving it as the default doesn't seem like too bad an idea.

Either way, the easiest transition would be reverting the changes and publishing 6.0.2.

[hzoo]

Right that's probably why it was hardcoded as such. However, in eslint itself you should be using that config option - does adding it fix the issue? sourceType: "module"

[KevinGrandon]

Either way, the easiest transition would be reverting the changes and publishing 6.0.2.

We're starting to see this fail everywhere, so +1 for this solution.

[markfinger]

@hzoo yeah, adding the following fixed it

{
  // ...
  "parserOptions": {
    "sourceType": "module"
  }
}

[josh]

Either way, the easiest transition would be reverting the changes and publishing 6.0.2.

I think reverting the change is the right immediate option. It definitely shouldn't be a "patch" release change.

We should still open some issue to allow the option to be configurable. Right now its hard coded and can't be changed by any means.

[hzoo]

Merged the reverts. Ugh I messed up some of the commits - backporting is difficult.

Thanks everyone! (and @josh for the reverts)

[hzoo]

@josh I think we just need to pass the eslint config options down to the escope method and do

opts.sourceType = options.sourceType || "module";