6

I have an API which denormalizes data causing circular dependency. Is there a way to refactor the following using abstract classes, interfaces, composition or other techniques where I wouldn't need to create N partial classes for each entity to avoid a circular dependency in my Angular application's type definitions?

model.ts

export abstract class Model {
  // ... Model related data members and functions
}

person.ts

import { Model } from './model';
import { Site } from './site';

export class Person extends Model {

  // ... Data

  site: Site; // Partially or Fully saturated site entity
}

site.ts

import { Model } from './model';
import { Person } from './person';

export class Site extends Model {

  // ... Data

  people: Person[]; // array of partially saturated people entities, site is left undefined
}

I would like to maintain a single definition for each model, instead of redefining Site, Person, etc each time there is a dependency like this.

JME
  • 2,293
  • 9
  • 36
  • 56
  • Even whithout talking about dependencie isn't having a Person which has a Site which has an array of Person which all have a Site which all have an array of Person which all has a Site etc. the best way to instantly blow up your memory? – Gilsido Jul 25 '19 at 15:12
  • 1
    The people entities that come back on Site don't have a site which prevents this. Hence the partially saturated comment. – JME Jul 25 '19 at 15:16
  • Yeah maybe in your current implementation you dodged this problem, but it's still a very good reason for Angular to prevent you from doing this? Can't you just have the Site id in the Person model? – Gilsido Jul 25 '19 at 15:24
  • 1
    I wonder if a decorator would be a good fit. You could maybe find or roll something like [JsonIgnore](https://fasterxml.github.io/jackson-annotations/javadoc/2.5/com/fasterxml/jackson/annotation/JsonIgnore.html). – ChiefTwoPencils Jul 25 '19 at 17:05
  • Have you seen new import type in TypeScript 3.8? When you import module as type, it works as interface and output javascript ignores all such imports. – Akash Kava Mar 09 '20 at 06:49
  • @AkashKava Import elision has been in TypeScript for a long long time. The thing that 3.8 adds is give more control to developers over when the compiler applies import elision. The code *shown in the question* does not need to use `import type` because import elision takes care of removing the circularity (`import { Site } from './site';` and `import { Person } from './person';` are elided because `Site` and `Person` are used as types, not values in the code shown. Mind you, if they were used as *values* the circularity would remain but then `import type` would *not* be a solution. – Louis Mar 09 '20 at 11:21
  • @Louis I know if they are used as `values` they are not the solution, but in that case op has not mentioned it, sometimes there are simpler solution but they are simply not aware of it, and manytimes when types are used deeply, import elision does not work correctly that was the reason 3.8 allows explicit control – Akash Kava Mar 09 '20 at 13:36
  • @AkashKava `import type` will not cause elision of imports in cases where it did not happen automatically before. I've relied on import elision dozens of times in my own code. `tsc` never failed to elide when possible. The problem though it that if code depends on elision to work correctly, then it is very easy to mess things up by accidentally referring to a value while refactoring. (I've done it.) This is not always a problem for the compiler and may be discovered late. Using `import type` for such files allows the discovery early because it prohibits value accesses. – Louis Mar 09 '20 at 13:51
  • 2
    Still not clear to me. The way I interpret above code is that it has nothing to do with TypeScript. A [complete example](https://stackoverflow.com/help/minimal-reproducible-example) would be helpful @ReactingToAngularVues. – ford04 Mar 09 '20 at 14:32
  • @ford04 Nothing to do with TypeScript? The OP's code contains TypeScript type annotations and the files have the extension normally used for TypeScript files. – Louis Mar 09 '20 at 14:49
  • @Louis I mean you could write it as JS code and still get the circular module dependency error (but I can only guess from above code). It has to do with the imported *values* and not the types. Unfortunately, OP only shows the types' usage, not how `people: Person[]`, `site: Site` are instantiated and used as values. – ford04 Mar 09 '20 at 14:55
  • @ford04 It is true that the issue may not be due to how TypeScript is used though the bounty giver mentions getting a warning from the compiler. It could be that the source of the warning was misidentified. In this context, if it is not the compiler, I'd expect Webpack to be the best candidate for who emitted the warning. In my experience it is a bit twitchy with circular dependencies and warns about circularities that are 100% fine at run-time. I just ignore it. – Louis Mar 09 '20 at 15:08
  • 3
    @ReactingToAngularVues can you provide a repo/codesandbox which demonstrates the issue? I took your code as posted and was not able to reproduce the issue – DoronG Mar 09 '20 at 17:27
  • Has not been it already answered already [here](https://stackoverflow.com/a/24444910/2039993) and [here](https://stackoverflow.com/a/48419531/2039993)? – Yeheshuah Mar 10 '20 at 07:39

2 Answers2

5

This problem is caused by a bad class model design and it does not depend on the language. I hope this helps :)

The circular dependency problems generally include the chicken & egg problem, because when you want to instantiate an object you do not know which you should instantiate first.

 Solution 1

This problem can be easily solved by only referencing objects by their identities, and instead of having a direct dependency on a big object as a part of constructor. For example, modifying Person class:

import { Model } from './model';

export class Person extends Model {

  // ... Data

  siteId: number;
}

 Solution 2

If you need the full object inside Person, I think a good approach would be to change the inheritance tree. I understand that your data model is about a website, its users and its owner. Well, lets break this down: make Person an abstract class and extend it with User and Owner classes.

person.ts

import { Model } from './model';

export abstract class Person extends Model {

  // ... Data
  
  // Does not include the Site attribute!
}

user.ts

Users will be the "readers" of the Site, but don't need a reference to it.

import { Person } from './person';

export class User extends Person {

  // ... Data

}

owner.ts

Owner will be the creator of the Site. This is the class that has the reference to the Site.

import { Person } from './person';

export class Owner extends Person {

  // ... Data

  site: Site;

}

site.ts

Finally, the Site keeps track of its users.

import { Model } from './model';
import { User } from './user';

export class Site extends Model {

  // ... Data

  users: User[];
}

I think this related article could be of help for understanding circular dependencies: How to fix nasty circular dependency issues once and for all in JavaScript & TypeScript

Community
  • 1
  • 1
adrisons
  • 3,443
  • 3
  • 32
  • 48
  • 1
    Thank you. Ultimately, as hard as it is to hear, this is better solved by a refactor of the code involved and a re-approach to how clientside models work. Specifically, at least for Angular, many sources suggest only including the data you need in each model, rather than producing a 1:1 mapping with your backend schema. – marked-down Mar 16 '20 at 00:06
2

Why don't use internal module pattern which export all the class and used internally by models to load dependencies.

Example Application: https://codesandbox.io/s/circular-deps-fix-using-internal-module-pattern-mtfro

internal.ts

export * from './model'
export * from './person'
export * from './site'

model.ts

export abstract class Model {
  // ... Model related data members and functions
}

person.ts

import { Model, Site  } from './internal';

export class Person extends Model {

  // ... Data

  site: Site; // Partially or Fully saturated site entity
}

site.ts

import { Model, Person } from './internal';

export class Site extends Model {

  // ... Data

  people: Person[]; // array of partially saturated people entities, site is left undefined
}

the above rules only apply to our local dependencies. External module imports are left as is. They are not involved in our circular dependency problems after all.

Kathak Dabhi
  • 399
  • 3
  • 16