Merged Declarations for Classes and Interfaces

[aozgaa]

This implements the proposal in #3332.

[msftclas]

Hi @aozgaa, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

It looks like you're a Microsoft contributor (Arthur Ozga). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

[DanielRosenwasser]

This isn't exported, so you don't need this.

[aozgaa]

Changed.

[DanielRosenwasser]

Nit: Can you put single quotes around node just so this reads easier? Thanks!

[aozgaa]

Changed.

[JsonFreeman]

If you do the other change I suggested (https://github.com/Microsoft/TypeScript/pull/3333/files#r31552915), then this goes away, and ClassExcludes becomes (Value | Type) & ~(ValueModule | Interface)

[mhegazy]

why not just letting classes and interfaces merge, and reporting an error later on in CheckClassDeclaration for instance if the class is not ambient? just like we do with merging order for module/class and module/class being in different files.

[mhegazy]

should not this be inAmbientContext instead?

[JsonFreeman]

👍

[aozgaa]

Thoughs @mhegazy, @ahejlsberg ?

[DanielRosenwasser]

Remove the leading space if you get the chance.

[aozgaa]

fixed.

[DanielRosenwasser]

There are apparently some issues with the build - you probably need to fix up baselines that don't agree with the merge.

[aozgaa]

@DanielRosenwasser The error codes needed to be changed as part of merging the upstream fixes and this broke some of the baselines. No other errors occurred. They've been added now :).

[mhegazy]

👍

[DanielRosenwasser]

👍

[DanielRosenwasser]

This should be added to the Roadmap and What's New In TypeScript page.

[heycalmdown]

How does this work with imported module?

When I have like this application.ts

import * as amqp from 'amqplib';

// I want to merge declaration with `interface amqp.ExchangeOptions`
interface ExchangeOptions {
  'x-recent-history-length' : number;
}

interface amqp.ExchangeOptions {
  'x-recent-history-length' : number;
}

Which one should work?

[JsonFreeman]

Neither one of those would work here. You'd need to do the following:

declare module 'amqplib' {
    interface ExchangeOptions {
       'x-recent-history-length' : number;
    }
}

[heycalmdown]

Thanks!

[masaeedu]

@JsonFreeman Is there an equivalent for external modules?

[mhegazy]

@masaeedu please see #6213

[nbransby]

This doesn't appear to be possible but feels like it should:

interface Foo {
    readonly name: string
}

class Foo {
    constructor(name: string) {
        this.name = name; //error
    }    
}

[DanielRosenwasser]

@nbransby thanks, I'll open an issue for you.