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

Relation decorators: allow to pass string instead of typeFunction #4190

Closed
haschu opened this issue May 25, 2019 · 19 comments · Fixed by #4195
Closed

Relation decorators: allow to pass string instead of typeFunction #4190

haschu opened this issue May 25, 2019 · 19 comments · Fixed by #4195

Comments

@haschu
Copy link
Contributor

haschu commented May 25, 2019

Issue type:

[ ] question
[ ] bug report
[x] feature request
[ ] documentation issue


To define table relations using decorators, this is the current way:

Photo.ts

    @ManyToOne(type => User, user => user.photos)
    user: User;

User.ts

    @OneToMany(type => Photo, photo => photo.user)
    photos: Photo[];

This works fine on Node.js, but can cause circular dependency issues on the frontend side (e.g. when using Angular CLI). See: #2059 or #1290

We could overcome those issues, if the following would be possible:

Photo.ts

    @ManyToOne('User', 'photos')
    user: User;

User.ts

    @OneToMany('Photo', 'user')
    photos: Photo[];

This should be fairly easy to implement, as it turns out that TypeORM already supports string instead of types (e.g. when using EntitySchema) - the only thing to do is to adjust the typeFunction type of the decorator functions.

@pleerock If that feature would be ok I could post a PR :)

@haschu
Copy link
Contributor Author

haschu commented May 26, 2019

PR is submitted.

As an example, the following pattern would now prevent circular dependency issues with @OneToMany/@ManyToOne (using abstract classes would be more suitable for bigger entities to prevent duplicate code):

IUser.ts

export interface IUser {

  id: number;

  name: string;

}

IPhoto.ts

export interface IPhoto {

  id: number;

  url: string;

}

User.ts

@Entity()
export class User implements IUser {

  @PrimaryGeneratedColumn()
  id: number;

  @Column()
  name: string;

  @OneToMany('Photo', 'user')
  photos: IPhoto[];

}

Photo.ts

@Entity()
export class Photo implements IPhoto {

  @PrimaryGeneratedColumn()
  id: number;

  @Column()
  url: string;

  @ManyToOne('User', 'photos')
  user: IUser;

}

@jordancue
Copy link

@haschu great work!

@haschu
Copy link
Contributor Author

haschu commented May 30, 2019

@jordancue Thank you :) Hope 0.2.18 will be released soon.

@Michael-Dawkins
Copy link

In my angular app, I have set "showCircularDependencies": false in the angular.json to simply ignore the circular dependency messages and it fixed my issues, it looks clean to me because there indeed are cycles in my typeORM models and this is fine.
The only downside I see to this approach is that, sometimes, the circular dep warning can be helpful to detect an actual code smell.

Am I missing something?

What I prefer with this method over using strings is the maintainability aspect, I can still refactor the name of a class and have my IDE propagate the rename from the db down to the frontend.

@haschu
Copy link
Contributor Author

haschu commented Jun 6, 2019

@Michael-Dawkins I guess you don't miss anything :)

The only downside I see to this approach is that, sometimes, the circular dep warning can be helpful to detect an actual code smell

Beware that circular dependencies can be pretty dangerous, as they lead to null/undefined references that may be hard to debug. That's why I find the warnings are overall helpful.

TypeORM can deal with it, because it forwards the reference using a typeFunction. That's btw. the reason you have to write @OneToMany(type => Something) instead of @OneToMany(Something) - because Something can be null/undefined at this time.

But if you by accident introduce another circular dependency, the compiler won't warn you, and debugging can be a hard time.

@Michael-Dawkins
Copy link

Thank you for the explanation.

It seemed weird to me at first why typeORM does not use @OneToMany(Something), thank you for the insight about the fact it may be undefined if evaluated too early.

@haschu
Copy link
Contributor Author

haschu commented Jun 6, 2019

You're welcome :)

As you you're using Angular: have a look a forwardRef, it's the exact same use-case.

@notHaiLuu
Copy link

Thanks @haschu , you save my life. Just change like this:

From: @OneToMany(() => Todo, (todo) => todo.author)
To: @OneToMany('Todo', 'author')

From: @ManyToOne(() => Author)
To: @ManyToOne('Author', 'todos')

It's resolved circular dependency.
Screenshot 2020-04-07 at 21 55 15

@jdnichollsc
Copy link

@notHaiLuu thanks for sharing! I was getting an issue with Swagger from a NestJS project because that circular dependency
Regards

@kamranblueeast
Copy link

@haschu, thanks good work, saves my day. :)

@valibraimi
Copy link

I HOPE YOU YOU fulfill all your desires. I was going crazy. its first time i comment on a github. you are great @haschu

@DJEDAINI
Copy link

DJEDAINI commented May 5, 2021

@haschu

You saved my day, thank you

loriswit added a commit to frilan/frilan that referenced this issue May 25, 2021
@hecmondev
Copy link

@haschu 2021 almost 2022 finding this solution after a lot of searching thank you so much cowboy nice job! 🥇

@JohnTheBeloved
Copy link

Thanks for the PR fix ,

I am using typeorm with react-native

unfortunately I still have this issue after implementing the fix,

Works on a debug build but not when released

I guess it has to do with the minified index.android.bundle file

[n: Entity metadata for t#entity was not found. Check if you specified a correct entity object and if it's connected in the connection options.] line: 625, column: 174, sourceURL: 'index.android.bundle' } }

@jdnichollsc
Copy link

@JohnTheBeloved try WatermelonDB instead, TypeORM is for Backend side (NodeJS), check my example for React Native here: https://github.com/proyecto26/MyApp

@HyopeR
Copy link

HyopeR commented Feb 15, 2023

@JohnTheBeloved Hello. I have the same problem when I define relations using string as suggested. However, I should point out that only in PRODUCTION mode.

If I define a relation using string. Initializing the database fails with this error;
Entity metadata for t#entity was not found.

If I do relationship definition normally. If I try to save a nested structure using the save method it fails with this error;
cyclic dependency "u".

Were you able to find any solution?

@HyopeR
Copy link

HyopeR commented Feb 15, 2023

A small update. I recreated the Entities using EntitySchema just to try it out. Details are here; https://typeorm.io/separating-entity-definition

There are no problems now and everything is working fine. However, Typeorm does not support Subscribers and Listeners events for EntitySchema. I think it's the decorators that are causing the problem. I'm wondering how we can solve this.

@HyopeR
Copy link

HyopeR commented Feb 17, 2023

If anyone is still struggling with these issues, check out this link. #4714

I am using Typeorm with React native. And the solution in the link makes your application run successfully in production mode.

@LincWong
Copy link

good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.