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

fix(no-forward-ref): loose fix #925

Merged
merged 3 commits into from
Jan 27, 2025
Merged

fix(no-forward-ref): loose fix #925

merged 3 commits into from
Jan 27, 2025

Conversation

hyoban
Copy link
Contributor

@hyoban hyoban commented Jan 23, 2025

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • I have added a convincing reason for adding this feature, if necessary

Other information

This can reduce the amount of code users need to modify after auto-fixing lint

Copy link

vercel bot commented Jan 23, 2025

@hyoban is attempting to deploy a commit to the Rel1cx's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

vercel bot commented Jan 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
eslint-react ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 23, 2025 3:46pm

@Rel1cx
Copy link
Owner

Rel1cx commented Jan 23, 2025

The current fix follows the remove-forward-ref in the official React codemod https://github.com/reactjs/react-codemod/blob/master/transforms/remove-forward-ref.ts.

Can you list some real-world examples where the types used in the current fix are not compatible, while the ones in this PR are?

@hyoban
Copy link
Contributor Author

hyoban commented Jan 24, 2025

We may be able to refer to React.DetailedHTMLProps, defined as ref?: Ref<T> | undefined, and Ref is RefCallback<T> | RefObject<T | null> | null.

And we may be able to use React.Ref directly. Did I miss something?

If the ref of a component is required, don't we need to pass the ref in every place?

@Rel1cx
Copy link
Owner

Rel1cx commented Jan 24, 2025

If the ref of a component is required, don't we need to pass the ref in every place?

The ref prop is passed by React, not by user. I tend to keep the existing fixes for a while longer to see if @types/react gets updated.

@Rel1cx Rel1cx closed this Jan 24, 2025
@Rel1cx Rel1cx reopened this Jan 25, 2025
@Rel1cx Rel1cx added the Status: On Hold The issue is currently on hold label Jan 26, 2025
@Rel1cx Rel1cx merged commit 28af27b into Rel1cx:main Jan 27, 2025
4 of 5 checks passed
@Rel1cx Rel1cx removed the Status: On Hold The issue is currently on hold label Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants