Skip to content
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

Circular dependency #4149

Closed
metabacalhau opened this issue Aug 4, 2015 · 17 comments
Closed

Circular dependency #4149

metabacalhau opened this issue Aug 4, 2015 · 17 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@metabacalhau
Copy link

Looks like I have a circular dependency between three classes ('->' means 'references'): A -> B -> C -> A.

Class A:

import B = require('services/B');

class A
{
    run()
    {
        B.run();
    }
}

export = A;

Class B:

import C = require('services/C');

class B
{
    run()
    {
        C.run();
    }
}

export = B;

Class C:

import A = require('services/A');

class C
{
    run()
    {
        A.run();
    }
}

export = C;

Whenever I try to execute C.run in B I get exception: "TypeError: C.run is not a function" and in debug mode C is nothing, but an empty object.

Ok, I can redesign my classes to break the chain, but is it possible to have such references in TypeScript and what do I need to do?

@mhegazy
Copy link
Contributor

mhegazy commented Aug 4, 2015

stupid question.. are these functions instance or static?

also what module loader are you using?

@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Aug 4, 2015
@jklmli
Copy link

jklmli commented Aug 4, 2015

@mhegazy Will using the ES6 Module syntax (#2242) prevent issues like this?

Note that destructuring import declarations are rewritten to property accesses on the imported module object. This ensures that exported members can circularly reference each other. For example:

Does this apply to the export = C style of exports as above?

@jklmli
Copy link

jklmli commented Aug 4, 2015

Specifically, we have comments like this in our codebase:

Use private static getters that call to memoized fns lazily
rather than static members which are resolved during load time
in order to avoid circular-dependency problems.

It's very hard to predict when these issues will happen, since compilation succeeds with the import as an empty object, and things blow up at runtime.

We're using browserify to bundle up our code, and we use the tsify plugin.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 4, 2015

@jiaweihli since you are using browserify i am assuming you are compileing with --module commonjs`. in this case, your output code will look something like:

var B = require('services/B');
var A = (function () {
    function A() {
    }
    A.prototype.run = function () {
        B.run();
    };
    return A;
})();
module.exports = A;

so it all depends on how your loader will handle circular dependencies at run time. i will need to play with the loader of a bit to get better understanding of what is going on.

@jklmli
Copy link

jklmli commented Aug 4, 2015

Seems to work correctly with ES6 modules (even circular static methods!) when using export default, and breaks at runtime as before with the older external module syntax. Going to move ahead with migrating our codebase to 1.5, will report back if things don't go smoothly.

@jklmli
Copy link

jklmli commented Aug 6, 2015

Migrated codebase over - it seems like using ES6 imports forces everything to be lazily evaluated, which solves the majority of circular import issues. However, given the scenario where you have a class A, and class B extends A, and A requires C, C requires B, B requires A, things will still break. My hunch is because there's a forced invocation of A due to __extend.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 10, 2015

@jiaweihli is still an issue?

@jklmli
Copy link

jklmli commented Aug 11, 2015

Yes, both for the case I mentioned above ^, and for the case of static getters, which are immediately evaluated.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 11, 2015

static getter/setter is by design. you can find more about it at #1520 (comment)

@mhegazy mhegazy closed this as completed Sep 17, 2015
@agaace
Copy link

agaace commented Aug 11, 2016

Hitting same issue on Node.js, with the "export = A;" syntax.
In pure JS I could manipulate the module.exports value and set it at the beginning of the file, for example. In TS it's not an option. What is the standard workaround for the issue on Node.js?

Example:
A.ts
import B = require('B');

class A {
public static foo(b: B): void {
// do something with b.
Console.log(b);
}
}
export = A;

B.ts
import A = require('A');

class B {
public static foo(a: A): void {
// do something with a.
Console.log(a);
}
}
export = B;

C.ts
B.foo(new A()); // Runtime error: B.foo is not a function

@Izhaki
Copy link

Izhaki commented Sep 22, 2016

I just had the same issue and it was a nightmare to figure out. While I do understand that the loaders may play part here, I wonder if at least the compiler should throw a warning?

First I had this (using import { X } from 'X'):

  • Module C called
  • Function B from module B
  • Function B called function A from module A

I then changed the code to add a call from Function A to a function in Module C... and got undefined for method A.

This was all within Jasmine tests using ts-node.

@agaace
Copy link

agaace commented Sep 28, 2016

Why is this issue closed when no answer to these issues has been posted???

I've been stuck with the same problem in Node JS for days now. I need to reference type A from B, and B from A. In pure JS, I would use runtime require and place "module.exports" at the beginning of the file, so Node can fill in the missing references later, as it resolves them. However, in Typescript I have no control over how and where "module.exports" is generated, so it always overwrites the whole object, causing unresolved modules. So what is the official workaround for Typescript files??

Not creating circular dependencies at all is NOT an option when models refer to each other (think foreign keys in a database).

@mhegazy
Copy link
Contributor

mhegazy commented Sep 29, 2016

Why is this issue closed when no answer to these issues has been posted???

See #4149 (comment) for details. the above sample should be authored with ES6 syntax. this way you get the defered reference to your functions.:

// a.ts
import { B } from './b';

export class A
{
    static run()
    {
        B.run();
    }
}
// b.ts
import { A }  from './a';

export class B
{
    static run()
    {
        A.run();
    }
}

@kamranayub
Copy link

kamranayub commented Dec 30, 2016

However, given the scenario where you have a class A, and class B extends A, and A requires C, C requires B, B requires A, things will still break. My hunch is because there's a forced invocation of A due to __extend.

This is still a problem. I'm trying to figure out how to use ES6 syntax with --outFile and --module amd, but this dependency thing for extended classes is killing me. This is a common scenario in large libraries. Somehow I need to move module.exports to the top of the file in the generated code.

@ashleydavis
Copy link

I have this issue as well. It's such a basic thing to be able to need to do and TypeScript can't handle it.

@Spongman
Copy link

it seems like typescript should emit warnings if it's compiling to a module system that doesn't support cycles.

@wtho
Copy link

wtho commented Feb 7, 2018

@Spongeman haven't looked much into it, but these tslint-rules might help you.

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

9 participants