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

Bug: Cannot read property 'references' of undefined in eslint-plugin-react-hooks v4.0.5 #19243

Closed
lencioni opened this issue Jul 2, 2020 · 3 comments · Fixed by #19260
Closed

Comments

@lencioni
Copy link
Contributor

lencioni commented Jul 2, 2020

Certain code patterns using optional chaining syntax causes eslint-plugin-react-hooks to throw an error.

React version: 16.10.2

Steps To Reproduce

  1. Install eslint-plugin-react-hooks v4.0.5
  2. Put this code in a file:
import React, { useEffect } from 'react';

export const Repro = (props) => {
  const foo = {};

  const bar = () => ({
    pizza: foo.pizza,
    pasta: foo?.pasta,
  });

  useEffect(bar, []);

  return <div />;
};

The current behavior

ESLint throws the following error:

TypeError: Cannot read property 'references' of undefined
Occurred while linting /path/to/repo/file.ts:102
    at /path/to/repo/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1681:23
    at Set.forEach (<anonymous>)
    at visitFunctionWithDependencies (/path/to/repo/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1672:29)
    at visitCallExpression (/path/to/repo/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:886:19)
    at /path/to/repo/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/path/to/repo/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/path/to/repo/node_modules/eslint/lib/linter/node-event-generator.js:254:26)
    at NodeEventGenerator.applySelectors (/path/to/repo/node_modules/eslint/lib/linter/node-event-generator.js:283:22)
    at NodeEventGenerator.enterNode (/path/to/repo/node_modules/eslint/lib/linter/node-event-generator.js:297:14)

The expected behavior

ESLint does not throw an error.

This looks related to #19043 and #19062.

I've noticed that it does not throw if a number of slight variations are made to the code. The following do not throw:

import React, { useEffect } from 'react';

export const Repro = (props) => {
  const foo = {};

  const bar = {
    pizza: foo.pizza,
    pasta: foo?.pasta,
  };

  useEffect(bar, []);

  return <div />;
};
import React, { useEffect } from 'react';

export const Repro = (props) => {
  const foo = {};

  const bar = () => ({
    pizza: foo?.pizza,
    pasta: foo?.pasta,
  });

  useEffect(bar, []);

  return <div />;
};
import React, { useEffect } from 'react';

export const Repro = (props) => {
  const foo = {};

  const bar = () => ({
    pizza: something.pizza,
    pasta: foo?.pasta,
  });

  useEffect(bar, []);

  return <div />;
};

cc @krailler

@lencioni lencioni added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jul 2, 2020
@gaearon
Copy link
Collaborator

gaearon commented Jul 2, 2020

Thanks for the report. Would you like to look into a fix?

@gaearon gaearon added Component: ESLint Rules Type: Bug and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Jul 2, 2020
lencioni added a commit to lencioni/react that referenced this issue Jul 6, 2020
Certain code patterns using optional chaining syntax causes
eslint-plugin-react-hooks to throw an error.

We can avoid the throw by adding some guards. I didn't read through the
code to understand how it works, I just added a guard to every place
where it threw, so maybe there is a better fix closer to the root cause
than what I have here.

In my test case, I noticed that the optional chaining that was used in
the code was not included in the suggestions description or output,
but it seems like it should be. This might make a nice future
improvement on top of this fix, so I left a TODO comment to that effect.

Fixes facebook#19243
lencioni added a commit to lencioni/react that referenced this issue Jul 6, 2020
Certain code patterns using optional chaining syntax causes
eslint-plugin-react-hooks to throw an error.

We can avoid the throw by adding some guards. I didn't read through the
code to understand how it works, I just added a guard to every place
where it threw, so maybe there is a better fix closer to the root cause
than what I have here.

In my test case, I noticed that the optional chaining that was used in
the code was not included in the suggestions description or output,
but it seems like it should be. This might make a nice future
improvement on top of this fix, so I left a TODO comment to that effect.

Fixes facebook#19243
@lencioni
Copy link
Contributor Author

lencioni commented Jul 6, 2020

@gaearon I put up a potential fix here: #19260

bvaughn pushed a commit that referenced this issue Jul 6, 2020
Certain code patterns using optional chaining syntax causes
eslint-plugin-react-hooks to throw an error.

We can avoid the throw by adding some guards. I didn't read through the
code to understand how it works, I just added a guard to every place
where it threw, so maybe there is a better fix closer to the root cause
than what I have here.

In my test case, I noticed that the optional chaining that was used in
the code was not included in the suggestions description or output,
but it seems like it should be. This might make a nice future
improvement on top of this fix, so I left a TODO comment to that effect.

Fixes #19243
@gaearon
Copy link
Collaborator

gaearon commented Jul 7, 2020

Optional chaining problems should be fixed in [email protected]. If you experience some problem with optional chaining after 4.0.6, please file a new issue. Thanks.

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

Successfully merging a pull request may close this issue.

2 participants