Skip to content

Fixing react defaultize+generic default props interaction #27088

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

Merged

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Sep 14, 2018

Specifically, this fixes where the contextual type is places for self-closing tags (it should be on the attributes since there's no children to wrap, and definitely not the expression the tag is within), and fixes propagating flags for jsx attributes types (previously they propagated none, which wasn't a big deal before we introduced inference, but now they need to propagate flags correctly so they propagate the ContainsAnyFunctionType flag to prevent spurious inference results during the first pass).

Fixes #26395

I've also updated our test harness react.d.ts with a newer one, since it's adopted many features since the version currently present. Right now it's s separate file from the old one, but we probably need to be validating our tests against the new .d.ts; however there's ~100 tests to look through once you make the swap, which'll take a bit; I'd like to save that for a follow-up.

@weswigham
Copy link
Member Author

Plus side, this should probably fix the formik user suite test, finally. The indexed-access distribution was the first big chunk of the issue, and this should fix the last of it.

!!! error TS2322: Type '(a: { x: string; }) => string' is not assignable to type '((a: { x: string; }) => string) & ((cur: { x: string; }) => { x: string; })'.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This intersection is unfortunate. Is there a way to avoid adding (a: { x: string }) => string to the type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The target is Props & BaseProps<Values> where we've already infered Props as { initialValues: {x: string}; nextValues: (cur: {x: string}) => string; } (since that's the entire attributes type as written, which is inferred to Props) and BaseProps<Values> as interface BaseProps<T> { initialValues: T; nextValues: (cur: T) => T; } instantiated with T = Values = {x: string} (since that's what first matches T in the first context-free inference pass) - the intersection of both's nextValues member is the type we see here. Everything is as it should be.

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, although might be good to follow up on the one test comment I had.

@weswigham weswigham merged commit 4eb59a2 into microsoft:master Sep 14, 2018
@weswigham weswigham deleted the fixing-that-strange-react-problem branch September 14, 2018 21:18
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.

Regression with >= v3 and React, generics, defaultProps
2 participants