32

Closed

Inheritance only works if parent class is declared before child class.

description

Hi. First of all, I'd like to thank all of you for the excellent work on TypeScript. I'm really excited about it.

Today, while doing some experiments, I encountered a problem with the output generated by tsc for this fragment of source code:
class A extends B {
  // ...
}

class B {
  // ...
}
As expected, the compiler finished execution normally, and produced the following output:
01  var __extends = this.__extends || function (d, b) {
02      function __() { this.constructor = d; }
03      __.prototype = b.prototype;
04      d.prototype = new __();
05  };
06  var A = (function (_super) {
07      __extends(A, _super);
08      function A() {
09          _super.apply(this, arguments);
10  
11      }
12      return A;
13  })(B);
14  var B = (function () {
15      function B() { }
16      return B;
17  })();
When I executed the code, I got the following error, on line 3:
Uncaught TypeError: Cannot read property 'prototype' of undefined
It seems that the current __extends implementation only works when the parent class is already initialized by the time the child class is initialized. This is of course a very minor issue for two classes in the same source file (as you can switch them around) and doesn't happen at all when using external modules (since require(...) will ensure dependencies are initialized on the right order) however, for internal modules defined with the module keyword across multiple files this can be a real issue, as in some circunstances you can't / don't want to specify the order the files are loaded/concatenated.

I originally found out this bug when compiling multiple .ts files with the tsc *.ts --out build.js command.

Edit: After fiddling a bit with the generated JS code, I believe I have a possible solution for this issue. It uses a similar approach to the one internal modules are currently using. The following code allows for inheritance regardless of the order the classes are declared in the file:
var A = (function (_placeholder, _super) {
    __init(_placeholder, A, _super, function (__super) {
        __extends(A, _super = __super);
    })
    function A() {
        _super.apply(this, arguments);
    }
    return A;
})(A, B || (B = { __pending: [] }));
It works by allowing the initialization of the child class to occur on a later time, after the parent class has been initialized. It requires, however the presence of the following helper function in the generated code:
var __init = this.__init || function(_placeholder, _class, _super, _callback) {
    function __() {
        for (var i = 0; i < _class.__pending.length; i++) {
            _class.__pending[i](_class);
        }
        delete _class.__pending;
    }
    _class.__pending = _placeholder ? _placeholder.__pending : [];
    if (_super && _super.__pending) {
        _super.__pending.push(function(s) {
            _callback(s);
            __();
        });
    } else {
        _callback(_super);
        __();
    }
}
I'm not sure what is the policy for adding helper functions like this, but since we already have __extends in place, it might be OK to add another one.

PS: I'm new to CodePlex and was not aware comments would appear in reverse chronological order. I edited the original issue now, so it reads in the natural order. Sorry about that.
Closed Jul 28 at 11:17 PM by jonturner
As part of our move to GitHub, we're closing our CodePlex suggestions and asking that people move them to the GitHub issue tracker for further discussion. Some feature requests may already be active on GitHub, so please make sure to look for an existing issue before filing a new one.

You can find our GitHub issue tracker here:
https://github.com/microsoft/typeScript/issues

comments

coreh wrote Jan 21, 2013 at 4:32 AM

After fiddling a bit with the generated JS code, I believe I have a possible solution for this issue. It uses a similar approach to the one internal modules are currently using. The following code allows for inheritance regardless of the order the classes are declared in the file:
var A = (function (_placeholder, _super) {
    __init(_placeholder, A, _super, function (__super) {
        __extends(A, _super = __super);
    })
    function A() {
        _super.apply(this, arguments);
    }
    return A;
})(A, B || (B = { __pending: [] }));

coreh wrote Jan 21, 2013 at 4:36 AM

It works by allowing the initialization of the child class to occur on a later time, after the parent class has been initialized. It requires, however the presence of the following helper function in the generated code:
var __init = this.__init || function(_placeholder, _class, _super, _callback) {
    function __() {
        for (var i = 0; i < _class.__pending.length; i++) {
            _class.__pending[i](_class);
        }
        delete _class.__pending;
    }
    _class.__pending = _placeholder ? _placeholder.__pending : [];
    if (_super && _super.__pending) {
        _super.__pending.push(function(s) {
            _callback(s);
            __();
        });
    } else {
        _callback(_super);
        __();
    }
}
I'm not sure what is the policy for adding helper functions like this, but since we already have __extends in place, it might be OK to add this one.

RyanCavanaugh wrote Jan 21, 2013 at 10:03 PM

Assigning to Luke to weigh in on this one.

oisin wrote Jan 25, 2013 at 6:16 PM

I think I'd rather have the compiler reorder dependent classes rather than complicate the generated output script. Or, at the very least emit a warning saying that the class dependencies are defined in an order that may cause problems at runtime (depending on the JS engine - browser, node, cscript etc.)

Typescript already reorders fields to the top of the class.

coreh wrote Jan 25, 2013 at 9:26 PM

Hmm, when concatenating multiple .ts files in a single .js output this approach is a very good fit.

When multiple .js files are being generated by the compiler, perhaps it could also generate an additional loader file that automatically loaded the files in the right order? Like an index.js file that dynamically added <script> tags.

jonturner wrote Mar 8, 2013 at 9:03 PM

I definitely agree that having to think about order feels less declarative (and exposes the JavaScript side). In some sense, this is intentional because of how lightweight TypeScript is intended to be. That said, if we assume your solution, can we work with a slightly different example:
class A extends B {
  // ...
}
var a = new A();  // need to see B's constructor here when doing super() call
class B {
  // ...
}
Unless we reorder the code for you, I think you still can come up with situations where a value hasn't been fully initialized. In the example above, we need to know the full knowledge of B before it's known because we're call into its constructor. At this time, we're explicitly not reordering code (or even adding code except in a couple of exceptions) that you've written.

duncan16 wrote Mar 29, 2013 at 2:29 AM

Can the compiler verify the order at least, and throw an error if it's not existing in the right area of the file? That would be a sufficient enough syntactical sugar to meet the needs of the C# guys that are looking for a bit more structure, as well as the ECMAScript guys that are used to this sort of thing, I'd think... that is... if doing this wouldn't be too much trouble.

Thanks guys! This is a wonderful product so far.

duncan16 wrote Mar 29, 2013 at 2:30 AM

sorry Oisin.. didn't really read through all the comments.. just wanted to put my $0.02 on the subject.

mattmelton wrote Apr 23, 2013 at 9:48 AM

Perhaps the significance of this feature/bug is underestimated.

In my app, which uses inheritance, the release html uses a single js and the debug uses a finely ordered set of script tags. Because of this reordering problem I'm unable to create a single application.js for the release build, directly from the individual modules I use in my debug build, without some post processing tool to extract the *.js from from the debug build and concatenate appropriately.

It's difficult for developers to track these kind of file by file orderings in 30+ ts files, not including the 'merge fun' it creates with a GIT/svn repository ("Oh that's just a script tag, it not my concern"). And it gets worst: different JIT compilers do things in awkward and often unpredictable ways. Recently I ran into a problem with the ordering of a debug build's script tags: on the desktop (IE, Firefox and Chrome), plus UIWebView on iOS, everything launched correctly. When I used the iOS version of Adobe's Folio Content Viewer, things were broken - out of order - until I placed a script defining an object ahead of a function using it. I'd like to think that was a difficult bug I could have been avoided with a single script release build, automatically ordered.

I guess what I'm trying to get at is that is that it's difficult for developers to continually reorder things, and in my case, wait until I find a bug in what I'd already consider a tested order. It's more reliable, from my developers' point of view, for the compiler to give me the correct order via --out or otherwise.

Hope this helps.

mistaecko wrote Apr 26, 2013 at 7:50 AM

Same problem domain as item #599

jonturner wrote Oct 24, 2013 at 6:32 PM

@mattmelton - we're definitely thinking about how to handle ordering issues like this, but really it comes down to how lightweight TypeScript is intended to be. In JavaScript, you still have to think about the ordering of how you initialize your classes and modules and statics (or their equivalent encoding in JavaScript). It doesn't handle that for you.

I agree that this is a bit frustrating to be so manual, but that a bit of the trade-off by trying to keep the output code very close to what you originally wrote.

That said, I think it'd definitely make sense, as much as possible, to warn the user when we can detect that the ordering will be incorrect. This would work for single files, but becomes more complex (and even impossible) in other cases of multiple files and multiples build stages.

Martin_Andersson wrote Jan 6 at 3:41 PM

Erm give just a "warning"? This is a serious error of the compiler. I think that the TypeScript compiler should compile into one file properly, or don't do that at all. Why just go half-way? And how can a correct ordering feature make TypeScript heavy weight? On the contrary, TypeScript is supposed to help facilitate large software projects. Indeed that is the sole reason for the existence of TypeScript! Otherwise we could all still be struggling with spaghetti JavaScript. Making compiled "executable files" actually "execute" should be a top priority.

Anyways, super big thank you for all your work! You make the developer's everyday life so much easier. Hopefully, TypeScript shall not halt on his glorious way but should continue to deliver.

jamesnw wrote Apr 4 at 4:37 PM

I've ran into this a few times as well. I always have to keep definition order in mind. Note, however, that inline functions such as "(function (){ ... })()" are supported in modules, and reordering classes can break this support if an expected class suddenly ends up farther down. Just something to keep in mind.

sapix wrote Apr 19 at 3:42 PM

I find this is a very serious issue, because the single file output feature is basically unusable for larger projects. But isn't it one of the core claims of typescript that it eases the pain of working in larger projects?

Is there any workaround for this issue until the compiler helps?

GustavS wrote Apr 24 at 12:34 PM

I don't understand why this isn't considered a more serious issue..

I have a base class and sub classes in the same namespace (module) but in different ts-files and this error occurred.

The only way I managed to not get this error was to put the base class and sub-classes in the same file, which I don't really like. We can manage right now because there will not be that many sub-classes, but still.

Am I doing something wrong?

madmaw wrote May 14 at 2:59 AM

It isn't a great solution, but in the past I've worked around it by having a "reference" comment at the top of the file. This gives the compiler enough of a hint to order the output correctly
///<reference path="A.ts"/>
export class B extends A {

}
Please fix this guys, it's probably the biggest source of headaches for me as a user of TypeScript.

madmaw wrote May 14 at 4:07 AM

If you use Grunt, there's a grunt-ts task that will automatically produce a reference.ts file with the references in the correct order for you as part of the build.

https://github.com/grunt-ts/grunt-ts#javascript-generation-and-ordering

madmaw wrote May 14 at 4:28 AM

Awesome, no way to edit comments. Turns out grunt-ts only produces a listing of all the TS files, it's up to you to order them yourself.

jamesnw wrote May 14 at 5:53 AM

Hi, welcome to CodePlex, where anything you say shall forever be immortalized. ;)

SaschaNaz wrote Jun 24 at 8:17 AM

This issue still exists and I'd like to see any error message rather than a broken code, if we need more time to fix this fundamentally.