Skip to content

Conversation

@ahejlsberg
Copy link
Member

This PR fixes the following two issues:

  • In --strictOptionalProperties mode, inference from an optional property to an index signature simply infers the property type as written. For example, inferring from { a?: number } to { [key: string]: T } infers number for T, and inferring from { a?: number | undefined } to { [key: string]: T } infers number | undefined for T.
  • In --strictOptionalProperties mode, a mapped type that strips optionality doesn't strip explicitly included undefined types. For example, Required<{ a?: number }> is { a: number } and Required<{ a?: number | undefined }> is { a: number | undefined }.

Fixes parts of #44494.

}

function removeType(type: Type, targetType: Type) {
return filterType(type, t => t !== targetType);
Copy link
Member

Choose a reason for hiding this comment

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

This is maybe a micro-optimization, but because unions are sorted, and all the undefined type variants are such low type IDs, if targetType is a union, we only need to check the first union element, rather than all of them exhaustively, IIRC.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but filterType also does stuff for the (denormalized) origin type and we'd have to duplicate that as well.

@DanielRosenwasser DanielRosenwasser changed the title Fixes to inference and mapped types in --strictOptionaProperties mode Fixes to inference and mapped types in --strictOptionalProperties mode Jun 15, 2021
@ahejlsberg ahejlsberg merged commit 6bb1f07 into main Jun 15, 2021
@ahejlsberg ahejlsberg deleted the fix44494 branch June 15, 2021 21:16
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants