Circular References & Inheritence

[YoavKa]

I have three files in my project; a.ts:

import {B} from './b';
import {C} from './c';

console.log(B.foo());
console.log(C.foo());

b.ts:

import {C} from './c';

export class B
{
    private c: C;
    public static foo()
    {
        return 1;
    }
}

c.ts:

import {B} from './b';

export class C extends B
{
    public static foo()
    {
        return 2;
    }
}

When bundling(?) the file a.ts, it inserts the class C before the class B to the file; therefore, the js output file cannot be run. With that being said, if you exchange the places of the import statements at the beginning of a.ts, class B is inserted before class C (as it should, because it inherits from it), and the js output file is totally fine. I think the problem is that if the code find circular references it doesn't care about the classes' relationships, when instead it should insert the parent classes before the classes that inherit from them to the output file.

[ToddThomson]

@YoavKa Sorry, I couldn't get to this issue sooner.

Yes, I've reproduced this issue and will have a fix soon.

[ToddThomson]

@YoavKa This comes down to how to order bundle inserts of dependent source files in the presence of circular dependencies. Currently TsProject does not look for circular dependencies when traversing the dependency graph. I will offer the following algorithm: When traversing the dependency graph, TsProject will look for circular dependencies. When found, I will look for inheritance to help resolve the bundle insert order. If there is no inheritance then I will insert the dependent source file in the order it was walked.

[ToddThomson]

@YoavKa TsProject now checks for circular dependencies. It deals with circular dependencies in a basic manner: if detected then the dependent source file will be written to the bundle first.

Please note that the class inheritance is handled by the fact that the base class must be imported in the parent source file ( there is no need to check for the actual class inheritance )

Here is the bundled output from your scenario:

export class B
{
    private c: C;
    public static foo()
    {
        return 1;
    }
}             

export class C extends B
{
    public static foo()
    {
        return 2;
    }
}

console.log(B.foo());
console.log(C.foo());

[YoavKa]

@ToddThomson You're solution seems good, but I was not able to reproduce it locally (maybe github/npm are not up to date?). The example I gave is a simplified version of my project, which has a lot of circular dependencies, so I'm hoping this would fix it. Thanks for the response!

[ToddThomson]

@YoavKa I have not released this fix yet. It needs a bit more testing time. Maybe what I'll do is push the changes to Github first so that you can build and test the change against your project.

EDIT: I've committed the changes to master. Please build TsProject from the sources and test against your project. Let me know how you make out.

[YoavKa]

OK, I tested the changes you made, but they don't work in my case; if you change the order of the import statements in the a.ts file, it won't be able to compile, as it includes C before B. If I understand correctly, the algorithm you implemented is the following: if A is the main bundle file, and -> denotes importing, then if A -> C -> B -> C, the bundler will try and add the source of C before B. This doesn't work in the case where c.ts is encountered before b.ts in the traversal. Another solution that I thought of is that if A -> C -> B, B will be added during the traversal only if it doesn't contain a class that inherit from a class in C; if it does contain one, B will be added immediately after the traversal of the other imports of C is complete.

EDIT: I went over the log when bundling my project, and I think my proposed solution also won't work. I think that the order of the inclusion of the files matters only to the inheritance, and thus a possible, general solution would be:

1. Collect all the files which should be included in the bundle.
2. Create an inheritance graph of the files (a file F inherits from a file G if there exists a class A in F that inherits from class B in G).
3. As the graph is directed, we could find all the nodes which has no "parents" (there is no edge going into them), add them to the bundle file and remove them from the graph. Continuously repeating this step will result in adding all the base classes before their descendants.

[YoavKa]

A pseudo code for the algorithm would look something like this:

1. files <- getAllImportedFiles()
2. parents <- a new, empty dictionary
3. for each file in files:
4.    parents[file] = getParents(file)
5. while there are keys in parents:
6.    for each file in parents:
7.        if parents[file] is an empty list:
8.            addSourceFile(file);
9.            parents.remove(file);
10.           for each file1 in parents:
11.               parents[file1].remove(file);

Of course, safety measures should be taken, such as making sure one does not delete an item from a list / dictionary while iterating over it etc.

EDIT: styling

[ToddThomson]

@YoavKa Thank-you for taking the time to investigate this issue.

You are correct the order of imports is important when handling inheritance. For the basic algorithm I added, my assumption was that import references to a circular dependent pair would always have the base class imported first. This is why I did not include a specific check for inheritance.

I would like to always have TsProject successfully compile the bundle if the source project compiles. However, I do not want to cover anything more than applying a general rule for inheritance. I will add this today.

[ToddThomson]

@YoavKa Diving into this issue has possibly led to a potentially interesting optimization. Can bundling work on import module dependencies ( just the source code referenced by the importClause named bindings ) rather than the whole source file.

For example if you had a bundle source file a.ts as

import {B} from './b';
import {C} from './c';

console.log(B.foo());
console.log(C.foo());

b.ts:

import {C} from './c';

export class B
{
    private c: C;
    public static foo()
    {
        return 1;
    }
}

export class SomeOtherClass {
    public static bar() {
        return 2;
    }
}

Then why include the "SomeOtherClass" source code from b.ts?

[ToddThomson]

@YoavKa It took a bit of experimenting, but the general solution is before including a source code module into the bundle, the source module import statements named bindings are checked against the module exports inheritance clauses. If a named binding matches an inheritance clause name then the import statement module with the match is added to the bundle first.

I want to clean up and fine tune the code changes and do a bit more testing before updating the master branch. I should be able to get a couple of hours tomorrow to finish up so that you can test.

[ToddThomson]

@YoavKa I've updated the master branch with additional changes to handle order of module insertion into the bundle for circular dependencies and inheritance. Please test against your project.

[YoavKa]

Your solution works just fine on my project, Thanks! I hope you will update the npm package shortly ;)

[ToddThomson]

@YoavKa I should be able to create the 2.0.2 release over the weekend. Glad the changes worked for you.