search

facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.89k stars 47.07k forks source link

Bug: eslint-plugin-react-hooks: Cannot read property 'type' of undefined at analyzePropertyChain #20204

Closed AriPerkkio closed 3 years ago

AriPerkkio commented 4 years ago

React version:

"react": "^16.14.0"
"eslint-plugin-react-hooks": "^4.2.0",

Steps To Reproduce

  1. Lint file with contents below
// useCustomHook.js
import { useEffect } from 'react';

export function useCustomHook(someObject) {
  useEffect(() => {
    }, [
      someObject?.optionalField.method(),
    ]);
}
// .eslint.rc
module.exports = {
    root: true,
    env: {
        es6: true,
    },
    parserOptions: {
        ecmaVersion: 2020,
        sourceType: 'module',
        ecmaFeatures: {
            jsx: true,
        },
    },
    settings: {
        react: {
            version: 'detect',
        },
    },
    plugins: ['react-hooks'],
    extends: [
        'plugin:react-hooks/recommended',
    ],
};

The current behavior

ESlint reports error: Cannot read property 'type' of undefined Occurred while linting <file>. https://github.com/facebook/react/blob/13a62feab8c39bc0292eb36d636af0bb4f3a78df/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js#L1624-L1625

The expected behavior

ESlint rule should not crash. According to https://github.com/facebook/react/issues/18819#issuecomment-655151489 optional chaining should be supported.

I was testing an ESLint testing tool I've been creating and ran into this issue. I can work on a fix for this later if needed.

Here's the results and log.

Error result ## Rule: unable-to-parse-rule-id - Message: `Cannot read property 'type' of undefined Occurred while linting :45` - Path: `elastic/kibana/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rules.tsx` - [Link](https://github.com/elastic/kibana/blob/HEAD/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rules.tsx#L45) ```tsx const reFetchRules = useRef<(refreshPrePackagedRule?: boolean) => void>(noop); const [loading, setLoading] = useState(true); const [, dispatchToaster] = useStateToaster(); useEffect(() => { let isSubscribed = true; const abortCtrl = new AbortController(); async function fetchData() { try { ``` ``` TypeError: Cannot read property 'type' of undefined Occurred while linting :45 at analyzePropertyChain (//eslint-remote-tester/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:2235:12) at analyzePropertyChain (//eslint-remote-tester/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:2264:20) at //eslint-remote-tester/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1297:34 at Array.forEach () at visitFunctionWithDependencies (//eslint-remote-tester/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1277:43) at visitCallExpression (//eslint-remote-tester/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1737:11) at //eslint-remote-tester/node_modules/eslint/lib/linter/safe-emitter.js:45:58 at Array.forEach () at Object.emit (//eslint-remote-tester/node_modules/eslint/lib/linter/safe-emitter.js:45:38) at NodeEventGenerator.applySelector (//eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:254:26) ```
Log ``` Full log: [DONE] AriPerkkio/js-framework-playground 0 errors [DONE] oldboyxx/jira_clone 0 errors [WARN] Linting cities.ts took 7s at reach/reach-ui/packages/combobox/examples [WARN] Linting cities.js took 6s at reach/reach-ui/website/src/components [DONE] reach/reach-ui 0 errors [DONE] ant-design/ant-design 0 errors [DONE] StreakYC/react-smooth-collapse 0 errors [WARN] pmndrs/react-spring crashed: no-useless-constructor [DONE] pmndrs/react-spring 1 errors [DONE] AriPerkkio/scrpr 0 errors [DONE] react-bootstrap/react-bootstrap 0 errors [DONE] AriPerkkio/suspense-examples 0 errors [DONE] AriPerkkio/state-mgmt-examples 0 errors [WARN] withspectrum/spectrum crashed: no-useless-constructor [DONE] withspectrum/spectrum 1 errors [DONE] codesandbox/codesandbox-client 0 errors [WARN] Linting index.js took 40s at mui-org/material-ui/packages/material-ui-icons/src [WARN] mui-org/material-ui crashed: no-useless-constructor [DONE] mui-org/material-ui 2 errors [DONE] reactjs/reactjs.org 0 errors [DONE] zesty-io/accounts-ui 0 errors [DONE] zesty-io/design-system 0 errors [DONE] segmentio/evergreen 0 errors [DONE] segmentio/ui-box 0 errors [DONE] kentcdodds/kentcdodds.com 0 errors [DONE] kentcdodds/react-fundamentals 0 errors [DONE] kentcdodds/testing-react-apps 0 errors [DONE] kentcdodds/react-suspense 0 errors [DONE] kentcdodds/react-hooks 0 errors [DONE] artsy/force 0 errors [DONE] kentcdodds/react-performance 0 errors [DONE] kentcdodds/advanced-react-hooks 0 errors [DONE] kentcdodds/advanced-react-patterns 0 errors [DONE] kentcdodds/bookshelf 0 errors [DONE] kentcdodds/react-testing-library-examples 0 errors [DONE] kentcdodds/react-testing-library-course 0 errors [DONE] kentcdodds/learn-react 0 errors [DONE] kentcdodds/concurrent-react 0 errors [WARN] Linting material.min.js took 7s at project-bobon/bobonroastprofile/public [DONE] project-bobon/bobonroastprofile 0 errors [DONE] gothinkster/react-redux-realworld-example-app 0 errors [DONE] 1ven/do 0 errors [DONE] dockunit/platform 0 errors [DONE] afghl/dribbble-demo 0 errors [DONE] ismaelgt/english-accents-map 0 errors [DONE] DevAlien/dripr-ui 0 errors [DONE] rwieruch/favesound-mobx 0 errors [DONE] rwieruch/favesound-redux 0 errors [DONE] skidding/flatris 0 errors [DONE] feednext/feednext 0 errors [DONE] pearofducks/foodprocessor 0 errors [DONE] limichange/flex-editor 0 errors [DONE] HVF/franchise 0 errors [DONE] vercel/hyper 0 errors [DONE] getguesstimate/guesstimate-app 0 errors [DONE] stevenhauser/i-have-to-return-some-videotapes 0 errors [DONE] bebraw/invoice-frontend 0 errors [DONE] gpbl/isomorphic500 0 errors [DONE] WebbyLab/itsquiz-wall 0 errors [DONE] docker/kitematic 0 errors [DONE] KrateLabs/KrateLabs-App 0 errors [DONE] afghl/dribble-demo 0 errors [DONE] zeit/hyper 0 errors [DONE] koodilehto/invoice-frontend 0 errors [DONE] insin/lifequote 0 errors [DONE] paulhoughton/mortgage 0 errors [DONE] paulhoughton/mortgage-mobx 0 errors [DONE] browniefed/pdxlivebus 0 errors [WARN] Linting jquery.js took 9s at Khan/perseus/lib [WARN] skidding/illustrated-algorithms failed to pull [DONE] skidding/illustrated-algorithms 0 errors [WARN] Linting kas.js took 9s at Khan/perseus/lib [WARN] Linting katex.js took 8s at Khan/perseus/lib/katex [WARN] Linting less.js took 11s at Khan/perseus/lib [WARN] Linting mathquill-basic.js took 8s at Khan/perseus/lib/mathquill [WARN] Linting raphael.js took 7s at Khan/perseus/lib [DONE] guyellis/plant 0 errors [DONE] benoitvallon/react-native-nw-react-calculator 0 errors [WARN] Linting react-with-addons.js took 22s at Khan/perseus/lib [DONE] insin/react-hn 0 errors [DONE] LeoAJ/react-iTunes-search 0 errors [WARN] FormidableLabs/react-music crashed: no-useless-constructor [DONE] FormidableLabs/react-music 1 errors [DONE] echenley/react-news 0 errors [WARN] Linting vendors.min.js took 27s at lkazberova/react-photo-feed/static [DONE] lkazberova/react-photo-feed 0 errors [DONE] Khan/perseus 0 errors [DONE] pl12133/react-solitaire 0 errors [WARN] Linting bundle.js took 18s at afonsopacifer/react-pomodoro/app [DONE] afonsopacifer/react-pomodoro 0 errors [DONE] chvin/react-tetris 0 errors [DONE] web-pal/react-trello-board 0 errors [DONE] fcsonline/react-transmission 0 errors [DONE] SKempin/reactjs-tmdb-app 0 errors [DONE] fullstackreact/react-yelp-clone 0 errors [DONE] hoppula/refire-forum 0 errors [WARN] Linting bootstrap.min.js took 8s at antoinejaussoin/retro-board/retro-board-app/public/marketing/js [DONE] ritoplz/ritoplz 0 errors [DONE] andrewngu/sound-redux 0 errors [DONE] antoinejaussoin/retro-board 0 errors [DONE] FormidableLabs/spectacle 0 errors [DONE] torontojs/torontojs.com 0 errors [DONE] sprintly/sprintly-ui 0 errors [WARN] captbaritone/winamp2-js crashed: no-useless-constructor [DONE] captbaritone/winamp2-js 1 errors [DONE] Automattic/wp-calypso 0 errors [DONE] marmelab/react-admin 0 errors [DONE] reactstrap/reactstrap 0 errors [DONE] palantir/blueprint 0 errors [DONE] Semantic-Org/Semantic-UI-React 0 errors [DONE] grommet/grommet 0 errors [DONE] rebassjs/rebass 0 errors [DONE] FortAwesome/react-fontawesome 0 errors [WARN] microsoft/fluentui crashed: no-useless-constructor [DONE] chakra-ui/chakra-ui 0 errors [WARN] reakit/reakit crashed: no-useless-constructor [DONE] reakit/reakit 1 errors [DONE] rsuite/rsuite 0 errors [WARN] Linting Calendar.js took 26s at primefaces/primereact/src/components/calendar [DONE] uber/baseweb 0 errors [DONE] couds/react-bulma-components 0 errors [DONE] kulakowka/react-bulma 0 errors [DONE] dfee/rbx 0 errors [WARN] Linting index.ts took 13s at microsoft/fluentui/packages/react-icons-mdl2/src [DONE] primefaces/primereact 0 errors [DONE] fibo/trunx 0 errors [DONE] knipferrc/tails-ui 0 errors [DONE] emortlock/tailwind-react-ui 0 errors [DONE] geist-org/react 0 errors [WARN] Linting List.tsx took 8s at microsoft/fluentui/packages/react-internal/src/components/List [DONE] brillout/awesome-react-components 0 errors [WARN] Linting react-datepicker.js took 16s at elastic/eui/packages [DONE] JedWatson/react-select 0 errors [DONE] atlassian/react-beautiful-dnd 0 errors [DONE] react-dnd/react-dnd 0 errors [DONE] strml/react-grid-layout 0 errors [DONE] microsoft/fluentui 1 errors [DONE] adazzle/react-data-grid 0 errors [DONE] tannerlinsley/react-table 0 errors [WARN] elastic/eui crashed: no-useless-constructor [DONE] mzabriskie/react-draggable 0 errors [DONE] strml/react-resizable 0 errors [DONE] bokuweb/react-resizable-and-movable 0 errors [DONE] elastic/eui 1 errors [DONE] axmz/react-searchbox-awesome 0 errors [DONE] bokuweb/react-resizable-box 0 errors [DONE] bokuweb/react-sortable-pane 0 errors [DONE] aeagle/react-spaces 0 errors [DONE] Hacker0x01/react-datepicker 0 errors [WARN] Linting DayPickerRangeController_spec.jsx took 8s at airbnb/react-dates/test/components [DONE] orgsync/react-list 0 errors [DONE] airbnb/react-dates 0 errors [WARN] Linting bundle.js took 45s at intljusticemission/react-big-calendar/examples [DONE] intljusticemission/react-big-calendar 0 errors [DONE] i18next/react-i18next 0 errors [DONE] davidtheclark/react-aria-modal 0 errors [WARN] Linting test262-main.ts took 10s at yahoo/react-intl/packages/intl-listformat [WARN] Linting app.js took 18s at glortho/react-keydown/example/public/js [DONE] glortho/react-keydown 0 errors [WARN] Linting test262-main.ts took 7s at yahoo/react-intl/packages/intl-numberformat [DONE] gilbarbara/react-joyride 0 errors [DONE] greena13/react-hotkeys 0 errors [DONE] bvaughn/react-window 0 errors [WARN] text-mask/text-mask crashed: no-useless-constructor [WARN] Linting test262-main.ts took 41s at yahoo/react-intl/packages/intl-relativetimeformat [DONE] yahoo/react-intl 0 errors [DONE] bvaughn/react-virtualized 0 errors [DONE] dvtng/react-loading-skeleton 0 errors [DONE] KyleAMathews/react-spinkit 0 errors [DONE] zpao/qrcode.react 0 errors [DONE] airbnb/rheostat 0 errors [DONE] pierpo/react-archer 0 errors [WARN] Linting bundle.js took 23s at text-mask/text-mask/website/static [DONE] text-mask/text-mask 1 errors [DONE] mkosir/react-parallax-tilt 0 errors [DONE] rackt/react-autocomplete 0 errors [DONE] phuoc-ng/react-pdf-viewer 0 errors [DONE] eliseumds/react-autocomplete 0 errors [DONE] moroshko/react-autosuggest 0 errors [DONE] prometheusresearch/react-autocomplete 0 errors [DONE] gragland/instatype 0 errors [DONE] paypal/downshift 0 errors [DONE] ericgio/react-bootstrap-typeahead 0 errors [DONE] matteobruni/tsparticles 0 errors [DONE] facebook/react-art 0 errors [DONE] Flipboard/react-canvas 0 errors [DONE] pilwon/react-famous 0 errors [DONE] kmkzt/react-hooks-svgdrawing 0 errors [DONE] gorangajic/react-svg-morph 0 errors [WARN] Linting kinetic-v5.1.0.js took 14s at freiksenet/react-kinetic/vendor [DONE] freiksenet/react-kinetic 0 errors [DONE] chrvadala/react-svg-pan-zoom 0 errors [DONE] reduction-admin/react-reduction 0 errors [DONE] jeffersonRibeiro/react-shopping-cart 0 errors [DONE] clintonwoo/hackernews-react-graphql 0 errors [DONE] firefox-devtools/debugger 0 errors [DONE] gaearon/overreacted.io 0 errors [WARN] Linting admin_definition.jsx took 6s at mattermost/mattermost-webapp/components/admin_console [DONE] dnote/dnote 0 errors [WARN] elastic/kibana crashed: no-useless-constructor [DONE] mattermost/mattermost-webapp 0 errors [WARN] elastic/kibana crashed: unable-to-parse-rule-id [DONE] elastic/kibana 8 errors [DONE] Finished scan of 164 repositories ✨ Done in 3720.16s. ```
AriPerkkio commented 4 years ago

Some debugging information: https://github.com/facebook/react/blob/13a62feab8c39bc0292eb36d636af0bb4f3a78df/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js#L1644-L1646 Seems like ChainExpressions are only expected to contain MemberExpressions. It doesn't consider CallExpressions. At this point:

node.type -> "ChainExpression"
node.expression.type -> "CallExpression"
node.expression.object -> undefined
node.expression.property -> undefined

Test case to cover this:

{
    code: normalizeIndent`
    function MyComponent(props) {
        function MyComponent(props) {
            useEffect(() => {}, [props?.attribute.method()]);
        }
    }
    `,
}

I'm not 100% sure but this use case looks valid to me. https://github.com/elastic/kibana/blob/f49ee068f4f1a9ed126ae2abf6b3108dde594f36/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rules.tsx#L100 If it's not, at least the unexpected crashing should be prevented.

AriPerkkio commented 4 years ago

After going through the current test cases I think this is the expected behavior:

useEffect(() => {}, [someObject?.optionalField.method()]);
// Expected: React Hook useEffect has a complex expression in the dependency array. Extract it to a separate variable so it can be statically checked.
// Actual: Cannot read property 'type' of undefined -> No lint errors are displayed

This is also what is reported for [someObject.method()].

If this doesn't look right, please let me know. I'll start to work on a fix at the end of the week. So far debugging this has been fun. 😄

nightvisi0n commented 3 years ago

If this doesn't look right, please let me know.

Just FYI: I also experienced this bug and it was triggered by exactly the scenario you have mentioned.

just-boris commented 3 years ago

The latest stable version 4.2.0 still contains this bug. Are there any plans on making a new release?

I can confirm, it was fixed in nightly builds.

samundra commented 3 years ago

Has there been any update on it? I am also facing the similar issue.

Plugin versions

eslint-plugin-react: ^7.21.5
eslint-plugin-react-hooks: ^4.2.0

Reproduction steps:

Stack trace taken from Eslint Output from vscode

[Error - 12:10:18 AM] ESLint stack trace:
[Error - 12:10:18 AM] TypeError: Cannot read property 'type' of undefined
Occurred while linting <file>:<line-number>
    at visitCallExpression ([removed]/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1734:24)
    at [removed]/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit ([removed]/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector ([removed]/node_modules/eslint/lib/linter/node-event-generator.js:256:26)
    at NodeEventGenerator.applySelectors ([removed]/node_modules/eslint/lib/linter/node-event-generator.js:285:22)
    at NodeEventGenerator.enterNode ([removed]/node_modules/eslint/lib/linter/node-event-generator.js:299:14)
    at CodePathAnalyzer.enterNode ([removed]/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:711:23)
    at [removed]/node_modules/eslint/lib/linter/linter.js:954:32
    at Array.forEach (<anonymous>)
just-boris commented 3 years ago

Please 👏 release 👏 the 👏 plugin 👏 already 👏

@gaearon

abstain23 commented 3 years ago

This problem still exists in version 4.2.0, but this problem will not occur when using version 1.7.0

unstable-compound commented 3 years ago

Are there plans for a new release? This is still an issue for me, with 

  "eslint-plugin-react": "7.26.1",
  "eslint-plugin-react-hooks": "4.2.0"
  "eslint": "7.32.0",

and "react": "17.0.2",

AriPerkkio commented 3 years ago

This is now included in the eslint-plugin-react-hooks@4.3.0 release.

gaearon commented 3 years ago

Sorry folks, I have thousands of GH notifications so I didn’t see your comments. Please ping me on Twitter next time if you try to reach me, at least DMs there aren’t as flooded.

valtism commented 3 years ago

I am still getting this issue on 4.3.0

The offending code is here, where restrictToPath is SVGGeometryElement | null | undefined

const samples = useMemo(() => {
    const parentSVG = getParentSVG(restrictToPath);
    const transform = parentSVG?.getCTM() || new DOMMatrix();
    return getSamples(restrictToPath, transform);
  }, [restrictToPath?.getTotalLength()]);

This is running through vite with the following dev dependencies:

"devDependencies": {
    "@nabla/vite-plugin-eslint": "^1.3.2",
    "@types/d3-format": "^3.0.1",
    "@types/react": "^17.0.27",
    "@types/react-dom": "^17.0.9",
    "@typescript-eslint/eslint-plugin": "^4.33.0",
    "@typescript-eslint/parser": "^4.33.0",
    "@vitejs/plugin-react": "^1.0.2",
    "autoprefixer": "^10.3.7",
    "eslint": "^7.32.0",
    "eslint-plugin-react": "^7.27.1",
    "eslint-plugin-react-hooks": "^4.3.0",
    "postcss": "^8.3.9",
    "tailwindcss": "^3.0.0-alpha.1",
    "typescript": "^4.4.4",
    "vite": "^2.6.12"
  }

I deleted my .eslintcache as well as restarting the dev server, but no luck. I get the following error:

[Info - 9:58:06 AM] Cannot read property 'type' of undefined Occurred while linting .../src/useDrag.ts:210

gaearon commented 3 years ago

Please send a failing regression test case similar to https://github.com/facebook/react/pull/20247/files, either with or without a fix.