@typescript-bot test this

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

Also run on DT

@typescript-bot run dt

Heya @weswigham, I've started to run the Definitely Typed test suite on this PR at 7c13b70. You can monitor the build here. It should now contribute to this PR's status checks.

Updated the test that needed changing to a constraint that actually typechecks, and looked at DT - failures are in ramda (and are also failing in master due to the recent function generic inference changes) and in a package callec victory - the diff in victory appears unrelated and already fixed, given that a local dtslint on it run doesn't show any errors.

So this seems pretty good - if anything, like the tests I just changed, it's going to identify places we were silently allowing unsafe operations.

@weswigham let's merge for 3.5

@weswigham this sounds like it's ready to go, right?

Probably. The change itself is small, so barring merge conflicts, should still be OK.

After a merge from master: toString is now available on type parameters that extend any. That's the only difference I can see.

Let's wait until @RyanCavanaugh is back to merge this.

I talked to @RyanCavanaugh about this just now, so I'm going to merge it.

@weswigham this causes a failure in DT on wordpress__block-editor-tests.tsx. Can you try to fix it? It looks like the offending signature is this:

declare function withColorContext<
    ProvidedProps extends Partial<withColorContext.Props>,
    OwnProps extends any,
    T extends ComponentType<ProvidedProps & OwnProps>
>(component: T):
    T extends ComponentType<infer U> ? ComponentType<
        Omit<U, 'colors' | 'disableCustomColors' | 'hasColorsToChoose'> &
        Omit<ProvidedProps, 'hasColorsToChoose'>> :
    never;

Previously the second type argument was inferred OwnProps=any; now it's OwnProps=unknown (as expected).

If there's a mechanical fix to update existing code, it'd worthwhile to have that in the release notes for 3.9.

If there's a mechanical fix to update existing code, it'd worthwhile to have that in the release notes for 3.9.

Write an Extract everywhere we now (correctly) flag as a type error, `(or, better yet, write types that actually check out without contortions). It's the typesystem equivalent of a typecast, but it's much more explicit.

eg,

type GetClass<T extends any> = T["class"];

// could be
type GetClass<T> = Extract<T, {class: any}>["class"];

obviously, you can replace Extract with a helper that provides a more reasonable default on failure than never for your given situation, too.

this causes a failure in DT on wordpress__block-editor-tests.tsx. Can you try to fix it? It looks like the offending signature is this:

What's broken about it? the signature itself has nothing wrong as far as I can tell, so you're talking about a specific usage, right?

What's broken about it?

If the answer is "we now infer unknown instead of any at use sites because we never make inferences", adding an = any default will silence the error and change the default only back to any. But I'd still consider an any-defaulting inference questionable, myself. You probably shouldn't have type parameters you can never infer...

Yes, line 142 of the tests. Probably = any to silence the error is good enough really.