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

[eslint-plugin-react-hooks] Crash when referencing "arguments" #16003

Closed
Jessidhia opened this issue Jun 27, 2019 · 5 comments
Closed

[eslint-plugin-react-hooks] Crash when referencing "arguments" #16003

Jessidhia opened this issue Jun 27, 2019 · 5 comments

Comments

@Jessidhia
Copy link
Contributor

Jessidhia commented Jun 27, 2019

Do you want to request a feature or report a bug?

Report a bug

What is the current behavior?

Referencing arguments from inside an arrow function (i.e. the arguments from the nearest non-arrow function) causes a crash in the eslint plugin.

TypeError: Cannot read property 'type' of undefined
Occurred while linting /.../src/react.tsx:92
    at gatherDependenciesRecursively (.../node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1032:21)
    at visitFunctionExpression (.../node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:985:7)
    at .../node_modules/eslint/lib/util/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (.../node_modules/eslint/lib/util/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (.../node_modules/eslint/lib/util/node-event-generator.js:251:26)
    at NodeEventGenerator.applySelectors (.../node_modules/eslint/lib/util/node-event-generator.js:280:22)
    at NodeEventGenerator.enterNode (.../node_modules/eslint/lib/util/node-event-generator.js:294:14)
    at CodePathAnalyzer.enterNode (.../node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:632:23)
    at .../node_modules/eslint/lib/linter.js:752:32

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

This doesn't reproduce in codesandbox as CRA hasn't been updated yet.

This snippet is enough to cause the crash:

function useMyHook(/*...*/) {
  useEffect(() => {
    arguments // crash because reference.resolved.defs is empty
  }, [])
}

This, however, does not crash:

function useMyHook(/*...*/) {
  useEffect(function() {
    arguments // ok
    return () => arguments // also ok
  }, [])
}

It is possible this depends on using the @typescript-eslint/parser parser; but I think scope analysis is run internally by eslint so it shouldn't matter.

What is the expected behavior?
No crash

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

eslint-plugin-react-hooks@1.6.1, react-hooks/exhaustive-deps rule

@quantizor
Copy link
Contributor

@Jessidhia According to this MDN article arrow functions actually don't get an arguments binding: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions

@gaearon
Copy link
Collaborator

gaearon commented Jul 2, 2019

Wanna send a fix?

@dhrubesh
Copy link

dhrubesh commented Jul 2, 2019

@gaearon Mind if I take this up?

@hristo-kanchev
Copy link
Contributor

@Jessidhia @gaearon I've opened a PR with the fix. 🚀

@hristo-kanchev
Copy link
Contributor

We can close this one.

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

No branches or pull requests

6 participants