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

Consider only showing builders in object literal completions its contextual type has an index signature #4089

Closed
DanielRosenwasser opened this issue Jul 30, 2015 · 8 comments
Labels
Suggestion An idea for TypeScript Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it

Comments

@DanielRosenwasser
Copy link
Member

Since users are no longer allowed to have excess properties in an object literal, it might be appropriate to only show a builder when the contextual type of an object literal has no index signature.

For instance:

type T = { x: string; y: number; };
function f(param: T) {
}

f({ /*$*/ }); // should only allow 'x' and 'y' and no builder.

However, this may not be sufficient; in the presence of a type assertion, one actually does want to allow a builder, since this is one of the recommended workarounds anyway:

type T = { x: string; y: number; };
function f(param: T) {
}

f(<T>{ /*$*/ }); // should only allow 'x' and 'y' and no builder.
@danquirk danquirk added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jul 30, 2015
@danquirk
Copy link
Member

Seems like a good idea

@DanielRosenwasser
Copy link
Member Author

Though I recall another edge condition is that we would need to allow a builder for the empty type as well.

@JsonFreeman
Copy link
Contributor

I believe you would also want a builder when the contextual type is from a type assertion, because you are allowed to specify extra properties there as well.

You may want to look at isKnownProperty in the checker for inspiration.

@DanielRosenwasser
Copy link
Member Author

@JsonFreeman sorry that's what I originally meant, I wrote "completion" instead of "a builder" in the original issue. Corrected.

@JsonFreeman
Copy link
Contributor

Yes, but I meant that maybe the builder would always be shown in a type assertion, since type assertions do not view their expressions as fresh.

@DanielRosenwasser
Copy link
Member Author

Right, that was what I was getting at. And actually there's another problem - consider the following:

function f(options: FooOptions): void;
function f(options: BarOptions): void;
function f(options: any) { }

Provided that FooOptions and BarOptions are non-empty and distinct, if you supply an empty object literal to f, you are potentially going to only get BarOptions (since resolution would fail on FooOptions and settle on BarOptions).

This would be bad experience. We should either supply properties from both FooOptions and BarOptions or not force completion.

@JsonFreeman
Copy link
Contributor

I think that is a different issue. You'd need to detect whether multiple overloads are involved in obtaining the contextual type, and decide on a policy for this case. I think it may be acceptable to pick one and show a builder. Alternatively, you could try to collect them from all the overloads, but I think that would be complicated, and not that useful. I would also try to have completion and signature help agree on which overload the user is likely trying to call.

@RyanCavanaugh RyanCavanaugh added Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it and removed In Discussion Not yet reached consensus labels Oct 5, 2015
@RyanCavanaugh
Copy link
Member

We'd rather error on the side of not restricting the typing here.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Suggestion An idea for TypeScript Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it
Projects
None yet
Development

No branches or pull requests

4 participants