Skip to content

Dependency ordering with internal modules (namespaces) #8013

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

Closed
volkanceylan opened this issue Apr 11, 2016 · 19 comments
Closed

Dependency ordering with internal modules (namespaces) #8013

volkanceylan opened this issue Apr 11, 2016 · 19 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@volkanceylan
Copy link

TypeScript Version:

1.8.0

Code

I'm porting my ASP.NET MVC app platform (Serenity) which were using Saltaralle C# -> JS transpiler to TypeScript. Currently TS is an alternate option but hoping to make it primary soon, after resolving some issues.

Saltaralle and its predecessor Script# uses some code similar to TypeScript internal modules or namespaces. So it was natural for me to choose this model over more complicated AMD/CommonJS etc. modules, which is a bit overkill for web apps in my humble opinion. Also it is much easier to explain to novice users.

I'm using tsconfig.json with outFile option, and no "files" list, so all .ts files in all subfolders are included by default.

If you have A.ts:

namespace Some {
        export class A {
        }
}

and B.ts:

namespace Some {
        export class B extends A {
        }
}

I'd expect combined output.js file to have A defined before B, otherwise it would cause a javascript error.

But, sometimes this is true, sometimes opposite. I know that you'd suggest me to put /// references or put files in correct order in tsconfig.json files property, or use external modules, but this exactly what i'm trying to avoid. I've read among several issues reported here, and workarounds offered like these, but dependency order resolving shouldn't be the job of developer, compilers are specially made for this.

I'd handle these workarounds myself, for my own projects, but my targeted users (devs) usually don't have the skills to understand what this ordering is all about. They'll simply report a bug with details: "not working", and without looking at their output file, i can't even guess what it is really about.

I understand that in some edge cases this might be difficult, but please offer an option to perform topological sort on input files for basic cases like this, e.g. a class extends another, references it in a decorator argument etc.

Expected behavior:

I'd expect internal modules to be topologically sorted based on dependencies.

Actual behavior:

Sometimes it is not. Derived class outputted before base class.

@RyanCavanaugh
Copy link
Member

I know that you'd suggest me to put /// references or put files in correct order in tsconfig.json files property, or use external modules, but this exactly what i'm trying to avoid.

Why?

What happens if the user writes code like this?
A.ts

console.log('A init');
class A extends C { }
console.log('B init');
class B { }

B.ts

console.log('C init');
class C { }
console.log('D init');
class D extends B { }

There's no real solution here other than understanding that code ordering matters.

@volkanceylan
Copy link
Author

I clearly understand code ordering matters for TypeScript and Javascript. As i commented above, there may be some edge cases. Let's say it should at least work with one class per file.

Anyway Saltaralle is also a transpiler and it can handle the case you wrote above without any problems at all. It will order classes correctly before outputting.

@RyanCavanaugh
Copy link
Member

You can re-order C# code arbitrarily because there's no code outside of classes in C#. But in the above example, where should the console.log statements go? What if those statements have important side effects that change the behavior of class initialization?

@volkanceylan
Copy link
Author

I understand, but my request wasn't this. I didn't ask TS to order classes, just input files. If something could be resolved with // reference statements, it should be resolved with topological sort too. If not, it's perfectly ok. Your sample can't be resolved with /// references too...

@volkanceylan
Copy link
Author

Maybe it was my mistake to state Saltaralle can do this, was just informational. Also this should be an "option" that goes into tsconfig.json, not default.

@RobertoMalatesta
Copy link

RobertoMalatesta commented May 12, 2016

+1 on @volkanceylan see for an instance:
In the same file:

class Utilizer {
public static iUseIt= Proxy.objectProxy
}

class Proxy {
public static objectProxy = Proxy.createTheObject()
private static createTheObject () : any {
return "test the dependency"
}
}

console.log(Utilizer.iUseIt)

Console shows the error:
Uncaught TypeError: Cannot read property 'objectProxy' of undefined

If you invert Proxy and Utilizer the run gives the correct result.
And code is class-encapsulated.

@RobertoMalatesta
Copy link

RobertoMalatesta commented May 12, 2016

Or worse:

class Utilizer {
public static iUseIt= Proxy.objectProxy
public static tellme() {
console.log(Utilizer.iUseIt)
}
}

class Proxy {
public static objectProxy = Proxy.createTheObject()
private static createTheObject () : any {
return "test the dependency"
}
}

Utilizer.tellme()

@rcollette
Copy link

rcollette commented May 18, 2016

After working on a project for over a year, I am surprised that we just ran into this issue as well. I'm surprised the compiler does not throw an error on something like:

namespace x{
    import test = y.test;

    export class Instance{
        public user:any = new test();
    }
}

namespace y{
    export class test{
        public anything:string = 'anything';
    }
}

var instance = new x.Instance();

At the time the import runs (in an IIFE), the y namespace has not yet been defined and you get an undefined error.

@volkanceylan
Copy link
Author

Worst part is it is really hard to understand what is the problem from the error message you get. This is especially true in large scale projects, where TypeScript is supposed to "scale" better compared to Javascript.

@RobertoMalatesta
Copy link

I Agree:
If being tied to the class definition order is the intended behavior of TS the compiler could at least emit a warning, based on the info available to it in tsconfig.json.

@rcollette
Copy link

In my use case we are either wiredep'ing individual files which just come in in alphabetic order or we concat/uglify them into one application code file. Coming back from ngConf, it is apparent that external modules (ES6 imports) and using rollup.js or WebPack v2 (to concatenate as needed) are the preferred solution.

@RobertoMalatesta
Copy link

@rcollette more than on file compilation order it's dependent
on the transpilation order and the dynamic nature of JS.

The small example I posted before works (breaks) even as a single file.

That's why I think that tsc should at least file a dependency warning.
Or change definition semantics and make another step away of JS.

@volkanceylan
Copy link
Author

Users already started to report issues related to this ordering problem. I think it should at least be deterministic in ordering. If input files are not specified, and located by searching in file system, compiler shouldn't enumerate them in random order. It should order them with some some deterministic sort, e.g. full path name. Otherwise, it produces different output in each compile, and it works on one compile, while it doesn't in another.

@basarat
Copy link
Contributor

basarat commented May 24, 2016

I've written about the hazards of outFile https://basarat.gitbooks.io/typescript/content/docs/tips/outFile.html 🌹

@basarat
Copy link
Contributor

basarat commented May 24, 2016

It should order them with some some deterministic sort, e.g. full path name

@volkanceylan You can order the files in tsconfig.json (or if not using that use _references.ts) 🌹 . Best to just use modules ❤️

@volkanceylan
Copy link
Author

Adding files to tsconfig.json manually and in the position it should is like something from ancient times. And i cant imagine a compiler that cant do such a simple thing. Using outFile is bad just because compiler cant handle it properly, not because its a bad idea. If a build tool can order files correctly, compiler should do it much easier.

@basarat
Copy link
Contributor

basarat commented May 24, 2016

if a build tool can order files correctly, compiler should do it much easier.

The build tool can only do it if you provide the hints needed aka module exports / imports. And then the TypeScript compiler can also do it (e.g. --outFile + --module requirejs) 🌹

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented May 24, 2016

As an analogy, we don't re-order statements in a function. It's up to you to determine what the correct order of statements in a function is, and it's up to you determine what the correct order of your files is (if ordering matters for your program). In any other mode than folder enumeration, that ordering follows a deterministic algorithm.

I agree that when enumerating files in a folder, that ordering should be deterministic. Logged #8776

@volkanceylan
Copy link
Author

Ok, thanks, i can live with deterministic order and references statements.

@mhegazy mhegazy closed this as completed Jun 7, 2016
@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jun 7, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

6 participants