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

Poor IDE errors on some types #173

Closed
joeduffy opened this issue Aug 30, 2018 · 9 comments
Closed

Poor IDE errors on some types #173

joeduffy opened this issue Aug 30, 2018 · 9 comments
Assignees
Labels
area/providers impact/usability Something that impacts users' ability to use the product easily and intuitively
Milestone

Comments

@joeduffy
Copy link
Member

joeduffy commented Aug 30, 2018

I was trying to write a blog post to demonstrate some of the benefits of having real code in an IDE: namely, interactive error messages for typos, type errors, and so on. Unfortunately, the first several times I tried, the error messages were in fact not very good. I am using our Kubernetes template project from https://github.com/pulumi/templates/tree/master/kubernetes-typescript, and I've filed this issue here because I suspect the problem might be in the way we've laid out our Kubernetes types.

I am assigning to @CyrusNajmabadi for now because he graciously offered to help track this down.

Example 1

Put a string in the replicas property, when it expects a number:

image

Example 2

Mistype the replicas property altogether:

image

Typings

Here are the types in question for this example:

@joeduffy joeduffy added area/providers impact/usability Something that impacts users' ability to use the product easily and intuitively labels Aug 30, 2018
@joeduffy joeduffy added this to the 0.17 milestone Aug 30, 2018
@CyrusNajmabadi
Copy link
Contributor

@DanielRosenwasser Can you shed some light here? Our understanding was that TS3.0+ would help out here by whittling things down the properties that are the problem. Is there some reason that isn't happening? A config error on our side perhaps? Or is there something about these types that leads to TS feeling like it can't trim this down?

@joeduffy
Copy link
Member Author

I should note that this is using TypeScript 3.0.3 for the project and VS Code is version 1.26.1 at version 3.0.1 of TypeScript (or so I assume, given that this is in the right hand corner):

image

Also worth noting that these types use our pulumi.Input<T> type fairly aggressively, which is a union type, and might be causing some extra troubles:

https://github.com/pulumi/pulumi/blob/1ebd698a7ab2926ef88eb97a8f268e6c95f44816/sdk/nodejs/resource.ts#L487

@DanielRosenwasser
Copy link

Sorry guys: microsoft/TypeScript#25695

Please leave feedback there and feel free to link back here!

@CyrusNajmabadi
Copy link
Contributor

@DanielRosenwasser Actually, we're still seeing an issue even when not using an argument. i.e.:

image

Known issue as well?

@DanielRosenwasser
Copy link

I think the problem there is that we're comparing to a union type which limits our ability to confidently specialize the error. Maybe microsoft/TypeScript#26450 would help there.

@CyrusNajmabadi
Copy link
Contributor

I see. That would likely definitely affect us. We have a union type which likely is the source of the issue (given the message about .apply int eh error. specifically, we call it:

type Input<T> = T | Promise<T> | Output<T>

Meaning: you can pass in a raw T value. Or you can pass something that asynchronously produces a T. Or you provide a 'node' (Output) into our graph, which itself has a value.

With our factories these types are recursively contained within themselves. After all, you could take something that asynchronously produces a value, which itself has properties which are asynchronously produced.

From a representation level, TS really makes it easy for us to describe this system. However, from an errors perspective, it really leaves a lot to be desired :) Def would love to see stuff help out here.

@lukehoban
Copy link
Member

Is there anything we can do to improve here prior to fixes/improvements on the TypeScript side?

My assumption is unfortunately no (short of somehow not using the type Input<T> = T | Promise<T> | Output<T> union - which at this stage is not really an option).

@CyrusNajmabadi
Copy link
Contributor

Moving out as i don't see anything feasible here. We're effectively at the mercy of microsoft/TypeScript#26450.

@CyrusNajmabadi CyrusNajmabadi modified the milestones: 0.17, 0.18 Sep 4, 2018
@joeduffy
Copy link
Member Author

joeduffy commented Sep 4, 2018

Let's just close and as TypeScript improves, we will improve. It seems there's nothing we can really do other than abandoning our union types (which clearly can't happen).

@joeduffy joeduffy closed this as completed Sep 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/providers impact/usability Something that impacts users' ability to use the product easily and intuitively
Projects
None yet
Development

No branches or pull requests

4 participants