react: Fix test failure from merge conflict. #38603
Conversation
A test written on 12 Sep was merged on 24 Sep. In between, the Props definition it used changed so that the new test was an error. This PR just clones the original Props type and gives it a new name.
sandersn
changed the title
typescript-bot
added
Owner Approved
|
@sandersn Thank you for submitting this PR! 🔔 @johnnyreilly @bbenezech @pzavolinsky @digiguru @ericanderson @DovydasNavickas @theruther4d @guilhermehubner @ferdaber @jrakotoharisoa @pascaloliv @Hotell @franklixuefei @Jessidhia @saranshkataria @lukyth @eps1lon - please review this PR in the next few days. Be sure to explicitly select If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead. |
|
A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped! |
| @@ -402,7 +402,13 @@ const ForwardingRefComponent = React.forwardRef((props: {}, ref: React.Ref<RefCo | |||
| return React.createElement(RefComponent, { ref }); | |||
| }); | |||
|
|
|||
| const ForwardingRefComponentPropTypes: React.WeakValidationMap<Props> = {}; | |||
| interface AttributeProps extends React.Attributes { | |||
There was a problem hiding this comment.
| interface AttributeProps extends React.Attributes { | |
| interface AttributeProps { |
React.Attributes is not meant to be used that way since it includes props that you never see inside your component or need to validate with propTypes. It's technically only a private type.
There was a problem hiding this comment.
The test is broken without extends React.Attributes -- WeakValidationMap<AttributeProps> doesn't have any properties in common with WeakValidationMap<RefAttributes<RefComponent>> from ForwardRefExoticComponent<RefAttributes<RefComponent>>
Does that mean the test is broken or the new propTypes property in ForwardRefExoticComponent is wrong? I suppose I could say
const ForwardingRefComponentPropTypes: React.WeakValidationMap<React.RefAttributes<RefComponent>> = {};in the test instead.
There was a problem hiding this comment.
I don't know React that well, I'm just trying to get it unblocked for other contributors, so any advice you have is helpful.
There was a problem hiding this comment.
Well lets merge this if this blocks other work. I'll take look later why we need this.
There was a problem hiding this comment.
Thanks. I merged it.
@rpokorny Maybe you can take another look too?
typescript-bot
moved this from Check and Merge
to Needs Author Attention
in Pull Request Status Board
typescript-bot
added
Revision needed
|
@sandersn One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits or comments. Thank you! |
A test written in #38337 on 12 Sep was merged on 24 Sep. In between in #38352, the Props definition it used changed so that the new test was an error.
This PR just clones the original Props type and gives it a new name. This fixes the error.