Investigate new RWC failures

[weswigham]

We haven't merged our RWC failures from the last 9 days and they've really added up (which is, I assume, why nobody is looking at them or merging them). Most of them are benign - error span/message changes for the new overload errors, some expected duplicate definition errors related to the Generator types change and older node declaration files. However, there's one pattern of changes alone these lines (mostly within s4l):

        protected _buildState(props: P, initialBuild: boolean): Partial<S> {
            const newState = {
                optionToPresent: this._getOptionFromConf()
            } as S;
            return newState;
            ~~~~~~~~~~~~~~~~
!!! error TS2322: Type 'S' is not assignable to type 'Partial<S>'.
!!! error TS2322:   Type 'ILayoutExperimentState' is not assignable to type 'Partial<S>'.
        }

that's suuuuuper concerning, since a T is pretty much always supposed to be assignable to a Partial<T>, meaning we've broken something pretty important ❤️

I've merged the current baselines so we can actually browse thru the diffs again, but this particular regression needs to be seriously investigated and addressed. Best guess is that #32071 interacted poorly with another change added to master (or the other failure in master hid this one) over the same period, but I dunno.

[weswigham]

cc @andrewbranch

[andrewbranch]

minimal repro:

function f<T extends { x: {} }>(): Partial<T> {
  return undefined! as T;
  // Type 'T' is not assignable to type 'Partial<T>'.
  //   Type '{ x: {}; }' is not assignable to type 'Partial<T>'.ts(2322)
}

[andrewbranch]

Confirmed reverting #32071 makes the error go away :/

[weswigham]

When we make the intersection of keys there, we should just have keyof T & keyof T, which keyof T should trivially be assignable to and from, right? So does that imply that it's the reindexing with keyof T that's at fault? (Since then we'll be comparing a T[keyof T] with a set of values.) Do we need to take the (lower) bound of the key before indexing (rather than relying on it happening after)? I'm just speculating, since I haven't actually debugged to see where the reality of the comparison differed from the expectation.

[andrewbranch]

type Partial<T> = { [P in keyof T]?: T[P]; }

function f<T extends { x: {} }>(): Partial<T> {
  return undefined! as T;
}

Ok, so stepping through the code how it was before #32071:

1. targetConstraint and sourceKeys are ===; both an object which I interpret to represent keyof T
2. hasOptionalUnionKeys is falsey so filteredByApplicability is undefined
3. The key comparison logic becomes isRelatedTo(targetConstraint, sourceKeys) which is of course true because they’re the same type
4. indexingType is getTypeParameterFromMappedType(target) which is P
5. indexedAccessType is T[P]
6. templateType is a union of indexedAccessType and undefined
7. Thus isRelatedTo(indexedAccessType, templateType) of course returns true

What happens after #32071:

1. Same
2. hasOptionalUnionKeys was replaced by includeOptional and it’s truthy
3. filteredByApplicability is the intersection of targetConstraint and sourceKeys which are equal, so filteredByApplicability is also immediately strict equal to both of those
4. The key checking logic, since includeOptional is truthy, is that filteredByApplicability isn’t never. It’s not, so we continue.
5. indexingType is filteredByApplicability which is keyof T instead of P 🚨
6. indexedAccessType is then T[keyof T] instead of T[P]
7. templateType is a union of T[P] and undefined
8. isRelatedTo ends up comparing T[keyof T] with T[P], which eventually compares keyof T with P, which eventually compares string | number | symbol with P, and string is not related to P.

[weswigham]

So... We just need to intersect the target type parameter into the type we index with, so it's (keyof T) & P, then?

[andrewbranch]

That seems to do the trick.