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]: react/no-danger β€” TypeError: Cannot read properties of undefined (reading 'split') #3833

Closed
2 tasks done
sylvainDNS opened this issue Sep 27, 2024 · 15 comments
Closed
2 tasks done
Labels

Comments

@sylvainDNS
Copy link

Is there an existing issue for this?

  • I have searched the existing issues and my issue is unique
  • My issue appears in the command-line and not only in the text editor

Description Overview

Hi team, thanks for your work on this lib πŸ™

Using react/no-danger rule with option ['error', { customComponentNames: ['*'] }] cause a command-line error.

Documentation about it: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/no-danger.md

Here is how I use the rule in my eslint configuration:

    'react/no-danger': ['error', { customComponentNames: ['*'] }],

When I run eslint ./src on my project, an error occurred (only using customComponentNames option):

❯ eslint ./src

Oops! Something went wrong! :(

ESLint: 8.57.0

TypeError: Cannot read properties of undefined (reading 'split')
Occurred while linting /Users/sylvain/react-app/src/App.tsx:74
Rule: "react/no-danger"
    at Minimatch.match (/Users/sylvain/react-app/node_modules/.pnpm/[email protected]/node_modules/minimatch/minimatch.js:743:9)
    at minimatch (/Users/sylvain/react-app/node_modules/.pnpm/[email protected]/node_modules/minimatch/minimatch.js:125:42)
    at /Users/sylvain/react-app/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint-plugin-react/lib/rules/no-danger.js:82:83
    at Array.some (<anonymous>)
    at JSXAttribute (/Users/sylvain/react-app/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint-plugin-react/lib/rules/no-danger.js:82:68)
    at ruleErrorHandler (/Users/sylvain/react-app/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/linter.js:1076:28)
    at /Users/sylvain/react-app/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/sylvain/react-app/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/Users/sylvain/react-app/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
/Users/sylvain/react-app:
Exit status 2

This error is triggered by this line

const enableCheckingCustomComponent = customComponentNames.some((name) => minimatch(functionName, name));

Actually I don't know why, but sometimes functionName is undefined

const functionName = node.parent.name.name;

Expected Behavior

Locally I patched eslint-plugin-react like this:

diff --git a/lib/rules/no-danger.js b/lib/rules/no-danger.js
index fb702632082068b08954ebb849d0994da6fac5b6..be0f82185cd9bc8358f29f5a19104ad3343996d7 100644
--- a/lib/rules/no-danger.js
+++ b/lib/rules/no-danger.js
@@ -77,10 +77,9 @@ module.exports = {
 
     return {
       JSXAttribute(node) {
-        const functionName = node.parent.name.name;
-
-        const enableCheckingCustomComponent = customComponentNames.some((name) => minimatch(functionName, name));
+        const functionName = node.parent.name.name ?? '';
 
+        const enableCheckingCustomComponent =  customComponentNames.some((name) => name === '*' || minimatch(functionName, name));
         if ((enableCheckingCustomComponent || jsxUtil.isDOMComponent(node.parent)) && isDangerous(node.name.name)) {
           report(context, messages.dangerousProp, 'dangerousProp', {
             node,

The first change is made to prevent functionName from being undefined (although this doesn't explain why).

The second change is made to skip the check on component names if we defined the rule on '*'.

I can make a PR if you want.

eslint-plugin-react version

v7.37.0

eslint version

v8.57.0

node version

v20.16.0

@sylvainDNS sylvainDNS added the bug label Sep 27, 2024
@ljharb
Copy link
Member

ljharb commented Sep 27, 2024

Can you provide the code that this is crashing on?

@halouvi
Copy link

halouvi commented Oct 21, 2024

I have the same issue and it seems that there are two conditions that need to be present for this error to be thrown:

  1. The component is rendered using dot notation.
  2. The component receives props.
import React from 'react';

const Component = () => <></>;

const NestedComponent = () => <></>;

Component.NestedComponent = NestedComponent;

export const RenderTest = () => <Component.NestedComponent key=""/>;

@ljharb
Copy link
Member

ljharb commented Oct 21, 2024

@halouvi that test case passes for me. What eslint config are you using?

@halouvi
Copy link

halouvi commented Oct 22, 2024

@ljharb these are my eslint package versions:

"eslint": "8.27.0",
"@typescript-eslint/eslint-plugin": "5.30.5",
"@typescript-eslint/parser": "5.30.5",
"eslint-config-airbnb-typescript": "16.2.0",
"eslint-config-prettier": "8.5.0",
"eslint-import-resolver-typescript": "2.7.1",
"eslint-plugin-filenames": "1.3.2",
"eslint-plugin-import": "2.26.0",
"eslint-plugin-jsx-a11y": "6.6.1",
"eslint-plugin-prettier": "4.2.1",
"eslint-plugin-react": "7.37.1",
"eslint-plugin-react-hooks": "4.6.0",
"eslint-plugin-styled-components-a11y": "0.1.0",

And this is my eslintrc.json:

{
    "root": true,
    "env": {
        "browser": true,
        "es6": true
    },
    "extends": [
        "airbnb-typescript",
        "prettier",
        "plugin:styled-components-a11y/recommended"
    ],
    "parserOptions": {
        "ecmaFeatures": {
            "jsx": true
        },
        "ecmaVersion": 2018,
        "sourceType": "module",
        "project": "./tsconfig.json",
        "extraFileExtensions": [".html"]
    },
    "parser": "@typescript-eslint/parser",
    "plugins": [
        "react",        
        "react-hooks",
        "jsx-a11y",
        "filenames",
        "import",
        "@typescript-eslint",
        "prettier"
    ],
    "rules": {
        "react/require-default-props": [0],
        "import/prefer-default-export": "off",
        "no-debugger": [1],
        "no-console": [1],
        "no-return-await": "off",
        "no-plusplus": "off",
        "no-prototype-builtins": "off",
        "no-underscore-dangle": "off",
        "no-use-before-define": "off",
        "arrow-body-style": "off",
        "@typescript-eslint/no-use-before-define": "off",
        "camelcase": "off",
        "react/button-has-type": [0],
        "react/no-unescaped-entities": [0],
        "no-param-reassign": [
            "error",
            {
                "props": false
            }
        ],
        "@typescript-eslint/indent": "off",
        "@typescript-eslint/camelcase": "off",
        "prefer-object-spread": "off",
        "react/no-danger": [
            "error",
            { "customComponentNames": ["*"] }
        ],
        "react/jsx-props-no-spreading": "off",
        "react/prop-types": "off",
        "react/jsx-fragments": "off",
        "react-hooks/rules-of-hooks": "error",
        "react-hooks/exhaustive-deps": "error",
        "lines-between-class-members": "off",
        "class-methods-use-this": "off",
        "filenames/match-exported": 2,
        "import/no-extraneous-dependencies": [
            "error",
            {
                "devDependencies": [
                    "**/*.stories.tsx",
                    "**/*.test.tsx",
                    "**/*.test.ts",
                    "./setupTests.ts"
                ]
            }
        ],
        "import/no-cycle": "off",
        "jsx-a11y/click-events-have-key-events": "warn",
        "jsx-a11y/interactive-supports-focus": "warn",
        "prettier/prettier": "error",
        "react/destructuring-assignment": "off",
        "jsx-a11y/label-has-associated-control": "off",
        "jsx-a11y/label-has-for": "warn",
        "jsx-a11y/mouse-events-have-key-events": "warn",
        "jsx-a11y/no-static-element-interactions": "warn",
        "jsx-a11y/alt-text": "warn",
        "styled-components-a11y/accessible-emoji": "warn",
        "styled-components-a11y/anchor-has-content": "warn",
        "styled-components-a11y/anchor-is-valid": "warn",
        "styled-components-a11y/aria-activedescendant-has-tabindex": "warn",
        "styled-components-a11y/aria-props": "warn",
        "styled-components-a11y/aria-proptypes": "warn",
        "styled-components-a11y/aria-role": "warn",
        "styled-components-a11y/aria-unsupported-elements": "warn",
        "styled-components-a11y/autocomplete-valid": "warn",
        "styled-components-a11y/click-events-have-key-events": "warn",
        "styled-components-a11y/control-has-associated-label": "warn",
        "styled-components-a11y/heading-has-content": "warn",
        "styled-components-a11y/img-redundant-alt": "warn",
        "styled-components-a11y/interactive-supports-focus": "warn",
        "styled-components-a11y/label-has-associated-control": ["warn"],
        "styled-components-a11y/label-has-for": "off",
        "styled-components-a11y/media-has-caption": "warn",
        "styled-components-a11y/mouse-events-have-key-events": "warn",
        "styled-components-a11y/no-access-key": "warn",
        "styled-components-a11y/no-autofocus": "warn",
        "styled-components-a11y/no-distracting-elements": "warn",
        "styled-components-a11y/no-interactive-element-to-noninteractive-role": "warn",
        "styled-components-a11y/no-noninteractive-element-interactions": "warn",
        "styled-components-a11y/no-noninteractive-element-to-interactive-role": "warn",
        "styled-components-a11y/no-noninteractive-tabindex": "warn",
        "styled-components-a11y/no-onchange": "warn",
        "styled-components-a11y/no-redundant-roles": "warn",
        "styled-components-a11y/no-static-element-interactions": "warn",
        "styled-components-a11y/role-has-required-aria-props": "warn",
        "styled-components-a11y/role-supports-aria-props": "warn",
        "styled-components-a11y/alt-text": "warn",
        "styled-components-a11y/scope": "warn",
        "styled-components-a11y/tabindex-no-positive": "warn",
        "styled-components-a11y/html-has-lang": "off",
        "styled-components-a11y/iframe-has-title": "off",
        "@typescript-eslint/lines-between-class-members": "off",
        "@typescript-eslint/naming-convention": "off",
        "@typescript-eslint/keyword-spacing": "off",
        "@typescript-eslint/return-await": "warn",
        "curly": ["error", "all"],
        "react/jsx-key": ["error", { "checkFragmentShorthand": true }],
        "react/jsx-indent": "off",
        "react/jsx-indent-props": "off",
        "react/jsx-no-bind": "off"
    },
    "settings": {
        "import/parsers": {
            "@typescript-eslint/parser": [".ts", ".tsx"]
        }
    }
}

@sylvainDNS
Copy link
Author

sylvainDNS commented Oct 22, 2024

Thank you @halouvi to find how to trigger the issue.

Here is a sandbox that reproduce it. Uncomment <Component.NestedComponent /> and you will see eslint crash.

https://codesandbox.io/p/devbox/react-no-danger-reading-split-error-9c2fy7?file=%2Fsrc%2FApp.tsx%3A5%2C1

@ljharb
Copy link
Member

ljharb commented Oct 23, 2024

Thanks, that helped!

@ljharb ljharb closed this as completed in 1c3621a Oct 23, 2024
@halouvi
Copy link

halouvi commented Nov 3, 2024

@ljharb Thanks for the fix πŸ™ Do you know when the next version containing this fix is scheduled to be released?

@ljharb
Copy link
Member

ljharb commented Nov 3, 2024

There's no release schedule, but probably within the next week or two.

@halouvi
Copy link

halouvi commented Nov 19, 2024

@ljharb I don't mean to bother but do you think there's any chance of releasing a version this week?

@chribjel
Copy link

chribjel commented Dec 3, 2024

I also encountered this problem now. If the release cannot be published yet, would you consider making a "canary"/"next" release, that is just the newest from the master branch? @ljharb

@ljharb
Copy link
Member

ljharb commented Dec 3, 2024

@chribjel no, that would be identical to just publishing the default branch.

@halouvi
Copy link

halouvi commented Dec 23, 2024

@ljharb respectfully, is there a schedule for a version release? it's been two months now.

@ljharb
Copy link
Member

ljharb commented Dec 23, 2024

There's no release schedule

@ljharb
Copy link
Member

ljharb commented Dec 24, 2024

https://github.com/jsx-eslint/eslint-plugin-react/releases/tag/v7.37.3 has been released.

@halouvi
Copy link

halouvi commented Dec 24, 2024

@ljharb Appreciate it! πŸ™

cswimr pushed a commit to cswimr/gauntlet-cswimr-plugins that referenced this issue Dec 27, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint-plugin-react](https://github.com/jsx-eslint/eslint-plugin-react) | devDependencies | patch | [`7.37.2` -> `7.37.3`](https://renovatebot.com/diffs/npm/eslint-plugin-react/7.37.2/7.37.3) |

---

### Release Notes

<details>
<summary>jsx-eslint/eslint-plugin-react (eslint-plugin-react)</summary>

### [`v7.37.3`](https://github.com/jsx-eslint/eslint-plugin-react/blob/HEAD/CHANGELOG.md#7373---20241223)

[Compare Source](jsx-eslint/eslint-plugin-react@v7.37.2...v7.37.3)

##### Fixed

-   \[`no-danger`]: avoid a crash on a nested component name ([#&#8203;3833][] [@&#8203;ljharb](https://github.com/ljharb))
-   \[Fix] types: correct generated type declaration ([#&#8203;3840][] [@&#8203;ocavue](https://github.com/ocavue))
-   \[`no-unknown-property`]: support `precedence` prop in react 19 ([#&#8203;3829][] [@&#8203;acusti](https://github.com/acusti))
-   \[`prop-types`]: props missing in validation when using generic types from a namespace import ([#&#8203;3859][] [@&#8203;rbondoc96](https://github.com/rbondoc96))

##### Changed

-   \[Tests] \[`jsx-no-script-url`]: Improve tests ([#&#8203;3849][] [@&#8203;radu2147](https://github.com/radu2147))
-   \[Docs] fix broken links: \[`default-props-match-prop-types`], \[`jsx-boolean-value`], \[`jsx-curly-brace-presence`], \[`jsx-no-bind`], \[`no-array-index-key`], \[`no-is-mounted`], \[`no-render-return-value`], \[`require-default-props`] ([#&#8203;3841][] [@&#8203;bastiendmt](https://github.com/bastiendmt))

[7.37.3]: jsx-eslint/eslint-plugin-react@v7.37.2...v7.37.3

[#&#8203;3859]: jsx-eslint/eslint-plugin-react#3859

[#&#8203;3849]: jsx-eslint/eslint-plugin-react#3849

[#&#8203;3841]: jsx-eslint/eslint-plugin-react#3841

[#&#8203;3840]: jsx-eslint/eslint-plugin-react#3840

[#&#8203;3833]: jsx-eslint/eslint-plugin-react#3833

[#&#8203;3829]: jsx-eslint/eslint-plugin-react#3829

</details>

---

### Configuration

πŸ“… **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

β™» **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

πŸ”• **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS44My40IiwidXBkYXRlZEluVmVyIjoiMzkuODMuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Reviewed-on: https://www.coastalcommits.com/cswimr/gauntlet-cswimr-plugins/pulls/9
Co-authored-by: Renovate <[email protected]>
Co-committed-by: Renovate <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants