Convert to async function refactoring loses generic parameter

Issue Type: Bug

This method

private get<T = any>(url: string, options: GotOptions<any>) {
  return this.authorize('GET', url, options)
      .then(opts => await got.get(url, opts))
      .then<T>(parseJson)
}

gets converted into this

private async get<T = any>(url: string, options: GotOptions<any>) {
  const opts = await this.authorize('GET', url, options)
  const response = await got.get(url, opts)
  return parseJson(response)
  //              ^ look, no <T> !
}

it should keep the explicit parameter on parseJson<T>.

Here’s the parseJson function in case that helps:

const parseJson = <T>(response: Response<string>) => JSON.parse(response.body) as T

VS Code version: Code - Insiders 1.30.0-insider (5fc60ec, 2018-11-14T07:58:01.008Z)
OS version: Darwin x64 18.2.0

System Info

Item	Value
CPUs	Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz (8 x 2800)
GPU Status	2d_canvas: enabled checker_imaging: disabled_off flash_3d: enabled flash_stage3d: enabled flash_stage3d_baseline: enabled gpu_compositing: enabled multiple_raster_threads: enabled_on native_gpu_memory_buffers: enabled rasterization: enabled video_decode: enabled video_encode: enabled webgl: enabled webgl2: enabled
Load (avg)	1, 2, 2
Memory (System)	16.00GB (2.98GB free)
Process Argv	-psn_0_1835456
Screen Reader	no
VM	0%

Extensions (14)

Extension	Author (truncated)	Version
vscode-hie-server	ala	0.0.24
Handlebars	and	0.4.1
bracket-pair-colorizer	Coe	1.0.61
gitlens	eam	8.5.6
prettier-vscode	esb	1.7.2
input-assist	fre	0.0.7
vscode-pull-request-github	Git	0.2.3
language-haskell	jus	2.5.0
dotenv	mik	1.0.1
vsliveshare	ms-	0.3.954
ide-purescript	nwo	0.19.1
language-purescript	nwo	0.2.0
elm	sbr	0.22.0
FilterText	yhi	0.0.11

(1 theme extensions excluded)

typescript@3.2.0-dev.20181114

Simple repo:

async function foo<T>(x: T): Promise<T> {
    return x;
}

function bar<T>(y: T): Promise<T> {
    return foo(y).then<T>(foo)
}

Matt Bierner (@mjbvz)

This is a bug, but the expected behavior described is wrong. To explain why, take this example:

type Response<T> = { success: true, data: T } | { success: false };

function wrapResponse<T>(response: T): Response<T> {
  return { success: true, data: response };
}

function get() {
  return Promise.resolve(undefined!).then<Response<{ email: string }>>(wrapResponse);
}

The return type of get is Promise<Response<{ email: string }>>. If we apply the refactor as suggested:

async function get() {
  const response = await Promise.resolve((undefined!));
  return wrapResponse<Response<{ email: string }>>(response);
}

then the return type becomes Promise<Response<Response<{ email: string }>>. The type argument provided to wrapResponse represents the type of its input, whereas the type argument provided to then represents the type of the fulfilled handler’s output. In the parseJson example and in Matt Bierner (@mjbvz)’s minimal example, those happen to be the same thing, but it’s purely coincidental.

I guess the best thing to do would be to move the type annotation to the return type if it was in a return expression:

async function get(): Promise<Response<{ email: string }>> {
  const response = await Promise.resolve((undefined!));
  return wrapResponse(response);
}

or to a constant, if it was originally a constant:

async function get() {
  const response = await Promise.resolve((undefined!));
  const result: Response<{ email: string }> = await wrapResponse(response);
  // ... other stuff I guess
}

but I’m not sure what to do if the original function declaration or constant already had a type annotation that doesn’t agree... I guess just keep ignoring the type argument like we do now, as the type would be discarded in the end result of the expression anyway.

Ah, I see. I think I was confusing myself by using the same name for both type parameters in my example.

Is there a mechanism to refuse to do the refactoring and present a useful message explaining why? Maybe that would be the best option (or a dialog presenting the user with a choice of which type to use, along with a cancel button)?

There’s not, currently. If the refactor fails it simply isn’t shown as an option. I think annotating either the return type or the constant is reasonably correct here... the most important thing is that the inferred return type of the function doesn’t change because of the refactor, which this approach should ensure. Perhaps, though, we should bail out of the refactor if there’s an existing type error because of a conflict between a then or catch type argument and the contextual type of the expression it’s in, because we’re likely to erase that conflict during the refactor.

❤️

Navigation Menu

Repository navigation

Convert to async function refactoring loses generic parameter #28529

7 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Convert to async function refactoring loses generic parameter #28529

Description

Activity

mjbvz commented on Nov 14, 2018

7 remaining items

andrewbranch commented on Feb 11, 2020

ianp commented on Feb 12, 2020

andrewbranch commented on Feb 12, 2020

ianp commented on Mar 24, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions