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

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

Closed
masaeedu opened this issue Feb 2, 2016 · 6 comments · Fixed by #6858
Closed

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

masaeedu opened this issue Feb 2, 2016 · 6 comments · Fixed by #6858
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead

Comments

@masaeedu
Copy link
Contributor

masaeedu commented Feb 2, 2016

Re-exports of external modules with no public members are erased from the generated JS for ES5 targets. This causes problems in scenarios where the module is used to extend some class by patching the prototype (see #6213 and #6022).

The ultimate goal is to be able to do the following:

// a.ts
export class A { }
// b.ts
import {A} from './a';
A.prototype.foo = function () { };
declare module './a' {
  interface A { foo(): void; }
}
// c.ts
import {A} from './a';
A.prototype.bar = function () { };
declare module './a' {
  interface A { bar(): void; }
}
// all.ts
export {A} from './a';
export * from './b';
export * from './c';

Once this is built and distributed as a set of JS and definition files, the user should be able to do:

import {A} from 'somemodule/a';
import 'somemodule/all';

var a = new A();
a.foo();
a.bar();

Currently, this compiles without errors, but produces the following output for all.js:

"use strict";
var a_1 = require('./a');
exports.A = a_1.A;

You can see that ./b and ./c are not imported, which results in the prototype patches for A being unavailable at runtime. The user will therefore get the following exception:

TypeError: a.foo is not a function

Hopefully I'm not misusing the feature here.

@ahejlsberg
Copy link
Member

The compiler omits the export * for ./b and ./c because it has concluded they don't export any values (this is so we don't emit export * for modules that contain only types, for example).

You can ensure that a side-effecting module with no exports is imported with a simple import 'xxx';. So, this should work:

// all.ts
import './b';
import './c';
export {A} from './a';

@ahejlsberg ahejlsberg added the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Feb 2, 2016
@masaeedu
Copy link
Contributor Author

masaeedu commented Feb 2, 2016

@ahejlsberg Should importing the side effecting module export its declared ambient modules to consumers? In other words, should the augmentations to a.ts in b.ts and c.ts be available to consumers of all.ts if it looks like this?

// all.ts
import './b';
import './c';
export {A} from './a';

Right now my workaround is to do this:

// all.ts
import './b'; export * from './b';
import './c'; export * from './c';
export {A} from './a';

Is there a nicer way to do this?

@ahejlsberg
Copy link
Member

It works just with this:

// all.ts
import './b';
import './c';
export {A} from './a';

The augmentations don't need to be exported, they behave as if they were written on A itself. All that matters is that ./b and ./c are directly or indirectly imported in your program. Once they are, the augmentations appear on A anywhere you can access A.

@masaeedu
Copy link
Contributor Author

masaeedu commented Feb 2, 2016

@ahejlsberg Unfortunately this is not the behavior I'm seeing. When writing all.ts as you have it, and all other files as in my first comment, the generated all.d.ts looks like this:

export { A } from './a';

As you can see, no trace of the augmentations or the modules they occur in has remained, so consumers of these typings will not be aware of a.foo or a.bar.

@vladima
Copy link
Contributor

vladima commented Feb 2, 2016

that is true, we won't elide import "..." from implementation files but such imports won't be recorded in .d.ts files

@mhegazy
Copy link
Contributor

mhegazy commented Feb 2, 2016

We should emit import "mod"; in declaration files then. In the past, modules could not have a side effects, so it was safe to elide the imports from declaration files. Now with module augmentations, modules has side effects, so we should preserve these in the declaration files.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants