Remove carve-out in conditional type instantiation that hopefully has been rendered unneeded

[weswigham]

Fixes #50688

This is all good w/ our local test suite, but the extended suite is what matters as, iirc, that's what originally motivated this line. Still, we've changed some other stuff about conditional instantiation since the original change, so hopefully this weird carve-out just isn't needed for compatibility anymore.

[weswigham]

@typescript-bot user test this inline
@typescript-bot test this
@typescript-bot run dt

[typescript-bot]

Heya @weswigham, I've started to run the diff-based user code test suite on this PR at 2fe0be0. You can monitor the build here.

Update: The results are in!

[typescript-bot]

Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at 2fe0be0. You can monitor the build here.

[typescript-bot]

Heya @weswigham, I've started to run the extended test suite on this PR at 2fe0be0. You can monitor the build here.

[typescript-bot]

@weswigham Here are the results of running the user test suite comparing main and refs/pull/51151/merge:

Something interesting changed - please have a look.

Details

puppeteer

packages/puppeteer-core/tsconfig.json

• [NEW] error TS2322: Type 'ElementHandle<any> | JSHandle<any>' is not assignable to type 'HandleFor<T[K]>'.
  • packages/puppeteer-core/src/common/JSHandle.ts#L169 in packages/puppeteer-core/src/tsconfig.esm.json
  • packages/puppeteer-core/src/common/JSHandle.ts#L169 in packages/puppeteer-core/src/tsconfig.cjs.json
• [NEW] error TS2536: Type 'FlattenHandle<K> | FlattenHandle<HandleFor<K>> | FlattenHandle<JSHandle<K>>' cannot be used to index type 'FlattenHandle<this>'.
  • packages/puppeteer-core/src/common/JSHandle.ts#L170 in packages/puppeteer-core/src/tsconfig.esm.json
  • packages/puppeteer-core/src/common/JSHandle.ts#L170 in packages/puppeteer-core/src/tsconfig.cjs.json
• [NEW] error TS2345: Argument of type 'string | Func' is not assignable to parameter of type 'string | EvaluateFunc<[JSHandle<FlattenHandle<HandleFor<NodeFor<Selector>>>[]>, ...Params]>'.
  • packages/puppeteer-core/src/common/ElementHandle.ts#L261 in packages/puppeteer-core/src/tsconfig.esm.json
  • packages/puppeteer-core/src/common/ElementHandle.ts#L261 in packages/puppeteer-core/src/tsconfig.cjs.json
• [NEW] error TS2322: Type 'unknown' is not assignable to type 'Awaited<ReturnType<Func>>'.
  • packages/puppeteer-core/src/common/ElementHandle.ts#L267 in packages/puppeteer-core/src/tsconfig.esm.json
  • packages/puppeteer-core/src/common/ElementHandle.ts#L267 in packages/puppeteer-core/src/tsconfig.cjs.json

[weswigham]

OK, I'm actually pretty happy with the puppeteer breaks - the source for puppeteer actually looks like it has the similar circularity issues to the linked issue (they just ignore them via comment pragma), which actually are fixed (at least partially) by this and thus surface these new (correct as far as I can tell) errors, since the return types in question no longer get error-any'd (though are maybe still circular in other ways, seeing as there's no Unused expect error directive error). RWC is clean (as may be expected of codebases predating conditional types entirely), DT is completely green (and, iirc, DT is what motivated this line in the first place).

[typescript-bot]

Heya @weswigham, I've started to run the diff-based top-repos suite on this PR at 2fe0be0. You can monitor the build here.

Update: The results are in!

[typescript-bot]

@weswigham Here are the results of running the top-repos suite comparing main and refs/pull/51151/merge:

Everything looks good!

[ahejlsberg]

I'm definitely good with removing that carve-out. In fact, I never really understood why it was needed in the first place.