Is this strategy for TS integration in rollup viable?

[pkozlowski-opensource]

So, I'm playing with this plugin and started to hit various transpilation problems (mostly around classes not ES6-exported properly. So I've started to dig into the source of those pbs and it turns out that by default tsc doesn't allow ES6 module with ES5 target combination of options. If you try to combine those 2 options together you get:

error TS1204: Cannot compile modules into 'es2015' when targeting 'ES5' or lower.

So it seems, on the surface, that tsc is not meant to output ES5 while preserving ES6 import / exports...

I guess that we are not getting this error from rollup since we are calling typescript.transpileModule where the check might be omitted (?)

cc: @robwormald

[Victorystick]

I explain this problem in the README. We're using a prerelease 1.7.0 version of TypeScript in order to use ES6 modules with an ES5 target, but I don't know how stable it is.

[pkozlowski-opensource]

@Victorystick you get the mentioned error with any version of TypeScript (1.7 bundled with this plugin all the way up to the latest released 1.8.0-dev.20151125). So I don't think it is a question of a TS version.

[pkozlowski-opensource]

I can see that there is a unit-test with classes:

class A {
    constructor(...args) {
        this.getArgs = () => args;
    }
}
exports.default = A;

but IMO this just works "by chance". If you try to do:

export class A {
    constructor(...args) {
        this.getArgs = () => args;
    }
}

this won't be transpiled correctly (you won't see export for the A class).

I can send a PR with a test that breaks. Once again, the pb is visible with any TS version (1.7.x, 1.8.x etc.). Unless I'm doing something terribly wrong, of course :-)

[pkozlowski-opensource]

Oh, actually there is confirmation from the TS issue linked from the README that TS won't support this, see microsoft/TypeScript#4806:

It should be an error to choose "es6" as the module emit type when targeting below ES6. When targeting ES6 or above, this can remain the default module emit type.

[pkozlowski-opensource]

@Victorystick here is a PR with a failing test: #10

IMO this test should be passing but TS transpilation fails with:

Error: Module test/sample/export-class/Foo.ts does not export Foo

The reason is that code generated for Foo.ts is "broken" - it doesn't contain any exports.

I'm still hoping that I'm doing sth terribly wrong here and the integration strategy of this plugin can work, but my experiments so far prove otherwise...

[Victorystick]

These two statements don't mean the same thing.

export class Foo { ... }            // named export for Foo
exports.default = class Foo { ... } // default export

If you want to export a single class, write like this instead:

// Foo.ts
export default class Foo { ... }

// and import it like this:
import Foo from './Foo.ts';

Rollup's warning that Foo.ts does not export Foo for your example is valid; TypeScript generates the wrong code.

Running the following in the Console on TypeScript's Playground

ts.version // 1.7.4
ts.transpileModule('export class A {}', {
  compilerOptions: {
    target: ts.ScriptTarget.ES5,
    module: ts.ModuleKind.ES6
  }
})

yields

var A = (function () {
    function A() {
    }
    return A;
})();
A = A;

This output is just blatantly wrong! Why is A = A generated? They should do:

  ...
- A = A;
+ export { A };

If TypeScript is supposed to be a proper superset of JavaScript, this is definitely a feature they will support. I can't find an issue for it in their repo, but I'll make sure to open one and link here. 😉

[pkozlowski-opensource]

These two statements don't mean the same thing.

Definitively. We are talking about default vs. named export.

If you want to export a single class, write like this instead:

I don't think it is about single class or not. This should work, right:

export class Foo { ... }

as well as:

export class Foo { ... }

export class Bar { ... }

Rollup's warning that Foo.ts does not export Foo for your example is valid; TypeScript generates the wrong code.

Exactly. We are in the agreement here :-)

his output is just blatantly wrong! Why is A = A generated? f TypeScript is supposed to be a proper superset of JavaScript, this is definitely a feature they will support.

My fear is that it might not be on their radar to support - at least this is what I got from reading through microsoft/TypeScript#4806.

I can't find an issue for it in their repo, but I'll make sure to open one and link here.

This would be totally awesome!

[Victorystick]

@pkozlowski-opensource Sorry about earlier. I was a bit thick, and didn't understand what you meant. I've hacked away at a fix with #18. Take a look and let me know what you think. 😄

[pkozlowski-opensource]

Wow, thnx @Victorystick - I will give your change a go in the coming day(s). Thnx so much for looking into this one! Hopefully the work-around will be enough (still would be good to have it fixed on the TS side).

[pkozlowski-opensource]

@Victorystick so finally I had time today to test a fix - the good news is that I've managed to have a working rollup bundle for an Angular2 application so your fixes worked!

The less-good news is that the fix of this issue probably introduced another pb: #27

FYI: using rollup allows us to shave off ~5kB from a 110kB bundle (all numbers minified + gzipped). I guess most of the saving (as compared to WebPack bundle) comes from eliminating "module system" overhead. Somehow we are not doing much of tree-shaking as TypeScript is already eliminating unused imports).

Anyway, there are some encouraging results here, thank you for all the work that goes into rollup and its plugins!