Skip to content

[NewErrors] 5.3.0-dev.20230917 vs 5.2.2 #55768

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

Closed
typescript-bot opened this issue Sep 17, 2023 · 4 comments
Closed

[NewErrors] 5.3.0-dev.20230917 vs 5.2.2 #55768

typescript-bot opened this issue Sep 17, 2023 · 4 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@typescript-bot
Copy link
Collaborator

The following errors were reported by 5.3.0-dev.20230917, but not by 5.2.2
Pipeline that generated this bug
Logs for the pipeline run
File that generated the pipeline

This run considered 200 popular TS repos from GH (after skipping the top 0).

Successfully analyzed 111 of 200 visited repos
Outcome Count
Detected interesting changes 1
Detected no interesting changes 110
Git clone failed 3
Package install failed 33
Project-graph error in old TS 4
Too many errors in old TS 45
Unknown failure 4

Investigation Status

Repo Errors Outcome
microsoft/vscode 2
@typescript-bot
Copy link
Collaborator Author

microsoft/vscode

5 of 54 projects failed to build with the old tsc and were ignored

src/tsconfig.tsec.json

  • error TS2769: No overload matches this call.
  • error TS2322: Type 'IWorkspaceEditDto' is not assignable to type '{ edits: ({ resource: UriComponents; textEdit: { range: { readonly startLineNumber: number; readonly startColumn: number; readonly endLineNumber: number; readonly endColumn: number; }; text: string; eol?: EndOfLineSequence | undefined; insertAsSnippet?: boolean | undefined; }; versionId: number | undefined; metadata...'.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Sep 18, 2023
@RyanCavanaugh
Copy link
Member

@andrewbranch can you see what's going on here?

@andrewbranch
Copy link
Member

I think both of these are correct errors caused by #55638.

In each case, there’s a type produced by this conditional type:

/**
 * Mapped-type that replaces all JSONable-types with their toJSON-result type
 */
export type Dto<T> = T extends { toJSON(): infer U }
	? U
	: T extends VSBuffer // VSBuffer is understood by rpc-logic
	? T
	: T extends CancellationToken // CancellationToken is understood by rpc-logic
	? T
	: T extends Function // functions are dropped during JSON-stringify
	? never
	: T extends object // recurse
	? { [k in keyof T]: Dto<T[k]>; }
	: T;

and a deeply nested property produced by the recursion of the mapped type branch here that legitimately doesn’t match the target. In the overload case, the issue is caused by the recursion on WorkspaceFileEditOptions['contents'], which is a Promise<VSBuffer>. This recurses over the structure of the promise which probably wasn’t intentional:

image

I tried naively unwrapping Promise<VSBuffer> in the conditional type by adding a T extends Promise<infer U extends VSBuffer> ? U : ... branch. This changed the bottom of the assignability error to something that looks somewhat less suspicious:

Types of property 'contents' are incompatible.
                    Type 'VSBuffer | undefined' is not assignable to type '{ type: "base64"; value: string; } | { type: "dataTransferItem"; id: string; } | undefined'.
                      Type 'VSBuffer' is not assignable to type '{ type: "base64"; value: string; } | { type: "dataTransferItem"; id: string; } | undefined'.ts(2322)

The target type mentioned there is spelled out in IWorkspaceFileEditDto:

export type IWorkspaceFileEditDto = Dto<
	Omit<languages.IWorkspaceFileEdit, 'options'> & {
		options?: Omit<languages.WorkspaceFileEditOptions, 'contents'> & { contents?: { type: 'base64'; value: string } | { type: 'dataTransferItem'; id: string } };
	}>;

This type seems to expect the options.contents of the DTO of a IWorksapceFileEdit to be a special representation of a VSBuffer that is not encoded in the Dto type itself. The most direct way to see the change in context:

let t1: IWorkspaceFileEditDto = undefined!;
let s1: Dto<IWorkspaceFileEdit> = undefined!;
t1 = s1;

This assignment use to succeed, I think because contents in the source was down several levels in a recursive mapped type. But you can see that the property is intentionally swapped out in IWorkspaceFileEditDto, so it’s expected that they’re not compatible.

@RyanCavanaugh RyanCavanaugh assigned mjbvz and unassigned andrewbranch Sep 19, 2023
@RyanCavanaugh
Copy link
Member

@mjbvz FYI 👆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

4 participants