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

"RangeError: Maximum call stack size exceeded" when file includes itself #210

Closed
AndrewRayCode opened this issue Mar 6, 2016 · 9 comments · Fixed by #212
Closed

"RangeError: Maximum call stack size exceeded" when file includes itself #210

AndrewRayCode opened this issue Mar 6, 2016 · 9 comments · Fixed by #212

Comments

@AndrewRayCode
Copy link

.eslintrc:

{
    "parserOptions": {
        "sourceType": "module",
    },
    "plugins": [
        "import"
    ],
    "rules": {
        "import/named": 2,
    }
}

import/named seems to be required for this to fail. Without that rule, eslint exits without error (as in the file passes the check)

Let's create a file that requires itself with the following code inside of it:

export { test } from './test.js'

...and name it test.js:

$ eslint --version
    v2.3.0

$ echo "export { test } from './test.js';" > test.js

$ eslint test.js
Maximum call stack size exceeded
RangeError: Maximum call stack size exceeded
    at Array.forEach (native)
    at test/node_modules/eslint-plugin-import/lib/core/getExports.js:243:26
    at test/node_modules/eslint-plugin-import/lib/core/getExports.js:258:13
    at Array.forEach (native)
    at Function.parse (test/node_modules/eslint-plugin-import/lib/core/getExports.js:187:16)
    at Function._for (test/node_modules/eslint-plugin-import/lib/core/getExports.js:139:29)
    at ExportMap.resolveReExport (test/node_modules/eslint-plugin-import/lib/core/getExports.js:74:27)
    at test/node_modules/eslint-plugin-import/lib/core/getExports.js:241:41
    at test/node_modules/eslint-plugin-import/lib/core/getExports.js:258:13
    at Array.forEach (native)

Obviously it's a mistake that the file includes itself, but this plugin should warn, not crash, in that case.

@benmosher
Copy link
Member

Ah, dang. I think this should be fixed when I fix #200, and even if not, I will definitely add as a test case. Thanks!

benmosher added a commit that referenced this issue Mar 8, 2016
benmosher added a commit that referenced this issue Mar 8, 2016
benmosher added a commit that referenced this issue Mar 8, 2016
benmosher added a commit that referenced this issue Mar 11, 2016
benmosher added a commit that referenced this issue Mar 11, 2016
@j-f1
Copy link
Contributor

j-f1 commented Sep 10, 2017

@benmosher this still happens when you do

// test.js
export * from './test.js'

@ljharb
Copy link
Member

ljharb commented Sep 11, 2017

That should be a runtime error; in other words, that code can't ever execute.

However, this eslint plugin shouldn't crash; but any rule touching imports should probably error out on it.

@j-f1
Copy link
Contributor

j-f1 commented Sep 11, 2017

Somehow it’s been working in production for 10 months in TypeScript in GitHub Desktop.

@ljharb
Copy link
Member

ljharb commented Sep 11, 2017

That seems like a Typescript bug then.

@j-f1
Copy link
Contributor

j-f1 commented Sep 11, 2017

Still, it seems like this plugin should parse invalid constructs, since the job of a linter is to warn you before you encounter a runtime (thrown or logic) error.

@ljharb
Copy link
Member

ljharb commented Sep 11, 2017

Totally agree! I'll reopen this; hopefully someone can out in a PR that fixes it.

@ljharb ljharb reopened this Sep 11, 2017
@j-f1
Copy link
Contributor

j-f1 commented Sep 11, 2017

Looks like this block

https://github.com/benmosher/eslint-plugin-import/blob/68f63f59538ceeb4f0348185683839be94d152c2/src/ExportMap.js#L386-L391

should check

if (remoteMap === m.path) return

before calling ExportMap.for().

@ljharb
Copy link
Member

ljharb commented Jan 4, 2018

#449/ #727 may have addressed this; happy to reopen if not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants