-
Notifications
You must be signed in to change notification settings - Fork 13k
Improve how intersections are classified as weak types #60889
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
base: main
Are you sure you want to change the base?
Improve how intersections are classified as weak types #60889
Conversation
if (!v) { | ||
return 0; | ||
} | ||
result = result > v ? result : v; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea behind the fix is that an empty object type should not be considered to be weak on its own. That's comes from the pre-existing requirement for weak types:
resolved.properties.length > 0 && every(resolved.properties, p => !!(p.flags & SymbolFlags.Optional))
However, when it's intersected with a weak type that should not make it, out of a sudden, non-weak. The new logic still maintains the invariant that an intersection of many empty interfaces is not considered to be weak (it should obey the same rules as a single empty interface).
All of that doesn't change the definition put in the existing code comment:
/**
* A type is 'weak' if it is an object type with at least one optional property
* and no required properties, call/construct signatures or index signatures
*/
Actually, the old behavior wasn't enforcing those rules for intersections precisely enough. The proposed adjustment is closer to checking the intersection as a single type with a flat list of properties instead of checking separate types and combining those results "blindly". I think it's more accurate to do it this way because for most purposes an intersection shouldn't be distinguishable from the same type with a "flattened" list of properties.
@typescript-bot test it |
Hey @jakebailey, the results of running the DT tests are ready. There were interesting changes: Branch only errors:Package: relay-test-utils
Package: react-gateway
Package: react-helmet-with-visor
Package: meteor-dburles-collection-helpers
Package: react-instantsearch-native
Package: react-aria-live
Package: react-native-modals
Package: reactable
Package: reselect-tools
Package: react-switch-case
Package: solid__react
Package: react-native-web
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@jakebailey Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
@jakebailey Here are some more interesting changes from running the top 400 repos suite Details
|
@jakebailey Here are some more interesting changes from running the top 400 repos suite Details
|
@jakebailey Here are some more interesting changes from running the top 400 repos suite Details
|
@jakebailey Here are some more interesting changes from running the top 400 repos suite Details
|
@jakebailey Here are some more interesting changes from running the top 400 repos suite Details
|
@jakebailey Here are some more interesting changes from running the top 400 repos suite Details
|
fixes #56995