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

Test: downloadAndUnzipVSCode could have a constant overload 'insiders' #76091

Closed
bpasero opened this issue Jun 25, 2019 · 7 comments
Closed

Test: downloadAndUnzipVSCode could have a constant overload 'insiders' #76091

bpasero opened this issue Jun 25, 2019 · 7 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug extensions Issues concerning extensions verified Verification succeeded
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Jun 25, 2019

Refs: #76039

export declare function downloadAndUnzipVSCode(version?: string | 'insiders'): Promise<string>;

@bpasero
Copy link
Member Author

bpasero commented Jun 25, 2019

Same for other places where this can be done, e.g. version in runTest options.

@octref octref added the under-discussion Issue is under discussion for relevance, priority, approach label Jun 25, 2019
@octref
Copy link
Contributor

octref commented Jun 25, 2019

Tried this but I wouldn't get any insiders IntelliSense. For what benefit should we do this?

image

@bpasero
Copy link
Member Author

bpasero commented Jun 26, 2019

@mjbvz I would expect a string constant to show up in the intellisense?

@octref yeah if it does not show up as help to the user, it is of not much value.

@mjbvz
Copy link
Collaborator

mjbvz commented Jun 26, 2019

@octref Can you please share the function declaration you are using?

@octref
Copy link
Contributor

octref commented Jun 26, 2019

@mjbvz Simple repro:

function downloadVSCodeArchive(version: string | 'foo') {}

downloadVSCodeArchive('|') // do completion here

I'm on TS 3.5.2.

@mjbvz
Copy link
Collaborator

mjbvz commented Jun 26, 2019

Thanks. TS will reduce the type to the common string type in cases like that for the intellisense details / parameter hints. I believe that is the expected behavior

@octref
Copy link
Contributor

octref commented Jun 26, 2019

@mjbvz Found upstream issue: microsoft/TypeScript#29729

Apparently this works:

type StringLiteralUnion<T extends U, U = string> = T | (U & { });
type DownloadVersion = StringLiteralUnion<'insiders' | 'stable'>;

image

I'll add insiders and stable constants.

@octref octref added extensions Issues concerning extensions polish Cleanup and polish issue bug Issue identified by VS Code Team member as probable bug and removed under-discussion Issue is under discussion for relevance, priority, approach polish Cleanup and polish issue labels Jun 26, 2019
@octref octref added this to the June 2019 milestone Jun 26, 2019
@aeschli aeschli added the verified Verification succeeded label Jun 27, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug extensions Issues concerning extensions verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants