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-object-type-as-default-prop is disabled when using a ref prop #3767

Closed
2 tasks done
JulienR1 opened this issue Jun 12, 2024 · 5 comments · Fixed by #3768
Closed
2 tasks done

[Bug]: react/no-object-type-as-default-prop is disabled when using a ref prop #3767

JulienR1 opened this issue Jun 12, 2024 · 5 comments · Fixed by #3768

Comments

@JulienR1
Copy link
Contributor

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

Brief description

The react/no-object-type-as-default-prop does not run when a component has a ref prop as a second argument.

Show example of your code (as text format), add images/videos/gifs to help explain example

Both of these components should error because an object type is used as a default prop.
However, the second component (with a ref) does not error.

const ErrorIsShown = ({ foo = {} }) => null;
const ErrorIsAbsent = ({ foo = {} }, ref) => null;

image

Link to repro: https://github.com/JulienR1/eslint-react-no-object-type-as-default-prop-repro

What command(s) did you run to reproduce issue?

npm run lint
# OR
eslint . --ext js,jsx --report-unused-disable-directives --max-warnings 0
// .eslintrc.cjs
module.exports = {
  root: true,
  env: { browser: true, es2020: true },
  extends: ["plugin:react/recommended"],
  ignorePatterns: ["dist", ".eslintrc.cjs"],
  parserOptions: { ecmaVersion: "latest", sourceType: "module" },
  rules: {
    "react/no-object-type-as-default-prop": "error",
    "react/prop-types": "off",
  },
};

Expected Behavior

Brief description

Components with ref props should also generate errors when using default object type props.

image

eslint-plugin-react version

v7.34.2

eslint version

v8.57.0

node version

v20.12.2

@JulienR1 JulienR1 added the bug label Jun 12, 2024
@ljharb
Copy link
Member

ljharb commented Jun 12, 2024

That's because forwardRef's callback is not a component. Function components take context as a second argument, not ref.

@JulienR1
Copy link
Contributor Author

JulienR1 commented Jun 12, 2024

Actually, the issue does not come from the ref directly, it comes from having a second argument. If I use forwardRef as described in React's docs, I still don't get an error from eslint.

From what I can see, this behavior is caused by this line in the rule's implementation: having more than one argument in the function will not execute the rule.

Second test:

const ErrorIsShown = ({ foo = {} }) => null;
const ErrorIsAbsent = forwardRef(function({ foo = {} }, ref) {
    return null
});

@ljharb
Copy link
Member

ljharb commented Jun 12, 2024

I see what you mean. Given that it's fine to have , context in a component, the rule should be triggered regardless of there being more than one param.

@JulienR1
Copy link
Contributor Author

That is exactly it.
Do you want me to set up a PR fixing this?

@ljharb
Copy link
Member

ljharb commented Jun 12, 2024

That’d be great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants