Skip to content

Bring typeof switch closer inline with if - #27680

Merged
Wesley Wigham (weswigham) merged 1 commit into
microsoft:masterfrom
jack-williams:bring-typeof-switch-inline-with-if
Oct 26, 2018
Merged

Bring typeof switch closer inline with if#27680
Wesley Wigham (weswigham) merged 1 commit into
microsoft:masterfrom
jack-williams:bring-typeof-switch-inline-with-if

Conversation

@jack-williams

@jack-williamsJack Williams (jack-williams) commented Oct 10, 2018•
edited
Loading

Copy link
Copy Markdown
Collaborator

This PR brings switch narrowing with typeof closer inline with if, which was recently updated in two keys ways:

  1. unknown narrowing for "function" and "object" cases: the former should be Function, the latter object | null.
  2. narrowing aswell as filtering for union type constituents.

Fixes#27180
Fixes#27335
Adds tests for #27181

@jack-williamsJack Williams (jack-williams) changed the title Bring typeof switch inline with ifBring typeof switch closer inline with ifOct 10, 2018
@jack-williamsJack Williams (jack-williams) mentioned this pull request Oct 10, 2018
Closed
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) reviewed Oct 10, 2018
View reviewed changes
Comment threadsrc/compiler/checker.ts Outdated
return caseType.flags & TypeFlags.Never ? defaultType : getUnionType([caseType, defaultType]);
}

function getTypeFromName(type: Type, text: string) {

@RyanCavanaughRyan Cavanaugh (RyanCavanaugh)Oct 10, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would prefer a more descriptive name like getNarrowedTypeByTypeofResult

@jack-williamsJack Williams (jack-williams)Oct 10, 2018

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Went with getImpliedTypeFromTypeofCase to be consistent with terminology used in the documentation of narrowBySwitchOnTypeOf.

@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Oct 10, 2018

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) test this

@typescript-bot

TypeScript Bot (typescript-bot) commented Oct 10, 2018•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Ryan Cavanaugh (@RyanCavanaugh), I've started to run the extended test suite on this PR at b12ed40. You can monitor the build here. It should now contribute to this PR's status checks.

@jack-williams
Bring typeof switch inline with if
b63b40b
- Narrow unknown
- Narrow union members (in addition to filtering)
@jack-williams
Jack Williams (jack-williams)force-pushed the bring-typeof-switch-inline-with-if branch from b12ed40 to b63b40bCompareOctober 10, 2018 20:02
@weswigham

Wesley Wigham (weswigham) commented Oct 10, 2018

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) test this but again since it was edited (it passed before, so it should be fine, but just in case~)

@typescript-bot

TypeScript Bot (typescript-bot) commented Oct 10, 2018•
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Wesley Wigham (@weswigham), I've started to run the extended test suite on this PR at b63b40b. You can monitor the build here. It should now contribute to this PR's status checks.

@weswigham
Wesley Wigham (weswigham) merged commit abce9ae into microsoft:masterOct 26, 2018
@jwmerrillJason Merrill (jwmerrill) mentioned this pull request Dec 13, 2018
Closed
@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@RyanCavanaughRyan Cavanaugh (RyanCavanaugh)RyanCavanaugh left review comments
@weswighamWesley Wigham (weswigham)weswigham approved these changes

Assignees

@weswighamWesley Wigham (weswigham)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

typeof switch filters union types but does not narrow them Switch on typeof does not narrow unknown correctly.

4 participants

@jack-williams@RyanCavanaugh@typescript-bot@weswigham