-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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: eslint-plugin-react-hooks: Cannot read property 'type' of undefined at analyzePropertyChain #20204
Comments
Some debugging information: react/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js Lines 1644 to 1646 in 13a62fe
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 |
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 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. 😄 |
Just FYI: I also experienced this bug and it was triggered by exactly the scenario you have mentioned. |
Summary: This sync includes the following changes: - **[c9aab1c9d](facebook/react@c9aab1c9d )**: react-refresh@0.10.0 //<Dan Abramov>// - **[516b76b9a](facebook/react@516b76b9a )**: [Fast Refresh] Support callthrough HOCs ([#21104](facebook/react#21104)) //<Dan Abramov>// - **[0853aab74](facebook/react@0853aab74 )**: Log all errors to console.error by default ([#21130](facebook/react#21130)) //<Sebastian Markbåge>// - **[d1294c9d4](facebook/react@d1294c9d4 )**: Add global onError handler ([#21129](facebook/react#21129)) //<Sebastian Markbåge>// - **[64983aab5](facebook/react@64983aab5 )**: Remove redundant setUpdatePriority call ([#21127](facebook/react#21127)) //<Andrew Clark>// - **[634cc52e6](facebook/react@634cc52e6 )**: Delete dead variable: currentEventWipLanes ([#21123](facebook/react#21123)) //<Andrew Clark>// - **[1102224bb](facebook/react@1102224bb )**: Fix: flushSync changes priority inside effect ([#21122](facebook/react#21122)) //<Andrew Clark>// - **[dbe98a5aa](facebook/react@dbe98a5aa )**: Move sync task queue to its own module ([#21109](facebook/react#21109)) //<Andrew Clark>// - **[3ba5c8737](facebook/react@3ba5c8737 )**: Remove Scheduler indirection ([#21107](facebook/react#21107)) //<Andrew Clark>// - **[46b68eaf6](facebook/react@46b68eaf6 )**: Delete LanePriority type ([#21090](facebook/react#21090)) //<Andrew Clark>// - **[dcd13045e](facebook/react@dcd13045e )**: Use Lane to track root callback priority ([#21089](facebook/react#21089)) //<Andrew Clark>// - **[5f21a9fca](facebook/react@5f21a9fca )**: Clean up host pointers in level 2 of clean-up flag ([#21112](facebook/react#21112)) //<Andrew Clark>// - **[32d6f39ed](facebook/react@32d6f39ed )**: [Fizz] Support special HTML/SVG/MathML tags to suspend ([#21113](facebook/react#21113)) //<Sebastian Markbåge>// - **[a77dd13ed](facebook/react@a77dd13ed )**: Delete enableDiscreteEventFlushingChange ([#21110](facebook/react#21110)) //<Andrew Clark>// - **[048ee4c0c](facebook/react@048ee4c0c )**: Use `act` in fuzz tester to flush expired work ([#21108](facebook/react#21108)) //<Andrew Clark>// - **[556644e23](facebook/react@556644e23 )**: Fix plurals ([#21106](facebook/react#21106)) //<Sebastian Markbåge>// - **[8b741437b](facebook/react@8b741437b )**: Rename SuspendedWork to Task ([#21105](facebook/react#21105)) //<Sebastian Markbåge>// - **[38a1aedb4](facebook/react@38a1aedb4 )**: [Fizz] Add FormatContext and Refactor Work ([#21103](facebook/react#21103)) //<Sebastian Markbåge>// - **[1b7e471b9](facebook/react@1b7e471b9 )**: React Native New Architecture: Support passing nativeViewTag to getInspectorDataForViewAtPoint callback, for React DevTools compat ([#21080](facebook/react#21080)) //<Joshua Gross>// - **[4a99c5c3a](facebook/react@4a99c5c3a )**: Use highest priority lane to detect interruptions ([#21088](facebook/react#21088)) //<Andrew Clark>// - **[77be52729](facebook/react@77be52729 )**: Remove LanePriority from computeExpirationTime ([#21087](facebook/react#21087)) //<Andrew Clark>// - **[3221e8fba](facebook/react@3221e8fba )**: Remove LanePriority from getBumpedLaneForHydration ([#21086](facebook/react#21086)) //<Andrew Clark>// - **[05ec0d764](facebook/react@05ec0d764 )**: Entangled expired lanes with SyncLane ([#21083](facebook/react#21083)) //<Andrew Clark>// - **[03ede83d2](facebook/react@03ede83d2 )**: Use EventPriority to track update priority ([#21082](facebook/react#21082)) //<Andrew Clark>// - **[a63f0953b](facebook/react@a63f0953b )**: Delete SyncBatchedLane ([#21061](facebook/react#21061)) //<Ricky>// - **[fa868d6be](facebook/react@fa868d6be )**: Make opaque EventPriority type a Lane internally ([#21065](facebook/react#21065)) //<Andrew Clark>// - **[eb58c3909](facebook/react@eb58c3909 )**: react-hooks/exhaustive-deps: Handle optional chained methods as dependency ([#20204](facebook/react#20204)) ([#20247](facebook/react#20247)) //<Ari Perkkiö>// - **[7b84dbd16](facebook/react@7b84dbd16 )**: Fail build on deep requires in npm packages ([#21063](facebook/react#21063)) //<Dan Abramov>// - **[2c9d8efc8](facebook/react@2c9d8efc8 )**: Add react-reconciler/constants entry point ([#21062](facebook/react#21062)) //<Dan Abramov>// - **[d0eaf7829](facebook/react@d0eaf7829 )**: Move priorities to separate import to break cycle ([#21060](facebook/react#21060)) //<Andrew Clark>// - **[435cff986](facebook/react@435cff986 )**: [Fizz] Expose callbacks in options for when various stages of the content is done ([#21056](facebook/react#21056)) //<Sebastian Markbåge>// - **[25bfa287f](facebook/react@25bfa287f )**: [Experiment] Add feature flag for more aggressive memory clean-up of deleted fiber trees ([#21039](facebook/react#21039)) //<Benoit Girard>// - **[8fe7810e7](facebook/react@8fe7810e7 )**: Remove already completed comment ([#21054](facebook/react#21054)) //<Sebastian Markbåge>// - **[6c3202b1e](facebook/react@6c3202b1e )**: [Fizz] Use identifierPrefix to avoid conflicts within the same response ([#21037](facebook/react#21037)) //<Sebastian Markbåge>// - **[dcdf8de7e](facebook/react@dcdf8de7e )**: Remove discrete lanes and priorities ([#21040](facebook/react#21040)) //<Andrew Clark>// - **[ca99ae97b](facebook/react@ca99ae97b )**: Replace some flushExpired callsites ([#20975](facebook/react#20975)) //<Ricky>// - **[1fafac002](facebook/react@1fafac002 )**: Use SyncLane for discrete event hydration ([#21038](facebook/react#21038)) //<Andrew Clark>// Changelog: [General][Changed] - React Native sync for revisions 6d3ecb7...c9aab1c jest_e2e[run_all_tests] Reviewed By: JoshuaGross Differential Revision: D27436763 fbshipit-source-id: da79a41e26bffdcdacd293178062edf098e9b58a
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. |
Has there been any update on it? I am also facing the similar issue. Plugin versions
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>) |
Please 👏 release 👏 the 👏 plugin 👏 already 👏 |
This problem still exists in version 4.2.0, but this problem will not occur when using version 1.7.0 |
Are there plans for a new release? This is still an issue for me, with
and |
This is now included in the |
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. |
I am still getting this issue on The offending code is here, where const samples = useMemo(() => {
const parentSVG = getParentSVG(restrictToPath);
const transform = parentSVG?.getCTM() || new DOMMatrix();
return getSamples(restrictToPath, transform);
}, [restrictToPath?.getTotalLength()]); This is running through "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
|
Please send a failing regression test case similar to https://github.com/facebook/react/pull/20247/files, either with or without a fix. |
React version:
Steps To Reproduce
The current behavior
ESlint reports error:
Cannot read property 'type' of undefined Occurred while linting <file>
.react/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
Lines 1624 to 1625 in 13a62fe
The expected behavior
ESlint rule should not crash. According to #18819 (comment) 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
Cannot read property 'type' of undefined Occurred while linting <text>:45
elastic/kibana/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rules.tsx
Log
The text was updated successfully, but these errors were encountered: