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

non-nullable types do not play nice with optional destructuring (ng2) #8681

Closed
opensrcken opened this issue May 19, 2016 · 1 comment
Closed
Assignees
Labels
Bug A bug in TypeScript

Comments

@opensrcken
Copy link

opensrcken commented May 19, 2016

TypeScript Version:

nightly (1.9.0-dev.20160217)

Code

export interface QueryMetadataFactory {
    (selector: Type | string, {descendants, read}?: {
        descendants?: boolean;
        read?: any;
    }): ParameterDecorator;
    new (selector: Type | string, {descendants, read}?: {
        descendants?: boolean;
        read?: any;
    }): QueryMetadata;
}

Expected behavior:
When compiling this with strict null checks, I would expect this to compile, since the destructured variables have been marked as optional, because this works:

export interface QueryMetadataFactory {
    (selector: Type | string, whatever?: {
        descendants?: boolean;
        read?: any;
    }): ParameterDecorator;
    new (selector: Type | string, whatever?: {
        descendants?: boolean;
        read?: any;
    }): QueryMetadata;
}

Actual behavior:
Upon compilation:

node_modules/@angular/core/src/metadata.d.ts(356,32): error TS2459: Type '{ descendants?: boolean | undefined; read?: any; } | undefined' has no property 'descendants' and no string index signature.
node_modules/@angular/core/src/metadata.d.ts(356,45): error TS2459: Type '{ descendants?: boolean | undefined; read?: any; } | undefined' has no property 'read' and no string index signature.
node_modules/@angular/core/src/metadata.d.ts(360,36): error TS2459: Type '{ descendants?: boolean | undefined; read?: any; } | undefined' has no property 'descendants' and no string index signature.
node_modules/@angular/core/src/metadata.d.ts(360,49): error TS2459: Type '{ descendants?: boolean | undefined; read?: any; } | undefined' has no property 'read' and no string index signatu
@ahejlsberg
Copy link
Member

ahejlsberg commented May 19, 2016

This one is borderline. The function

function foo({ a, b }?: { a: number, b: number }) {
}

is actually an error because the argument you're trying to destructure might be undefined. So, implementing the exact signature you have in your example wouldn't be possible. Now, if you change the declaration to

function foo({ a, b }: { a: number, b: number } = { a: 0, b: 0 }) {
}

it is no longer an error because undefined would be replaced with the default value. Yet, from the callers perspective the parameter is still optional and if you now generate a declaration file we spit out the declaration

declare function foo({ a, b }?: { a: number, b: number }): void;

which causes an error. And that is a bug.

As an aside, I'd suggest avoiding destructuring parameters in non-implementation signatures. They're really an implementation detail, i.e. whether the function implementation destructures the argument or just uses property access shouldn't really concern the caller. However, in the declaration file case, because we have no actual parameter name to emit we (reluctantly) do emit the destructuring pattern and so we need to fix the bug.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

2 participants