initial revision of external module augmentations #6213

vladima · 2015-12-23T00:23:33Z

Module augmentation is a declaration of ambient module that directly nested either in external module or in top level ambient external module.
Name of module augmentation is resolved using the same set of rules as module specifiers in import \ export declarations.
If name is successfully resolved to some external module then declarations in module augmentation are merged with declarations inside module using standard rules for declaration merging.
Module augmentations cannot add new items to the top level scope but rather patch existing declarations.

for example

// observable.ts
export class Observable<T> {}

// map.ts
import {Observable} from "./observable";
Observable.prototype.map = function () { /* ... */ }

declare module "./observable" {
    interface Observable<T> {
        map<U>(proj: (el: T) => U): Observable<U>;
    }
}

// consumer.ts
import {Observable} from "./observable";
import "./map";
let o: Observable<number>;
o.map(x => x.toFixed());

Here module map can declare that internally it will patch Observable type and add map function to it.

Fixes: #5269, #6022

Pending work:

- add syntactic support for augmenting global scope. Currently augmentation for the global scope is defined as ambient module with special name "/" but this is just a placeholder. The version that we ended up during the design meeting was:

// in external module
export {};
declare global {}

or

// in script file
declare module "array" {
    global {}
}

DanielRosenwasser · 2015-12-23T09:04:55Z

src/compiler/binder.ts

@@ -109,6 +109,7 @@ namespace ts {
        let blockScopeContainer: Node;
        let lastContainer: Node;
        let seenThisKeyword: boolean;
+        let isSourceFileExternalModule: boolean;


I don't see this getting used anywhere apart from the later assignment.

right, forgot to remove that. Thanks

DanielRosenwasser · 2015-12-23T22:53:49Z

src/compiler/checker.ts

+                        //   error should already be reported so all errors in the body of augmentation can be ignored.
+                        const checkBody = isNameOfGlobalAugmentation(<LiteralExpression>node.name) || (getSymbolOfNode(node).flags & SymbolFlags.Merged);
+                        if (checkBody) {
+                            const globalAugmentation = isNameOfGlobalAugmentation(<LiteralExpression>node.name);


isGlobalAugmentation or augmentsGlobal

DanielRosenwasser · 2015-12-23T23:55:26Z

What exactly is the recommended way to augment a default export? Is it possible in this implementation?

vladima · 2015-12-23T23:56:51Z

it is not possible

DanielRosenwasser · 2015-12-29T16:17:19Z

This also appears to take care of #2784. You might want to take note and give a heads up on that issue when this goes in.

mhegazy · 2016-01-13T18:51:44Z

src/compiler/declarationEmitter.ts

@@ -139,6 +141,14 @@ namespace ts {
                allSourcesModuleElementDeclarationEmitInfo = allSourcesModuleElementDeclarationEmitInfo.concat(moduleElementDeclarationEmitInfo);
                moduleElementDeclarationEmitInfo = [];
            }
+
+            if (!isBundledEmit && isExternalModule(sourceFile) && sourceFile.moduleAugmentations.length && !resultHasExternalModuleIndicator) {


what about external modules with no exports or no module augmentation? soemthing like:

import {a} from "./mod"; // do something with a

i thought we will do this for any external module, that we never emitted any import, export, or module augmentation for.

mhegazy · 2016-01-13T22:12:26Z

👍

ahejlsberg · 2016-01-14T17:52:02Z

src/compiler/checker.ts

                    }
                }
            }
            checkSourceElement(node.body);
        }

+        function checkBodyOfModuleAugmentation(node: Node, isGlobalAugmentation: boolean): void {


This should be checkModuleAugmentationElement. It checks and element in the body, not the body itself.

vladima · 2016-01-14T19:13:28Z

build failure is related to #6478

vladima · 2016-01-14T20:05:44Z

@ahejlsberg done, do you have any other comments?

ahejlsberg · 2016-01-14T22:12:04Z

src/compiler/checker.ts

+                        if (!reportError) {
+                            if (isGlobalAugmentation) {
+                                // global symbol should not have parent since it is not explicitly exported
+                                reportError = symbol.parent !== undefined;


The error message below seems a bit odd for this case, but maybe it's fine.

for global case we can easily lift this restriction since it is possible to add new entries to global scope from within a module. Currently the fact that we have it is only because of consistency in behavior for augmentations.

No sure I understand, maybe we're talking about different issues. This code is saying that you get an error when you have an exported member in a global augmentation block, right? I'm thinking it should never be valid to use export in a global augmentation block.

the idea of this check is following: symbol.parent will be undefined if symbol was initially defined in the global scope. If symbol.parent is not undefined this means that this symbol was declared inside augmentation and will declare new entry in the global scope which is disallowed in current design

If symbol.parent is not undefined this means that this symbol was declared inside augmentation

Why is that so? symbol.parent is non-undefined when the symbol is exported, but it might still be declared inside the augmentation. I'm not getting this.

symbols whose initial declaration is in global scope are never exported and symbol.parent for them is always undefined. if symbol.parent !== undefined this means that this symbol is exported from somewhere and as a consequence initially defined in augmentation not in global scope

masaeedu · 2016-01-14T22:23:36Z

In map.ts: since the ambient module declaration advertises to the compiler that Observable<T> has a map member, would you be able to attach a function of the appropriate kind to the prototype without casting to <any>?

ahejlsberg · 2016-01-14T22:31:19Z

src/compiler/checker.ts

+                            else {
+                                // this symbol contains only merged content from external modules and augmentations so it should always be exported (parent !== undefined)
+                                // and parent should have value side (valueDeclaration !== undefined)
+                                Debug.assert(symbol.parent !== undefined && symbol.parent.valueDeclaration !== undefined);


I have a hard time reasoning about this.

this assert is redundant. it basically says: augmentation can only change exported symbol (parent !== undefeind) that really has a declaration (not a virtual symbol like prototype) - this should always be the case. I'll remove it

So did you mean symbol.valueDeclaration !== undefined? You currently have symbol.parent.valueDeclaration.

symbol.parent.valueDeclaration - is an augmentation: declare module .... symbol.valueDeclaration is declaration inside the augmentation. Currently we report error if container that this symbol is module augmentation and not a normal external module

initial revision of external module augmentations

benlesh · 2016-01-16T09:55:43Z

Awesome! Great stuff. Thank you!

DanielRosenwasser · 2016-01-16T23:48:41Z

🎉 👏 👏 👏 👏 👏 👏 👏

weswigham · 2016-01-18T03:11:31Z

👍

initial revision of external module augmentations

4a963a2

msftclas added the cla-not-required label Dec 23, 2015

DanielRosenwasser reviewed Dec 23, 2015
View reviewed changes

address PR feedback

3e1bc01

DanielRosenwasser reviewed Dec 23, 2015
View reviewed changes

use 'declare global' to define augmentations for the global scope

7f2ebf9

DanielRosenwasser mentioned this pull request Dec 29, 2015

Reopening external modules broken in TS 1.5-alpha #2784

Closed

vladima added 2 commits January 11, 2016 22:12

merge with origin/master

7cef1ef

fix linter issues

0e1c6e3

mhegazy reviewed Jan 13, 2016
View reviewed changes

address PR feedback

1e11a55

ahejlsberg reviewed Jan 14, 2016
View reviewed changes

addressed PR feedback

3aa92f5

Merge remote-tracking branch 'origin/master' into moduleAugmentations

a69dcd1

merge with origin/master

e90db46

ahejlsberg reviewed Jan 14, 2016
View reviewed changes

kwonoj mentioned this pull request Jan 14, 2016

Cannot opt-in import operator's Type Definitions from 'rxjs/add/operator/***' ReactiveX/rxjs#1193

Closed

This was referenced Jan 14, 2016

Merged Declarations for Classes and Interfaces #3333

Merged

Need to support declaration merging in external modules #2821

Closed

address PR feedback

36c489c

vladima added a commit that referenced this pull request Jan 16, 2016

Merge pull request #6213 from Microsoft/moduleAugmentations

c6b0cf1

initial revision of external module augmentations

vladima merged commit c6b0cf1 into master Jan 16, 2016

vladima deleted the moduleAugmentations branch January 16, 2016 05:28

vladima mentioned this pull request Jan 16, 2016

External modules that don't export anything emit empty declarations. #6022

Closed

vladima mentioned this pull request Jan 22, 2016

Provide a way to augment global interfaces from external modules #4166

Closed

This was referenced Jan 29, 2016

Merged external modules break protected member access checking #6731

Closed

Re-exports of external modules with side effects are erased from ES5 targets #6835

Closed

aikoven mentioned this pull request Mar 31, 2016

Add state type to dispatch interface reduxjs/redux#1537

Merged

jkillian mentioned this pull request Apr 8, 2016

false negative: no-internal-module palantir/tslint#1069

Closed

ktsn mentioned this pull request Aug 24, 2016

Add typescript definition vuejs/vue#3509

Closed

4 tasks

david-driscoll mentioned this pull request Jan 4, 2017

Partial classes #563

Closed

microsoft locked and limited conversation to collaborators Jun 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

initial revision of external module augmentations #6213

initial revision of external module augmentations #6213

vladima commented Dec 23, 2015

DanielRosenwasser Dec 23, 2015

vladima Dec 23, 2015

DanielRosenwasser Dec 23, 2015

DanielRosenwasser commented Dec 23, 2015

vladima commented Dec 23, 2015

DanielRosenwasser commented Dec 29, 2015

mhegazy Jan 13, 2016

mhegazy Jan 13, 2016

mhegazy commented Jan 13, 2016

ahejlsberg Jan 14, 2016

vladima Jan 14, 2016

vladima commented Jan 14, 2016

vladima commented Jan 14, 2016

ahejlsberg Jan 14, 2016

vladima Jan 14, 2016

ahejlsberg Jan 14, 2016

vladima Jan 14, 2016

ahejlsberg Jan 14, 2016

vladima Jan 15, 2016

masaeedu commented Jan 14, 2016

ahejlsberg Jan 14, 2016

vladima Jan 14, 2016

ahejlsberg Jan 14, 2016

vladima Jan 14, 2016

benlesh commented Jan 16, 2016

DanielRosenwasser commented Jan 16, 2016

weswigham commented Jan 18, 2016

initial revision of external module augmentations #6213

initial revision of external module augmentations #6213

Conversation

vladima commented Dec 23, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielRosenwasser commented Dec 23, 2015

vladima commented Dec 23, 2015

DanielRosenwasser commented Dec 29, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhegazy commented Jan 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladima commented Jan 14, 2016

vladima commented Jan 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masaeedu commented Jan 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benlesh commented Jan 16, 2016

DanielRosenwasser commented Jan 16, 2016

weswigham commented Jan 18, 2016