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

Destructured parameter should not enforce presence / non-nullability from type annotation on a property that's default-initialized #7678

Closed
Arnavion opened this issue Mar 24, 2016 · 8 comments
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@Arnavion
Copy link
Contributor

TypeScript Version:

55cbdc9 (current master)

Code

type Props = { propA: number, propB: string, propC: number };

var p: Props = { propC: 0 }; // Not a Props, missing propA and propB. Error, as expected
var q: Props = { propA: 6, propB: "b", propC: 0 }; // Ok, is a Props
var r = { propA: 6, propB: "b", propC: 0 }; // Not annotated, but is a Props
var s = { propA : 6, propC: 0 }; // Not a Props, missing propB. Not annotated, so not an error
var t = { propC: "c" }; // Not a Props, propC is the wrong type. Not annotated, so not an error

// No type annotation on parameter
function Component({ propA = 5, propB = "a", propC }) {
    console.log(propA, propB, propC); // propC is any, undesirable
}

Component(q); // No error, expected
Component({ propC: 0 }); // No error, expected
Component(r); // No error, expected
Component(s); // No error, expected
Component(t); // Error, expected

// Type annotation on parameter
function Component2({ propA = 5, propB = "a", propC }: Props) {
    console.log(propA, propB, propC); // propC is number, desirable
}

Component2(q); // No error, expected
Component2({ propC: 0 }); // Error, propA and propB are missing
Component2(r); // No error, expected
Component2(s); // Error, propB is missing
Component2(t); // Error, expected
tsc -t es5 ./foo.ts

Expected behavior:

Component2({ propC: 0 }) and Component2(s) compile even though their parameters don't have propA and propB from Props, because Component2 default-initializes those parameters.

Actual behavior:

foo.ts(26,12): error TS2345: Argument of type '{ propC: number; }' is not assignable to parameter of type '{ propA: number; propB: string; propC: number; }'.
  Property 'propA' is missing in type '{ propC: number; }'.
foo.ts(28,12): error TS2345: Argument of type '{ propA: number; propC: number; }' is not assignable to parameter of type '{ propA: number; propB: string; propC: number; }'.
  Property 'propB' is missing in type '{ propA: number; propC: number; }'.

plus errors about p and Component2(t) which are expected.


The reason to add the annotation on Component2 is to get type information for the non-default-initialized property propC.

Is there any downside to allowing Component2({ propC: 0 }) and Component2(s) ? Component2(t) should of course still not be allowed. s and { propC: 0 } should still not be considered to be Props for the purpose of type inference.

Otherwise the user has to make a new type for each such function based on what's default-initialized and what isn't, like type PropsForComponentFunction = { propA?: number, propB?: string, propC: number }, and make sure the types match with the original Props. (Or inline the whole type declaration into Component2 but that has the same drawback.)

@mhegazy
Copy link
Contributor

mhegazy commented Mar 28, 2016

the type inferred from function Component({ propA = 5, propB = "a", propC }) is { propA?: number; propB?: string; propC: any }. obviously this is less strict that Props = { propA: number, propB: string, propC: number }; that require all three properties.

I think what you are asking for is #7355

@mhegazy mhegazy closed this as completed Mar 28, 2016
@Arnavion
Copy link
Contributor Author

@mhegazy No, I'm aware of #7355 and this is not it.

Please reopen this.

@mhegazy mhegazy reopened this Mar 29, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Mar 29, 2016

what is the proposal then?

@Arnavion
Copy link
Contributor Author

If you tell me what is unclear in the OP I can fix it.

Expected behavior:

Component2({ propC: 0 }) and Component2(s) compile even though their parameters don't have propA and propB from Props, because Component2 default-initializes those parameters.

@mhegazy
Copy link
Contributor

mhegazy commented Mar 29, 2016

but component2 has an argument type of Props, which says that they should have this shape. the intialization of the parameters does not participate in the type, because you provided one.

this is similar to:

type RequiredParam = (a) => void;
var func: RequiredParam = (a?) => { }
func(); // Error

@Arnavion
Copy link
Contributor Author

the intialization of the parameters does not participate in the type

And this issue is saying that it should, otherwise the user has to write verbose type aliases for each such use case.

this is similar to:

No, this issue is only about destructured properties being optional, not the entire argument.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Mar 29, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Mar 29, 2016

I will bring this to discussion.

@RyanCavanaugh RyanCavanaugh added Too Complex An issue which adding support for may be too complex for the value it adds and removed In Discussion Not yet reached consensus labels May 9, 2016
@RyanCavanaugh
Copy link
Member

Discussed for a while and decided this was just too confusing / complex (see #8228), with the acknowledgement that this is going to be annoying one way or another (i.e. making a new type or writing a big annotation or etc). Ultimately people (and the compiler) should be able to trust whatever type annotation is written as gospel.

@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 Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

3 participants