Skip to content

Fix nested excess property checks when target is a union with non-discriminated + non-object constituents #30771

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

Closed
wants to merge 3 commits into from

Conversation

RyanCavanaugh
Copy link
Member

After checking an object type for excess properties, we un-fresh it if the target is a union with nullable members. The intent of that is that code like this

type Q = { a: string } & Partial<{ b: string>};
const q: Q = { a: "" }

doesn't error because a is excess in Partial<{ b: string>} (even though it's assignable to it).
Similarly, we don't want to consider a excess here:

type Q = { a: string } & Partial<{ b: string>};
const q: Q = { a: "" }

This un-freshening doesn't happen if there's a findable discriminating property, as you would expect.

However, we don't consider this example to have a discriminant

interface IProps {
  nestedProp: {
    testBool?: boolean;
  }
}
 
interface INestedProps {
  nestedProps: IProps;
}
// Want an error generated here but there isn't one.
const propA1: INestedProps | number = { nestedProps: { INVALID_PROP_NAME: 'share', iconProp: 'test' } };

... even though the number type is sort of "self-discriminating" in the context of a union with other types.

The fix is to a) rename the singly-called function to match its use and b) skip non-object targets when considering whether or not we need to un-fresh the source type.

The only non-new-test baselines are improvements in error spans

@RyanCavanaugh RyanCavanaugh changed the title Fix nested excess property checks Fix nested excess property checks when target is a union with non-discriminated + non-object constituents Apr 5, 2019
@weswigham
Copy link
Member

@RyanCavanaugh #30853 may subsume this - it remodels how we track excess property checking entirely, so we're no longer concerned with tracking which types need "unfresh"ness, and instead just always "unfresh" when comparing union/intersection members, and then do a followup check for excess properties on the whole union/intersection construct if needed.

weswigham added a commit to weswigham/TypeScript that referenced this pull request Apr 10, 2019
@weswigham
Copy link
Member

Yep, it does. I've included your test case in that PR. 🚂

weswigham added a commit that referenced this pull request Apr 17, 2019
…30853)

* Perform excess property checking on intersection and union members

* Allow partial union props to contain the undefined type

* Add test case from #30771

* Un-terse getPossiblePropertiesOfUnionType side-effecting code

* Fix bug exposed in RWC

* Cache results of getPossiblePropertiesOfUnionType

* Fix whitespace
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