Incorrect JS emitted for a simple case of a base/derived class in 1.5-beta

[ddotlic]

I hope this is not a dupe, I was really surprised by this bug because - being so simple in nature - it appears it should have been caught by your tests (or at least stumbled upon by other people), hence there is a slight probability that I misunderstood something or that the compiler isn't capable to do the necessary work (at which point I'd be doubly disappointed).

Anyway, the example is artificial because it's extracted from a larger code base.

Imagine a simple project, let's start with tsconfig.json

{
    "compilerOptions": {
        "target" : "ES5",
        "noImplicitAny": true,
        "out": "badout.js"
    }
}

In the same folder, add a single file c.ts which is just class C {}

Add a subfolder called sub and put the following two files inside:

// file a.ts
class A extends B {
    method1(p1:number, p2:string):void {}
}

// file b.ts
class B {
    method1(p1:number, p2:string):void {}
}

Then simply run tsc in the project folder (the one containing tsconfig.json and c.ts)

The output will be:

var C = (function () {
    function C() {
    }
    return C;
})();
var __extends = this.__extends || function (d, b) {
    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
    function __() { this.constructor = d; }
    __.prototype = b.prototype;
    d.prototype = new __();
};
var A = (function (_super) {
    __extends(A, _super);
    function A() {
        _super.apply(this, arguments);
    }
    A.prototype.method1 = function (p1, p2) { };
    return A;
})(B);
var B = (function () {
    function B() {
    }
    B.prototype.method1 = function (p1, p2) { };
    return B;
})();

The problem is obvious - we're trying to use B before it is defined.

If this has been noted and fixed, please forgive the noise.

If not, please try to look into fixing this before 1.5 is "done".

[ddotlic]

This cannot be worked around, but will work if I turn the base class into an interface and thus am required to implement all its methods in all of derived classes.
In my real project, I have mulitple methods in the base class/interface and multiple derivatives, so it's very annoying to "work around" this problem.

[ddotlic]

Also, this (not visible in the example project) is also a perfect example of the place where abstract classes could be useful, so once more 👍 for abstract classes (I know this should be coming in 2.0, but if it happens before, I won't be cross 😉 )

[kitsonk]

I maybe wrong, but as it stands at the moment, your a.ts is not valid TypeScript. You would have to reference b.ts in your a.ts for it to be valid and then the compiler would correctly resolve the ordering of the modules in the output file. The compiler should be complaining though that as a standalone, your a.ts is not valid (as class B does not exist).

[ddotlic]

@kitsonk The point of tsconfig.json is exactly to avoid this kind of ceremony. I have around 45 .ts files in my actual project and not a single ///<reference in any of the files, things work like a charm.
The issue clearly is that the compiler is unable to figure out the order of dependencies in this particular case which I conclude must then be a bug.

Unless I misunderstood the extent to which tsconfig.json is/can be used...

I find it very convenient not to have to ///<reference other files, it's just noise.

[kitsonk]

The point of tsconfig.json is exactly to avoid this kind of ceremony.

But you don't have a filesGlob or files in your tsconfig.json in the example you provided. Was that intentional?

[ddotlic]

@kitsonk Yes, of course.

First, filesGlob is not officially supported (it's accepted extension used by Atom plugin team for example, but it is not used by the compiler).

Second, if you put files you're obliged to then list all the files you're considering a part of your project. Again, ceremony that one wants to avoid if possible and indeed, it's not necessary - as you can see from the compiler output, it figured out the files that it should use to produce the final result, but it did not figure out the order of emitted code (it does do that correctly for every other file in the 45 files I have in my actual project).

[tinganho]

@ddotlic I think you must at least use /// <reference ... or import .... statements. Even though you think your refer to the same class. The compiler don't actually think you refer at all. The compiler doesn't know and just concatenate the files.

[ddotlic]

@tinganho I do not, my 45 files prove that I don't 😄

Could someone from compiler team please comment on this?

[NoelAbrahams]

Possible duplicate of #21.

[ddotlic]

@NoelAbrahams Thanks for pointing this out, I found a treasure trove of linked items from that one item.

It appears that this is a known problem for which there is no fix at the moment. I see several items linked which partially aid in similar cases, but the issue still exists.

I can confirm that manually adding ///<reference to the file a.ts will actually work around the problem (confused whether I should put a smiley or sad smiley here).

Why would this still be a problem - compiler clearly sees all files, it's a question of emit order but this does not seem a too difficult thing to do (admittedly I'm oversimplifying)? Wasn't tsconfig.json in mode where there's no files made precisely to avoid ///<reference ceremony??

[NoelAbrahams]

#2181 is a proposed solution for Visual Studio that I would like to see. It would do the following as suggested by @danquirk:

Solution Explorer should probably just reflect the correct file order and support re-ordering files rather than an alphabetical ordering (this is how F# handles the same problem).

For non-VS compilation, I believe the solution is to list the items in the required order in .tsconfig.

[ddotlic]

For non-VS compilation, I believe the solution is to list the items in the required order in .tsconfig

I did not want to come to this conclusion as I've said many times in this thread (I like not having to list files in files), but if this is the approach we have to live with then I guess that's what I'll do (annoys me less than putting ///<reference comments "everywhere")

Can someone from compiler team please confirm this? Thanks @NoelAbrahams for additional info.

[tinganho]

Why would this still be a problem - compiler clearly sees all files, it's a question of emit order but this does not seem a too difficult thing to do (admittedly I'm oversimplifying)? Wasn't tsconfig.json in mode where there's no files made precisely to avoid ///<reference ceremony??

The compiler doesn't do cross file binding automatically — imagine you having several classes named the same. How would the compiler know which you mean?

[ddotlic]

The compiler doesn't do cross file binding automatically — imagine you having several classes named the same. How would the compiler know which you mean?

Fair point. This is not necessarily TS issue as much as it is JS issue.

It's also unexpected because the compiler appears to see the classes/files fine, just not order them in the emit phase.

[RyanCavanaugh]

We can't determine the "right" order to run your code.

For example, you could have some code that must run before class initialization, or some code that must run after class initialization. We have no idea what the affinity of any given piece of code to another is.

/* File A */
let x = B;
class A {
  static y = x;
}

/* File B */
class B extends A { }

Clearly the correct order is to put File B before File A, because let x depends on B. And clearly the correct order is to put File A before File B, because B depends on A. Or clearly the correct order is to move let x to after class B, but now we've introduced a runtime error because x is in the TDZ for A's static initialization? Or maybe there's another order that might be correct? Who knows.

Now, you might say, that is a contrived example and the compiler should work in the "simple" cases. But this is worse than not trying to order at all, because if you depend on re-ordering for the "simple" cases, then enter the "complex" case (which is going to be a very subtle line of transition), then your code is going to break in a way that seems completely random.

The proper fixes have been noted above and in linked threads: Use external modules, use reference comments on a per-file basis, have a single file with a list of reference comments that define the overall file ordering, or list the files in dependency order in tsconfig.json.

[ddotlic]

The proper fixes have been noted above and in linked threads: Use external modules, use reference comments on a per-file basis, have a single file with a list of reference comments that define the overall file ordering, or list the files in dependency order in tsconfig.json.

While I don't personally like none of the proposed solutions, I can see why TypeScript cannot do much better than it already does. Some things in JavaScript are clearly unavoidable.

Thanks everyone for an interesting discussion.