Activity

1. RyanCavanaugh commented on Jun 9, 2022

   

   Ryan Cavanaugh (RyanCavanaugh)

   on Jun 9, 2022

   Last edited by RyanCavanaugh

   Member

   More actions

   kamranahmedse/developer-roadmap - Ryan
   coder/code-server - Daniel
   microsoft/playwright - Andrew
   vercel/hyper - Ryan
   react-hook-form/react-hook-form - Daniel
   typeorm/typeorm - Andrew
   mobxjs/mobx - Ryan
   palantir/blueprint - Daniel
   apollographql/apollo-client - Andrew

   collate

   kamranahmedse/developer-roadmap - Ryan
   mobxjs/mobx - Ryan
   vercel/hyper - Ryan
   react-hook-form/react-hook-form - Daniel
   coder/code-server - Daniel
   palantir/blueprint - Daniel
   microsoft/playwright - Andrew
   typeorm/typeorm - Andrew
   apollographql/apollo-client - Andrew
2. RyanCavanaugh commented on Jun 9, 2022

   

   Ryan Cavanaugh (RyanCavanaugh)

   on Jun 9, 2022

   Last edited by RyanCavanaugh

   Member

   More actions

   kamranahmedse/developer-roadmap

   const descendants = useRef(new DescendantsManager<T, K>())

   Here, K is defaulted to { } but unconstrained. DescendantsManager's second type argument is constrained: K extends Record<string, any> = {}. There are two instances in this file. Adding extends Record<string, any> to both instances fixes the errors with no follow-on errors.
3. ahejlsberg commented on Jun 9, 2022

   

   Anders Hejlsberg (ahejlsberg)

   on Jun 9, 2022

   Member

   More actions

   Note that I would expect a significant number of these to result from the fact that an unconstrained type parameter is no longer assignable to {} or Record<string, any> in --strictNullChecks mode (introduced in #49119, which re-incorporated the change from #48366). Those are entirely appropriate new errors since an unconstrained type parameter could be null or undefined.
4. RyanCavanaugh commented on Jun 9, 2022

   

   Ryan Cavanaugh (RyanCavanaugh)

   on Jun 9, 2022

   Last edited by RyanCavanaugh

   Member

   More actions

   mobxjs/mobx

   This method called object has an unconstrained type parameter defaulted to any:

       object<T extends object = any>(
           props: T,
           decorators?: AnnotationsMap<T, never>,
           options?: CreateObservableOptions
       ): T {
           return extendObservable(
               globalState.useProxies === false || options?.proxy === false
                   ? asObservableObject({}, options)
                   : asDynamicObservableObject({}, options),
               props,
               decorators
           )
       },

   it calls extendsObservable which has a constraint of Object (yes, Object) on both of its type parameters:

   export function extendObservable<A extends Object, B extends Object>(
   

   Adding extends object to T fixes the error with no follow-on errors.
5. RyanCavanaugh commented on Jun 9, 2022

   

   Ryan Cavanaugh (RyanCavanaugh)

   on Jun 9, 2022

   Member

   More actions

   vercel/hyper

   All these breaks were in external .d.ts files.

   conf had

   export declare type Migrations<T> = Record<string, (store: Conf<T>) => void>;

   where Conf<T> was constrained

   declare class Conf<T extends Record<string, any> = Record<string, unknown>> 

   This was fixable by upstreaming the constraint

   type-fest had two instances

   Key extends keyof WithStringKeys<BaseType>

    WithStringKeys<BaseType>[Key]

   This required two layers of upstreaming the constraint, which in turn broke another package (electron-store) because we surfaced the constraint on Except's first type argument:

   type Options<T> = Except<ConfOptions<T>, ...
6. sandersn commented on Jun 9, 2022

   

   Nathan Shively-Sanders (sandersn)

   on Jun 9, 2022

   Last edited by sandersn

   Member

   More actions

   I believe conf and type-fest have fixes in their newest versions.
   Edit: I double-checked, and they do.
   Edit 2: electron-store also has a fix, but it hasn't shipped yet. I pinged Sindre about it.
   Edit 3: electron-store shipped its fix about 10 hours ago.
7. andrewbranch commented on Jun 9, 2022

   

   Andrew Branch (andrewbranch)

   on Jun 9, 2022

   Member

   More actions

   playwright

   Ostensibly a missing constraint on the type parameter of a subclass whose superclass required a constraint. However, the type of said type parameter is never meaningfully witnessed in the subclass. Its only usage is like this:

   class DummyChannelOwner<T> extends ChannelOwner<T> {} // unconstrained `T` not assignable to `{}`
   // ...
   let result: ChannelOwner<any> = new ChannelOwner(/* only inference source typed `any` here */)

   and then result is returned in a method whose return type is any, and the one caller of that method does not use the return value. In other words, this error did not catch a bug in practice, because the offending type parameter should never have been declared in the first place.

   typeorm

   Every issue was resolvable by adding a missing constraint. The appropriate constraint, for internal consistency, was an interface called ObjectLiteral defined as just a string-to-any index signature. It seems like extends object or even extends {} would have squelched the errors due to the very loose assignability of that index signature, but it seemed like the correct thing was to propagate ObjectLiteral through, so the codefix wasn’t usable where offered. I also noticed that there were places where the codefix wasn’t offered due to the failing assignment being of the array form T[] is not assignable to ObjectLiteral[]. I also noticed one instance where the codefix would have been good, and the related diagnostic “...probably needs an extends object constraint” was attached, but there was no corresponding codefix.

   Did this catch a real bug: I think so? It’s really hard to tell. The unconstrained Entity type parameters are treated like opaque values throughout most of the codebase. I finally traced it down to where it does some object operations on those values, and some of those functions actually guard against null and undefined even though the existing types refute that possibility. Others guard only against undefined, and others do not check for either. So, I’m pretty sure in 4.7 it would have been possible to invoke something with null and/or undefined and find something that crashes like 20 stack frames later.

   apollo-client

   First one I looked at showed the codefix / related diagnostic as buggy again; duplicated related info and no codefix:

   

   Second one, related diagnostic is correct but again no codefix. Starting to wonder if the codefix ever works?

   A couple more and it appears that this codebase has probably mistaken type parameter defaults for constraints through and through. Virtually every type parameter has a default and no constraint. Many of the defaults are any. I tried to see if I could trace one down to where it would crash, but the values of these unconstrained type parameters almost always get assigned to a property that was optional, so down the line, the code tends to guard against falsy values. It seems 100% clear that an object constraint is the intent, but I can’t easily find a place where a null or undefined would not be gracefully handled. (This is basically what I expected to find in all of these: yes, the types are clearly wrong; yes, because the types are wrong, it would be easy to make a code change that type checks but crashes at runtime; no, I cannot find existing instances of code like that.)
8. DanielRosenwasser commented on Jun 10, 2022

   

   Daniel Rosenwasser (DanielRosenwasser)

   on Jun 10, 2022

   Member

   More actions

   react-hook-form

   • I don't think I got the quick fix at all.
   • Lots of places just needed an constraint.
     • Some of those constraints were clearly intentional and more specific than - for example,
       • In createFormControl, there's an unconstrained U which is used against an all-optional object type called SetValueConfig. U should have had a meaningful constraint that extended that type.
       • In form.ts, getProxyFormState.ts, getResolverOptions.ts, fieldArray.ts, resolvers.ts, and others a TFieldValues probably should always extends FieldValues, though it's just an alias for Record<string, any>. So there is readability intent, but this is the same as just adding object for most cases.
   • There's a few cases that does look questionable - like probable bugs - in getFieldAsValue.ts and validateField.ts - but it's not from the stronger check on generics, it's from the improved narrowing. At the end of the day, there's 100% no reason that these functions had to be generic though.
   • I don't think it was hard to fix these, but I don't think these would've been solvable on the publicly-produced .d.ts file

   I sent the fixes upstream over at react-hook-form/react-hook-form#8484.

   code-server

   This repo feels kind of like a less-than-useful benchmark because it's just letting us report over an old copy of VS Code; but I guess it gave us a chance to rerun on VS Code from before they dealt with this break over two months ago (microsoft/vscode@46abb2b). In theory this double type assertion should no longer be necessary Matt Bierner (@mjbvz).

   Otherwise, Matt Bierner (@mjbvz) already upgraded to a recent nightly and fixed other breaks due to narrowing, this one involving how functions are no longer narrowed (microsoft/vscode@640898d).

   Since there was no work for me to do, I will have to defer to the VS Code team's feedback.

   blueprint

   I spent a bit of time last week trying to figure this one out with Anders Hejlsberg (@ahejlsberg). It seems like we have regressed in some capacity with respect to destructuring defaults. Andrew Branch (@andrewbranch) might know more about this since he played around with destructuring recently, but I don't recall the exact change or whether it's even relevant.

   export interface BarProps {
       barProp?: string;
   }
   
   export interface FooProps {
       fooProps?: BarProps & object;
   }
   
   declare const foo: FooProps;
   const { fooProps = {} } = foo;
   
   fooProps.barProp;
   //       ~~~~~~~
   // error! Property 'barProp' does not exist on type '{}'.ts(2339)

   It seems like every single break here comes from this. There's not a clear fix, and it seems like a regression, so I didn't try fixing it (though I spent a lot of time getting a minimal repro).
9. Ryan Cavanaugh (RyanCavanaugh)

   closed this as completedon Jun 10, 2022
10. DanielRosenwasser commented on Jun 10, 2022

    

    Daniel Rosenwasser (DanielRosenwasser)

    on Jun 10, 2022

    Member

    More actions

    I opened up #49480 for the {} default issue, which was introduced by the intersection reduction PR at #49119 Anders Hejlsberg (@ahejlsberg).
11. Daniel Rosenwasser (DanielRosenwasser)

    added

    DiscussionIssues which may not have code impactIssues which may not have code impact

    on Jun 13, 2022
12. DanielRosenwasser commented on Jun 13, 2022

    

    Daniel Rosenwasser (DanielRosenwasser)

    on Jun 13, 2022

    Last edited by DanielRosenwasser

    Member

    More actions

    Have others sent PRs to these projects? I sent out

    • Funnel along appropriate constraints for (upcoming) TypeScript 4.8 react-hook-form/react-hook-form#8484
    • Add constraint to DummyChannelOwner for (upcoming) TypeScript 4.8 playwright#14840
13. RyanCavanaugh commented on Jun 13, 2022

    

    Ryan Cavanaugh (RyanCavanaugh)

    on Jun 13, 2022

    Member

    More actions

    type-fest seems to have been updated already

    I sent a PR to the other project

    👍React with 👍1Reacted by Daniel Rosenwasser
14. Andrew Branch (andrewbranch)

    added a commit that references this issue on Jun 13, 2022

    build: Fix build errors in TypeScript 4.8

    ...

    cbbe3ff
15. andrewbranch commented on Jun 13, 2022

    

    Andrew Branch (andrewbranch)

    on Jun 13, 2022

    Member

    More actions

    • Fix compilation errors in TypeScript 4.8 typeorm/typeorm#9106
    • Fix compilation error in TypeScript 4.8 playwright#14847
    • I didn’t make it all the way through updating apollo-graphql before recognizing that all the issues were the same
16. Umed Khudoiberdiev (pleerock)

    added a commit that references this issue on Jun 28, 2022

    fix: support for TypeScript 4.8 (#9106)

    ...

    d924b70