Code Monkey home page Code Monkey logo

Comments (29)

BrainCrumbz avatar BrainCrumbz commented on March 28, 2024 8

Just to mention that we've just being bitten by this today, and it took us some time to figure out what was going on.

TypeScript v1.8.10, Webpack-based build, both base and derived class defined in same file, but (apparently) in the wrong order, no compile errors nor warnings, and even if source maps are working, the error call stack was pointing to a highly unuseful location (the end of another class importing the derived one).

Not going through the whole discussion, but as a first aid it seems like a compiler warning would help. Just our 2ยข

from typescript.

RyanCavanaugh avatar RyanCavanaugh commented on March 28, 2024 1

If your classes simply refer to each other in the type system or in instance methods, that's no problem. The only "mutually recursive" pattern that's a problem is this one:

class Alpha {
    static myFriendBeta = new Beta();   
}

class Beta {
    static myFriendAlpha = new Alpha(); 
}

You can rewrite this as a clodule:

class Alpha {
}

class Beta {
    static myFriendAlpha = new Alpha();
}

module Alpha {
    export var myFriendBeta = new Beta();
}

from typescript.

Spongman avatar Spongman commented on March 28, 2024 1

#12673

from typescript.

everson avatar everson commented on March 28, 2024

While throwing a compiler error is a good solution, perhaps the compiler could output the classes in the right order. That would be a killer feature. E.g. The compiler keeps track of dependency relationship and output the classes according to that, throwing the compiler error only when unable to resolve the dependency order.

from typescript.

jvilk avatar jvilk commented on March 28, 2024

The compiler keeps track of dependency relationship and output the classes according to that, throwing the compiler error only when unable to resolve the dependency order.

Should we make this a new suggestion? This is why I currently use AMD modules rather than TypeScript internal modules; the RequireJS compiler determines the appropriate module serialization order using the dependencies I specify across the codebase (using require()).

from typescript.

RyanCavanaugh avatar RyanCavanaugh commented on March 28, 2024

Linking to #274. We need to outline what the rules and scope of this would be

from typescript.

mhegazy avatar mhegazy commented on March 28, 2024

The extends case seems like a good candidate for lexical checking; we just need to ensure that lexically the base class comes before the derived one. Are there other cases that we should consider?

from typescript.

sparecycles avatar sparecycles commented on March 28, 2024

One issue is that reordering class definitions might reorder the static initializer order silently. If the base class comes after a derived class I vote to either maintain the static initializer code at the class definition site or just flag an error.

I think that the multiple file case is more interesting and useful from a large project / maintenance standpoint (which is the ostensible goal of typescript, after all).

So, I think we need to consider the output order in single-file output mode. (it would also be nice to be able to get this order for building html files which include multiple files).

Here are some statements that I think would ensure ordering:

class X extends Y {} // ensure Y is defined in prior file
module { new X(); } // ensure X is defined in prior file
class S { static z = new Z(); } // ensure Z is defined in prior file

We could also extend this to functions and variables being defined before use, not just classes.

P.S. I have a prototype.

from typescript.

danquirk avatar danquirk commented on March 28, 2024

I don't think there is any intention to attempt to reorder emit for you, only to give errors where we can for things that are sure to fail at runtime.

from typescript.

sparecycles avatar sparecycles commented on March 28, 2024

Dan, I agree with you about reordering within a single file, but when multiple files are combined using --out, the compiler has control over the emit order and I'd prefer that the order it chooses, works.

from typescript.

mhegazy avatar mhegazy commented on March 28, 2024

@sparecycles functions are hoisted to the top of the scope anyways at run time. so functions are not interesting. variables are hoisted as well, the issue is that they will not be initialized at that point. now use before initialization is a different issue, and I think it is tracked under #274.

for reordering; the philosophy we have followed, is to let the output code be as close as possible to the input code. in essence we let the user code pass through, we just strip out types. in this sense, an error would be more inline with what we have done thus far.

As for the implementation, I have added a lexical order verification recently with Let and Const, and can be extracted out as a general check and used for these different cases. you can find it here:
https://github.com/Microsoft/TypeScript/blob/master/src/compiler/checker.ts#L329

We need to clearly identify the cases where we are checking, and a PR would be definitely welcomed :)

from typescript.

sparecycles avatar sparecycles commented on March 28, 2024

Yes, I agree that we don't want to reorder within a single typescript file, but in the --out file case, the order is not specified by the user so, again, I would prefer that the compiler makes a best effort to choose an order that works.

The function hoisting is a good example of where we don't need to care in the single file case, but where compiling to multiple files and choosing a sequence to include them in an .html file can be non-trivial for a human. Variables being undefined at use is a great example of where unexpected behavior can be introduced by the compiler because of a change in /// <reference> lines.

from typescript.

RyanCavanaugh avatar RyanCavanaugh commented on March 28, 2024

but in the --out file case, the order is not specified by the user

This isn't really the case. We have very simple rules here -- use the order implied by the reference tags and the order of files on the command-line. In both cases, the user is providing us an order. Having the compiler ignore the ordering that the user provided is a dangerous route to go down. What if the compiler decides an order different from the one you'd prefer? How would you override that? What if one order breaks 2 classes and another order breaks 2 variables?

from typescript.

sparecycles avatar sparecycles commented on March 28, 2024

Then we shouldn't change the order but should we at least (have the option to) warn the user that the order the compiler uses is probably wrong?

from typescript.

mhegazy avatar mhegazy commented on March 28, 2024

yup. we should not order, but error instead.

from typescript.

metaweta avatar metaweta commented on March 28, 2024

What's the right idiom in TypeScript for mutually recursive classes? A declare class before the actual definition?

from typescript.

vladima avatar vladima commented on March 28, 2024

Ok, what rules we would like to implement as a part of this issue besides 'base class should be lexically defined before the derived class'?

from typescript.

RyanCavanaugh avatar RyanCavanaugh commented on March 28, 2024

Disallow forward references to internal module members, e.g.

var x = M.fn(); // Should error
module M {
    export function fn() {}
}

Disallow forward references to enum members

var x = E.A; // Should error
enum E { A }

from typescript.

NoelAbrahams avatar NoelAbrahams commented on March 28, 2024

IMO the scope of this issue is rather limited as it stands because people do not normally define all their code in a single file: the base class is more likely to exist in a separate file to the derived class.

I suggest the following from @sparecycles should also be part of the resolution to this issue:

Then we shouldn't change the order but should we at least (have the option to) warn the user that the order the compiler uses is probably wrong?

The "order the compiler uses" should include the order specified in tsconfig.

from typescript.

sparecycles avatar sparecycles commented on March 28, 2024

When the base class and derived classes are in the same file, the problem is not as bad: the program will crash on startup and the programmer will change the order in that one file and the problem will be fixed.

The multiple file case is where the compiler should warn when concatenating multiple files in a dubious order, because that order can change for subtle reasons.

Consider the case where there is a base class and multiple derived classes, all in separate files. The base class uses some of the derived classes in its implementation, and so it references them, but it still needs to be placed first in the output. Similarly, all the derived classes need to reference the base class.

Well, there is no problem with mutually referenced files, if A.ts mutually references B.ts and X.ts includes A.ts, then the output order will be [B,A,X], and if it references B.ts the order will be [A,B,X]. (But only one of these orders might work at runtime.) This makes things fragile, as compilation will succeed equally well if either B or A is referenced.

My solution for my base-derived class system problem: add an index.ts for my class hierarchy and include, in this file, all the derived classes, followed by the base class. This guaranteed that the output would put the base class first. (completely counter-intuitive!). I found that if I directly referenced the files I wanted it would end up generating the base class after the derived one.

The compiler warning will be very nice, but it would also be great to be able to flag one of the references in a mutual-reference scenario as emit-ordering, and the other is just for pulling in declarations. Mutual emit-ordering references would be an error.

( I currently have this implemented for automatically generating the list of .js includes in our Visual Studio/Typescript project since we're using multi-file output (easier to debug). But the code is in C# as an inline task. If there is interest I will ask if I can share it. It's basically two runs of Tarjan's CC algorithm. )

Both warning when emit-order is wrong and stabilizing emit-order with an explicit directive would go a long way toward making typescript a viable language for large projects... yes?

from typescript.

xwipeoutx avatar xwipeoutx commented on March 28, 2024

I run into this issue fairly frequently with my rather small code base (around 80 .ts files). Ideally, I'd like to not have any <reference> tags at the top of any of my files, and the compiler can work it all out for me.

My app has only 1 file that instantiates classes and executes the application (my composition root), a few files that add extensions (eg. adding Array.prototype.distinct) and the rest are just class/interface definitions.

In this case, most of the code is fair game for reordering, and shouldn't require manually <reference> definitions to get right. I see class definitions as being fair game for any compiler reordering, and should be shunted up to the top of the combined output, while the rest of the statements can preserve ordering as it was when input.

Would a compiler flag --looseSorting be possible? It seems like a fairly sought after feature.

from typescript.

yuit avatar yuit commented on March 28, 2024

In emit class-declaration in ES6, we do static property assignment after class-declaration. This makes referring to class static property in computed property name becomes use-before-definition.

Emitted JS:

class C {
    [C.p] () {}  // Use before definition
    [C.p+ C.e]() {}  // Use before definition
    [D.f] () {}  // Use before definition
}
C.p = 10;
C.e = 20;

class D {
}
D.f = "hi";

We only want to warn about this error in two cases: computed-property names refers to its class static property, or refer to other class, that defined below it, property.

from typescript.

DanielRosenwasser avatar DanielRosenwasser commented on March 28, 2024

The trivial example we were playing with today, just to include in any tests:

function f() {
    function g() {
        i = 10;
    }

    let i = 20;
    g();
}

It'd be good to get the permutations of uses/definitions of g around i.

from typescript.

jvilk avatar jvilk commented on March 28, 2024

Don't forget to think about functions defined within block scope. That's undefined behavior as per the JavaScript standard, and I know at least Firefox and Chrome disagree in their implementation.

e.g.:

function f() {
    if (true) {
        g(); // iirc, g executes in Chrome, and is undefined in Firefox
        function g() {
        }
        g(); // works in both browsers
    }
}

from typescript.

danquirk avatar danquirk commented on March 28, 2024

Tracked by #2854 now.

from typescript.

MrGuardian avatar MrGuardian commented on March 28, 2024

I find it ridiculous that TS does not support this feature out of the box. The confusion it causes is similar as using standard JS. Also, virtual methods, anyone?

from typescript.

RyanCavanaugh avatar RyanCavanaugh commented on March 28, 2024

@MrGuardian the repro described in the OP has been fixed. Perhaps you can clarify in a new issue or existing issue that better describes the problem you're having?

from typescript.

Spongman avatar Spongman commented on March 28, 2024

(#12673) here's another two cases that IMO should be errors:

class Test
{
    _b = this._a; // undefined, no error/warning
    _a = 3;

    static _B = Test._A; // undefined, no error/warning
    static _A = 3;
    
    method()
    {
        let a = b; // Block-scoped variable 'b' used before its declaration
        let b = 3;
    }
}

from typescript.

RyanCavanaugh avatar RyanCavanaugh commented on March 28, 2024

@Spongman can you log that in a separate issue please? Thanks!

from typescript.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.