Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NewErrors] 4.5.0-dev.20210912 vs 4.4.3 #45846

Closed
typescript-bot opened this issue Sep 12, 2021 · 11 comments
Closed

[NewErrors] 4.5.0-dev.20210912 vs 4.4.3 #45846

typescript-bot opened this issue Sep 12, 2021 · 11 comments
Assignees

Comments

@typescript-bot
Copy link
Collaborator

The following errors were reported by 4.5.0-dev.20210912, but not by 4.4.3

microsoft/vscode

2 of 51 projects failed to build with the old tsc

extensions/github/tsconfig.json

  • error TS2339: Property 'pulls' does not exist on type '({ [x: string]: any; } & { [x: string]: any; } & Octokit & void & { paginate: PaginateInterface; } & RestEndpointMethods) | ReposCreateForkResponseData'.

src/tsconfig.json

src/tsconfig.monaco.json

src/tsconfig.tsec.json

reduxjs/react-redux

test/typetests/tsconfig.json

  • error TS2322: Type 'Context<ReactReduxContextValue<any, AnyAction> | null> | MutableRefObject<unknown> | ((instance: unknown) => void) | Omit<...>' is not assignable to type 'Context<ReactReduxContextValue<any, AnyAction> | null>'.
  • error TS2339: Property 'Consumer' does not exist on type 'Context<ReactReduxContextValue<any, AnyAction> | null> | MutableRefObject<unknown> | ((instance: unknown) => void) | Omit<...>'.

tsconfig.json

  • error TS2322: Type 'Context<ReactReduxContextValue<any, AnyAction> | null> | MutableRefObject<unknown> | ((instance: unknown) => void) | Omit<...>' is not assignable to type 'Context<ReactReduxContextValue<any, AnyAction> | null>'.
  • error TS2339: Property 'Consumer' does not exist on type 'Context<ReactReduxContextValue<any, AnyAction> | null> | MutableRefObject<unknown> | ((instance: unknown) => void) | Omit<...>'.

youzan/vant

2 of 9 projects failed to build with the old tsc

packages/create-vant-cli-app/tsconfig.json

lensapp/lens

4 of 5 projects failed to build with the old tsc

tsconfig.json

@andrewbranch
Copy link
Member

Most of these are from #45350@rbuckton, you’ll probably want to take a look at some of the assignability errors mentioning Awaited. There are a couple type argument issues but those seem trivial to fix. It’s more important to understand the others and see if they have easy fixes too.

@rbuckton
Copy link
Member

rbuckton commented Sep 16, 2021

Regarding extensions/github/src/pushErrorHandler.ts#L106: It looks like the call to withProgress at line 32 was previously returning a tuple type of [Octokit, ReposCreateForkResponseData], but is now returning (Octokit | ReposCreateForkResponseData)[]. If we were previously inferring a tuple here, but aren't due to the change to await then that's definitely a bug.

@rbuckton
Copy link
Member

It looks like extensions/github/src/pushErrorHandler.ts#L106 is due to #45719 since we're no longer inferring the type arguments as a tuple due to the binding pattern.

@rbuckton
Copy link
Member

@rbuckton
Copy link
Member

rbuckton commented Sep 17, 2021

@mjbvz I saw that you updated pushErrorHandler.ts for TS 4.5 using an as any on line 107

I would recommend you add as const on line 85 instead to preserve type safety. We are no longer treating untyped binding patterns as type argument inference sources as of #45719.

@mjbvz
Copy link
Contributor

mjbvz commented Sep 17, 2021

@rbuckton Ah nice! Just made the change. Thanks for the help as I was super confused by the error

@rbuckton
Copy link
Member

rbuckton commented Sep 17, 2021

For youzan/vant:

For lensapp/lens:

@rbuckton
Copy link
Member

I'm unsure as to the cause of the errors in react-redux, but they don't seem to be due to await->Awaited<T> change as far as I can tell.

@rbuckton
Copy link
Member

Ah, the issue with react-redux is also due to #45719:

image

We're no longer inferring a tuple type for the return statement because we don't infer a contextual type from the binding pattern. This seems like an unfortunate loss due to that change:

declare const f: <T>(cb: () => T) => T;
const [a, b, c] = f(() => [1, "hi", true]);

image

vs

image

@andrewbranch
Copy link
Member

Hm, there are so many cases where binding patterns as an inference source are so bad, but these tuple cases are kind of disappointing. Maybe I can bring that back for tuples. It's a lot more suspicious to say that an object binding pattern implies the presence of some properties on an otherwise unknown type than it is to say that an array binding pattern implies that an array literal should be interpreted as a tuple.

@andrewbranch
Copy link
Member

Ron fixed the Awaited issues and I rolled back the binding pattern changes for now (see design notes linked above). @mjbvz, you should be able to roll back the as const changes you had to do if you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants