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

no-undef error on ES6 import #1978

Closed
ai opened this issue Mar 8, 2015 · 48 comments
Closed

no-undef error on ES6 import #1978

ai opened this issue Mar 8, 2015 · 48 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion blocked This change can't be completed until another issue is resolved bug ESLint is working incorrectly core Relates to ESLint's core APIs and features

Comments

@ai
Copy link

ai commented Mar 8, 2015

As I understand ESLint doesn’t understand that import create variable.

So I get a 'Warning' is not defined error on line 2 of this code:

import Warning from '../lib/warning';
var warn = new Warning('text');

I uses eslint 0.16.1 and my .eslintrc is:

{
    "ecmaFeatures": {
        "modules": true,
        "module":  true
    },
    "env": {
        "mocha": true,
        "node":  true,
        "es6":   true
    }
}

/cc @nzakas

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@AsaAyers
Copy link

AsaAyers commented Mar 9, 2015

I'm having the same issue. I've turned on all of the ecmaFeatures.

Based on having seen that #1543 wasn't picking up the rest args I was figuring this is probably an issue with escope. escope definitely supports imports

eslint's [email protected] uses [email protected] which does include es6-import

@nzakas nzakas added bug ESLint is working incorrectly core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion labels Mar 9, 2015
@nzakas
Copy link
Member

nzakas commented Mar 9, 2015

Could be an escope issue, but also could be the rule itself. Need to investigate.

@jasonkuhrt
Copy link

I'm also seeing this.

Source:

import Promise from 'bluebird'
import { partial as par, compose, merge, waitForEvent } from 'lodash'
import shortid from 'shortid'
import Log from './log'
import JsonStream from 'json-stream2'



export default function DeviceConnection(socket) {

  socket.subscribers = {}

  socket.log = Log.child(socket.profile) // <--- ERROR no un-def for `Log`
λ cat .eslintrc
---
  env:
    node: true
    mocha: true
    browser: true
    es6: true
  ecmaFeatures:
    modules: true
  rules:
    no-underscore-dangle: false
    space-in-parens: "never"
    space-before-blocks: "always"
    space-in-brackets: "always"
    no-lonely-if: true
    default-case: true
    quotes: [1, "single", "avoid-escape"]
    strict: false
    no-use-before-define: "nofunc"
    camelcase: false
    new-cap: "newIsCap"
    curly: [2, "multi-line"]
    semi: [0, "always"]

@fczuardi
Copy link
Contributor

fczuardi commented Mar 9, 2015

the link sent by @AsaAyers is a good hint, the identifiers of an import are not expected to be on global scope (scopes[0]) https://github.com/estools/escope/blob/master/test/es6-import.coffee#L39 but on a separate one instead https://github.com/estools/escope/blob/master/test/es6-import.coffee#L42

The no-undef rule uses a context.getScope https://github.com/eslint/eslint/blob/master/lib/rules/no-undef.js#L99 that returns only scopes[0] https://github.com/eslint/eslint/blob/master/lib/eslint.js#L953 and the imported variable is not there because it is stored in a different object (with scopes type module).

The eslint-babel parser project is having this same bug babel/babel-eslint#15 here is one patch example of a fork that attempts to fix this: BenoitZugmeyer/babel-eslint@f9a84aa

@xjamundx
Copy link
Contributor

xjamundx commented Mar 9, 2015

Not yet helpful, but diving a bit into the getScope method I see the following when I log all of the scope types and variables for the following code.

import $ from 'jquery';
import BaseView from 'BaseView';
import NotifcationView from 'view/notification';

Here is the logging I added to the getScope() method:

currentScopes.forEach(function(scope) {
    console.log(scope.type, scope.variables.map(function(x) { return x.name; }), "\n");
});

Here is the result:

global [ 'Array',
  'ArrayBuffer',
  'Boolean',
  // ... all of the rest of the expected globals
 ] 
function [ 'arguments' ] 
module [] 
function [ 'arguments', 'loadError' ] 
block [ 'bodyData' ] 
block [] 
block [] 
function [ 'arguments' ] 
function [ 'arguments' ] 
function [ 'arguments', 'event' ] 

So I'm not sure if we're getting what we need from escope (or I'm logging the wrong things...) I'd assume variables should exist inside of the module scope or something?

@fczuardi
Copy link
Contributor

fczuardi commented Mar 9, 2015

Here is a similar testcase to help understand the issue:

// install each tip of the branch with:
//     npm install jquery/esprima#harmony
//     npm install eslint/espree
//     npm install estools/escope
//     cd node_modules/escope && node_modules/.bin/gulp build && cd ../../

var esprima = require('esprima');
var espree = require('espree');
var escope = require('escope');

var code = 'import v from "mod"';

var esprimaConfig = {
    sourceType: 'module'
};

var espreeConfig = {
    ecmaFeatures: {
        modules: true
    }
};

var escopeConfig = {
    ecmaVersion: 6,
    sourceType: 'module'
};

var ast1 = esprima.parse(code, esprimaConfig);
var ast2 = espree.parse(code, esprimaConfig);
var ast3 = espree.parse(code, espreeConfig);

var scopeManager1 = escope.analyze(ast1, escopeConfig);
var scopeManager2 = escope.analyze(ast2, escopeConfig);
var scopeManager3 = escope.analyze(ast3, escopeConfig);

console.log('esprima harmony:', scopeManager1.scopes[1].variables);
console.log('eslint espree w/ esprima conf', scopeManager2.scopes[1].variables);
console.log('eslint espree:w/ espree conf', scopeManager3.scopes[1].variables);

Output:

esprima harmony: [ { name: 'v',
    identifiers: [ [Object] ],
    references: [],
    defs: [ [Object] ],
    tainted: false,
    stack: true,
    scope: 
     { type: 'module',
       set: {},
       taints: {},
       dynamic: false,
       block: [Object],
       through: [],
       variables: [Circular],
       references: [],
       variableScope: [Circular],
       functionExpressionScope: false,
       directCallToEvalScope: false,
       thisFound: false,
       __left: null,
       upper: [Object],
       isStrict: true,
       childScopes: [] } } ]
eslint espree w/ esprima conf []
eslint espree:w/ espree conf []

@nzakas
Copy link
Member

nzakas commented Mar 9, 2015

This is an interesting discussion, but it should be taking place in an escope issue, not here.

It looks like escope hasn't been updated to respect the latest AST structure for modules.

@fczuardi
Copy link
Contributor

fczuardi commented Mar 9, 2015

is this the estools/escope#33 or a new unfilled one?

@nzakas
Copy link
Member

nzakas commented Mar 9, 2015

Best to start a new one.

@fczuardi
Copy link
Contributor

fczuardi commented Mar 9, 2015

filed estools/escope#51

@nzakas
Copy link
Member

nzakas commented Mar 9, 2015

Thanks!

@ai
Copy link
Author

ai commented Mar 10, 2015

@fczuardi estools/escope#51 is fixed. does we need bugfix release for ESLint or i just need to update node_modules?

@fczuardi
Copy link
Contributor

I don't know if that fix also fixes this one, need to test it, can you please try updating your node_modules/eslint/node_modules/escope to the tip of their master and see if it works please?

@ai
Copy link
Author

ai commented Mar 10, 2015

@fczuardi seems like they didn’t release it yet.

Anyway there is same issue with fresh node_modules.

@fczuardi
Copy link
Contributor

you don't need to wait for a release, you can always install tip of the master of any npm package with npm install githubuser/project in the case of escope, after downloading the master branch with npm install you will also have to run a build task to build their library, but it should be as simple as a npm install followed by node_modules/.bin/gulp build inside their module directory

@ai
Copy link
Author

ai commented Mar 10, 2015

@fczuardi it is open soource project, so I can’t ask every contributor to has so difficult install process. Also I prefer to wait rather that create difficult script tasks in Travis CI.

@ai
Copy link
Author

ai commented Mar 10, 2015

@fczuardi but @keithamus said that this fix is works for him.

@ai
Copy link
Author

ai commented Mar 10, 2015

Fixed with latest escope 2.0.7 update

@ai ai closed this as completed Mar 10, 2015
@fczuardi
Copy link
Contributor

eslint still has 2.0.6 https://github.com/eslint/eslint/blob/master/package.json#L42 have you tested before closing the bug?

@ai
Copy link
Author

ai commented Mar 10, 2015

2.0.7 is in ^2.0.6 dependency, becuase of ^. And I tested it in PostCSS project.

@emmenko
Copy link

emmenko commented Mar 10, 2015

@fczuardi since it's specified with caret ^ you can install eslint and the patch version of escope gets installed (no need to release a new version).

@emmenko
Copy link

emmenko commented Mar 10, 2015

PS: I can confirm it's working now! 👍

@fczuardi
Copy link
Contributor

nice \o/

@wavded
Copy link
Contributor

wavded commented Mar 10, 2015

This resolved the undef issue, great work... unfortunately, now I'm getting this one... #1985

@ivanoats
Copy link

My work-around was to go into node_modules/eslint/ and edit package.json to require escope 2.0.7 then nom install [email protected] from there. This might have to be done in your global packages, too, for me it was /Users/ivan/.nvm/v0.10.36/lib/node_modules/eslint

@AsaAyers
Copy link

Upgrading to 2.0.7 broke other things whichis why it was not in the latest release: #2001

@zpratt
Copy link

zpratt commented Mar 12, 2015

I too am using the inline global variable whitelist as a temp workaround. Not ideal, but easy to remove once the issue is fixed.

@nzakas
Copy link
Member

nzakas commented Mar 12, 2015

@emmenko we are waiting for a fix from escope, so you'll need to workaround this for now. When there is an update, it will be posted here.

@emmenko
Copy link

emmenko commented Mar 12, 2015

@nzakas perfect, many thanks! :)

@nzakas nzakas added the blocked This change can't be completed until another issue is resolved label Mar 14, 2015
@nzakas
Copy link
Member

nzakas commented Mar 14, 2015

Going to see if I can workaround while waiting for escope.

nzakas added a commit that referenced this issue Mar 14, 2015
Fix: module import specifiers should be defined (refs #1978)
@wavded
Copy link
Contributor

wavded commented Mar 15, 2015

Maybe this is in line with this issue, but using 0.17, given this code (with es6 env and modules turned on):

import auth from 'basic-auth'
var foo = {}
var creds = auth(foo)
foo.creds = {
  username: creds.name,
  password: creds.pass
}

Gives this error:

/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/rules/no-func-assign.js:42
                if (def.name.name === name && def.type === "FunctionName") {
                            ^
TypeError: Cannot read property 'name' of undefined
    at checkIfIdentifierIsFunction (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/rules/no-func-assign.js:42:29)
    at EventEmitter.AssignmentExpression (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/rules/no-func-assign.js:73:17)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:166:7)
    at Controller.controller.traverse.enter (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/eslint.js:735:25)
    at Controller.__execute (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/node_modules/estraverse/estraverse.js:393:31)
    at Controller.traverse (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/node_modules/estraverse/estraverse.js:491:28)
    at EventEmitter.module.exports.api.verify (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/eslint.js:728:24)
    at processFile (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/cli-engine.js:193:27)
    at /Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/cli-engine.js:293:26

Another one is this code:

import auth from 'basic-auth'

Gives this error:

/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/rules/no-unused-vars.js:43
            if (definition.type === "VariableDeclarator") {
                          ^
TypeError: Cannot read property 'type' of undefined
    at isExported (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/rules/no-unused-vars.js:43:27)
    at EventEmitter.Program:exit (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/rules/no-unused-vars.js:120:70)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:166:7)
    at Controller.controller.traverse.leave (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/eslint.js:743:25)
    at Controller.__execute (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/node_modules/estraverse/estraverse.js:393:31)
    at Controller.traverse (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/node_modules/estraverse/estraverse.js:481:28)
    at EventEmitter.module.exports.api.verify (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/eslint.js:728:24)
    at processFile (/Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/cli-engine.js:193:27)
    at /Users/wavded/.nvm/versions/io.js/v1.5.1/lib/node_modules/eslint/lib/cli-engine.js:293:26

@nzakas
Copy link
Member

nzakas commented Mar 16, 2015

@wavded that's a completely separate issue, can you open a separate issue?

@wavded
Copy link
Contributor

wavded commented Mar 16, 2015

Sure thing. #2071

@ai
Copy link
Author

ai commented Mar 28, 2015

Maybe I can help? 1.8 release and still not fix this issue (but we had some fix before)

@nzakas
Copy link
Member

nzakas commented Mar 28, 2015

The "fix" from before resulted in regressions in other areas, which is why it was removed. The only real solution here is to get to the point where we can upgrade escope. To do that, the parser needs updating.

@nzakas
Copy link
Member

nzakas commented Apr 11, 2015

Fixed by #2235

@FrenchBen
Copy link

Still having the same error 'Illegal import declaration (undefined)' in atom.io:
[email protected]
[email protected]
[email protected]

import _ from 'underscore';
import Promise from 'bluebird';

@IanVS
Copy link
Member

IanVS commented Aug 3, 2015

@FrenchBen, what is your .eslintrc? You will need to specify ecmaFeatures: { modules: true } as mentioned in http://eslint.org/docs/user-guide/configuring#specifying-language-options

@FrenchBen
Copy link

@IanVS eslintrc is as follow:

root: true

ecmaFeatures:
    modules: true
    jsx: true
    arrowFunctions: true
    blockBindings: true

env:
    node: true
    es6: true
    browser: true

extends:
    "eslint:recommended"

then a bunch of rules... mostly taken from the eslintrc found in this repo

@IanVS
Copy link
Member

IanVS commented Aug 3, 2015

Do you get the same error if you run eslint directly, outside of atom?

@FrenchBen
Copy link

It deosn't seem to complain about it - Guess the plugin/package for atom is to blame :[ ?
Package is: https://atom.io/packages/linter-eslint

@IanVS
Copy link
Member

IanVS commented Aug 3, 2015

I'd suggest opening an issue there, if one does not already exist.

@FrenchBen
Copy link

will do - thanks for the quick response.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion blocked This change can't be completed until another issue is resolved bug ESLint is working incorrectly core Relates to ESLint's core APIs and features
Projects
None yet
Development

No branches or pull requests