This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

@typescript-bot test it

Starting jobs; this comment will be updated as builds start and complete.

Hey @jakebailey, it looks like the DT test run failed. Please check the log for more details.

You can check the log here.

@jakebailey Here are the results of running the user tests with tsc comparing main and refs/pull/60718/merge:

There were infrastructure failures potentially unrelated to your change:

• 1 instance of "Package install failed"

Otherwise...

Everything looks good!

Hey @jakebailey, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@jakebailey
The results of the perf run you requested are in!

The perf test indicates an extra error in the vscode codebase, which should be looked at.

Follow up about the need of the considerNonUniformPrimitivePropDiscriminant flag to disable this new thing in assignments. Could we just accept the following three? They come from tests/cases/conformance/types/union/contextualTypeWithUnionTypeObjectLiteral.ts.

var objStrOrNum3: { prop: string } | { prop: number } = {
    prop: strOrNumber
};

var objStrOrNum6: { prop: string; anotherP: string; } | { prop: number } = {
    prop: strOrNumber,
    anotherP: str
};

var objStrOrNum8: { prop: string; anotherP: string; } | { prop: number; anotherP1: number } = {
    prop: strOrNumber,
    anotherP: str,
    anotherP1: num
};

They are all sound if you think about it, maybe just a little bit controversial. We could remove that flag and the implementation of this PR would be cleaner and probably a little bit faster as well.

Edit: Super seeded by the following changes.

@typescript-bot test it

Starting jobs; this comment will be updated as builds start and complete.

@gabritto Here are the results of running the user tests with tsc comparing main and refs/pull/60718/merge:

Everything looks good!

Hey @gabritto, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@gabritto
The results of the perf run you requested are in!

22 new errors just in vscode. This will be fun

@gabritto Here are the results of running the top 400 repos with tsc comparing main and refs/pull/60718/merge:

Something interesting changed - please have a look.

compiler-explorer/compiler-explorer

if (currentLabel !== null && currentLabel.initialLine === undefined) {
    currentLabel.initialLine = ln;
}

currentLabel has type Label | null. If it is not null then currentLabel.initialLine has type number. It cannot be undefined! TS now understands this impossible situation and types currentLabel as never inside the if.

continuedev/continue

In addition to this, this, this and this. Please note they do not set strict: true and have strictNullChecks disabled.

A PackageDocsResult is defined as:

type PackageDocsResult = {
  packageInfo: ParsedPackageInfo;
} & (
  | { error:  string; details?: never }
  | { details: PackageDetailsSuccess; error?: never }
);

In AddDocsDialog they also do:

if (docsResult.error || !docsResult.details) {
  // ...
  return
}

docsResult // used after but has type 'never' here

It seems the above if makes the { details: PackageDetailsSuccess; error?: never } case disappear when checking docsResult.error. It seems just another instance of #60702. Typing properties as never, even as ?: never when strictNullChecks: false, seems to be a really bad idea.

When it comes to CompletionProvider.ts, they do:

// Temporary fix for JetBrains autocomplete bug as described in https://github.com/continuedev/continue/pull/3022
if (llm.model === undefined && llm.completionOptions?.model !== undefined) {
  llm.model = llm.completionOptions.model;
}

But llm.model has type string. It cannot be undefined! So guess who becomes never...

desktop/desktop

About app/src/ui/app.tsx:

enum DragType {
  Commit,
}

switch (currentDragElement.type) {
  case DragType.Commit:
    return (
      <CommitDragElement
        gitHubRepository={gitHubRepository}
        commit={commit}
        selectedCommits={selectedCommits}
        emoji={emoji}
        accounts={this.state.accounts}
      />
    )
  default:
    return assertNever(
      currentDragElement.type,
      `Unknown drag element type: ${currentDragElement}`
    )
}

It seems they do not need that assertNever anymore 🙄.

When it comes to app/src/ui/test-notifications/test-notifications.tsx:

TestNotificationType {
  PullRequestReview,
  PullRequestComment,
  ChecksFailed,
}

switch (selectedFlow.type) {
  case TestNotificationType.PullRequestReview: {
    // ...
    break
  }
  case TestNotificationType.PullRequestComment: {
    // ...
    break
  }
  case TestNotificationType.ChecksFailed: {
    // ...
    break
  }
  default:
    assertNever(selectedFlow.type, `Unknown flow type: ${selectedFlow}`)
}

Same thing!
Here is a possible solution.

ether/etherpad-lite

Nothing new! Same as here.

google/blockly

if (typeof label === 'string') {
  return [parsing.replaceMessageReferences(label), value];
}

hasImages = true;

const imageLabel =
  label.alt !== null
    ? { ...label, alt: parsing.replaceMessageReferences(label.alt) }
    : { ...label }; // <-- new error here

The alt property has type string. It cannot be null! So label is typed as never in the false branch of the ternary. This is a consequence of having narrowed label few lines before from string | ImageProperties to ImageProperties.

ionic-team/ionic-framework

if (data.year !== undefined) {
  // ...
} else if (data.hour !== undefined) { // <-- new error here
  rtn = twoDigit(data.hour) + ':' + twoDigit(data.minute); // <-- new errors here
}

We have that data was previously narrowed from DatetimeParts | DatetimeParts[] to DatetimeParts, which year field has type number. It cannot be undefined! So in the else block data is typed as never.

P.S. DatetimeParts is defined as:

interface DatetimeParts {
  month: number;
  day: number | null;
  year: number;
  dayOfWeek?: number | null;
  hour?: number;
  minute?: number;
  ampm?: 'am' | 'pm';
}

Maybe they forgot to set year?: number | number? Dunno.

microsoft/vscode

In addition to this.

We have something interesting when it comes to error TS7022: 'groupEntry' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer. at src/vs/editor/standalone/common/monarch/monarchLexer.ts#L614.

This is interesting because TS loses the ability to solve a circular type dependency while inferring. There is this situation where types of identifiers are mutually dependent. We are in a big while loop with multiple re-assignments: groupEntry and matches depend on groupMatching, which starts as null but gets reassigned near the end, and from groupEntry we get action which will be part, directly or indirectly, of the next version of groupMatching itself:

let groupMatching: GroupMatching | null = null;

let forceEvaluation = true;
let pos = 0
let lineLength = 0

while (forceEvaluation || pos < lineLength) {
  let matches: string[] | null = null;
  let action: FuzzyAction | FuzzyAction[] | null = null;
  let rule: IRule | null = null;

  if (groupMatching) {
    const groupEntry = groupMatching.groups.shift()!; // <-- new error here

    // they depend on groupMatching
    action = groupEntry.action;
    matches = groupMatching.matches;
    rule = groupMatching.rule;
  }

  // things happens with action and matches
  result = ...

  // recursive dependency here
  if (matches.length === fn(result)) {
    // and here
    groupMatching = {
      rule: rule,
      matches: matches,
      groups: []
    };
  }
}

I don't know the reason why the new discriminating capability doesn't let TS solve this circularity anymore. Of course we could easily fix this situation by manually solving the circularity:

const groupEntry: {
  action: monarchCommon.FuzzyAction;
  matched: string;
} = groupMatching.groups.shift()!;

As a bonus, the problem seems to be related to the "recursive" inferred type for groupMatching. In fact, even the following type assertion solves this issue:

groupMatching = {
  rule: rule,
  matches: matches,
  groups: []
} as GroupMatching;

src/vs/platform/workspaces/node/workspaces.ts#L76:

if (typeof folderStat.birthtimeMs === 'number') {
  ctime = Math.floor(folderStat.birthtimeMs); 
} else {
  ctime = folderStat.birthtime.getTime(); // <-- new error here
}

We have that folderStat was already narrowed before from Stats | undefined to Stats. But birthtimeMs has type number, undefined is not allowed! So folderStat becomes never in that else branch.

src/vs/workbench/services/label/common/labelService.ts#L278:

if (hasMultipleRoots && !options.noPrefix) {
  const rootName = folder?.name ?? basenameOrAuthority(folder.uri); // <-- new error here
  relativeLabel = relativeLabel ? `${rootName} • ${relativeLabel}` : rootName;
}

Same story: folder was narrowed and its name property has type string so it cannot be undefined. Probably they should use || to catch the empty string case if their intention was to provide a fallback.

src/vs/workbench/services/workingCopy/common/workingCopyBackupService.ts#L515

if (typeof meta?.typeId === 'string') {
    delete meta.typeId; // <-- new error here
    
    if (isEmptyObject(meta)) {
    meta = undefined;
    }
}

The type of typeId is ?: undefined, so that typeof check is now considered by TS as impossible to be satisfied.

Redocly/redoc

We have:

const { type, oneOf, discriminatorProp, isCircular } = schema;
// discriminatorProp has type string

// ...

if(discriminatorProp !== undefined) {
  // ...
}

// schema is never from now on, so new errors here:
schema.fields?.length
schema.description
schema.externalDocs

Here discriminatorProp, that comes from destructuring schema itself, has type string. It cannot be undefined! Therefore it is typed as never after that if, and so will be schema.