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: nearly identical props interface with the same name is added #2019

Open
pjonsson opened this issue Nov 30, 2024 · 4 comments · May be fixed by #2057
Open

Bug: nearly identical props interface with the same name is added #2019

pjonsson opened this issue Nov 30, 2024 · 4 comments · May be fixed by #2057
Labels
area: fixers Around how TypeStat fixes code. area: react Related to JSX syntax, particularly for React components and the like. status: accepting prs Please, send a pull request to resolve this! 🙏 type: bug Something isn't working :( 🐛

Comments

@pjonsson
Copy link
Contributor

🐛 Bug Report

  • TypeStat version: 0.8.1
  • TypeScript version: 5.2
  • Node version: 20

Actual Behavior

diff --git a/lib/Styled/Checkbox/Elements/CheckboxIcon.tsx b/lib/Styled/Checkbox/Elements/CheckboxIcon.tsx
index 8443eb5cc..880d23934 100644
--- a/lib/Styled/Checkbox/Elements/CheckboxIcon.tsx
+++ b/lib/Styled/Checkbox/Elements/CheckboxIcon.tsx
@@ -19,8 +19,14 @@ const StyledCheckboxIcon = styled(StyledIcon).attrs({
   `}
 `;

+interface CheckboxIconProps {
+isIndeterminate: boolean;
+isChecked: boolean;
+isDisabled: boolean;
+}
+
 const CheckboxIcon: React.FC<CheckboxIconProps> = (
-  props: CheckboxIconProps
+  props: CheckboxIconProps: CheckboxIconProps
 ) => {
   if (props.isDisabled) {
     return (

Expected Behavior

The inferred interface is nearly identical to the one that already exists (missing ? on the members), and even has the same name as the existing interface.

Could there be some "equivalence" between the inferred isChecked and the declared isChecked? missing?

This also shows another example of #2018: the previous type annotation is not replaced in the function signature.

Reproduction

Same as #2014.

@JoshuaKGoldberg JoshuaKGoldberg changed the title Bug: nearly identical interface with the same name is added Bug: nearly identical props interface with the same name is added Nov 30, 2024
@JoshuaKGoldberg JoshuaKGoldberg added type: bug Something isn't working :( 🐛 status: accepting prs Please, send a pull request to resolve this! 🙏 area: react Related to JSX syntax, particularly for React components and the like. area: fixers Around how TypeStat fixes code. labels Nov 30, 2024
@JoshuaKGoldberg
Copy link
Owner

@all-contributors please add @pjonsson for bug.

🤖 Beep boop! This comment was added automatically by all-contributors-auto-action.
Not all contributions can be detected from Git & GitHub alone. Please comment any missing contribution types this bot missed.
...and of course, thank you for contributing! 💙

Copy link
Contributor

@JoshuaKGoldberg

I've put up a pull request to add @pjonsson! 🎉

JoshuaKGoldberg pushed a commit that referenced this issue Nov 30, 2024
Adds @pjonsson as a contributor for bug.

This was requested by JoshuaKGoldberg [in this
comment](#2019 (comment))

---------

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
@rubiesonthesky
Copy link
Collaborator

Similar thing can be seen in mutation test result here: https://github.com/JoshuaKGoldberg/TypeStat/pull/2053/files#diff-6319c84205dbc04fcb4e4500e85fdb381ba38ed92154df87de3a043702b361f0

It seems that the problem is mostly happening when creating component with a function. I suspect that the React area is written mostly old class components in mind. In that test we can see, that it's actually also adding the interface twice.

@rubiesonthesky
Copy link
Collaborator

I think at least one of these React problems probably comes isReactComponentNode being wrong, or at least it looks little bit wrong for me. There is comment // Functions can generally be React components if they have 0 or 1 parameters which is maybe correct but I actually think we should check if return type is JsxElement (or like). AST viewer I'm trying to understand / remember how you could access that information nicely.

rubiesonthesky added a commit to rubiesonthesky/TypeStat that referenced this issue Dec 7, 2024
rubiesonthesky added a commit to rubiesonthesky/TypeStat that referenced this issue Dec 7, 2024
@rubiesonthesky rubiesonthesky linked a pull request Dec 7, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: fixers Around how TypeStat fixes code. area: react Related to JSX syntax, particularly for React components and the like. status: accepting prs Please, send a pull request to resolve this! 🙏 type: bug Something isn't working :( 🐛
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants