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

feat: add InjectMikroORMs inject decorator #192

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tsangste
Copy link
Contributor

@tsangste tsangste commented Nov 9, 2024

Create an inject decorator for multiple mikro-orms

@tsangste tsangste force-pushed the multiple-mikro-orms-inject branch from 56f0209 to 345961b Compare November 9, 2024 20:32
@B4nan
Copy link
Member

B4nan commented Nov 10, 2024

What is this good for?

@tsangste
Copy link
Contributor Author

What is this good for?

Kind of a follow on from #88 where instead of doing:

@Injectable()
export class MyService {

  constructor(@Inject('MikroORMs') private readonly orms: MikroORM[]) { }

}

we can do:

@Injectable()
export class MyService {

  constructor(@InjectMikroORMs() private readonly orms: MikroORM[]) { }

}

which makes it look nicer and less strings.

I can also add it within the README.md too if that help?

@B4nan
Copy link
Member

B4nan commented Nov 10, 2024

Yes, but what is that good for?

@tsangste
Copy link
Contributor Author

tsangste commented Nov 10, 2024

Yes, but what is that good for?

Its just a helper and just gives us these benefits:

  • Consistency with the other helper (even those they are used for context names)
  • Less risk of typos for the string name
  • Intellisense

@B4nan
Copy link
Member

B4nan commented Nov 10, 2024

Once again, what is this good for? Why would you as a user want this? I understand what you did there, I am asking for what use case? Its not documented anywhere, neither the explicit @Inject('MikroORMs') nor this new version.

@tsangste
Copy link
Contributor Author

tsangste commented Nov 10, 2024

Once again, what is this good for? Why would you as a user want this? I understand what you did there, I am asking for what use case? Its not documented anywhere, neither the explicit @Inject('MikroORMs') nor this new version.

Sorry I understand now, we basically want an easy way to get all registered database so we can do the following:

  • Get all databases for health checks
  • Multiple Multi-Tenancy database to apply setFilterParams

This would make it easier than having to manually add a new InjectMikroORM for every new database.

src/mikro-orm.common.ts Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@tsangste tsangste requested a review from B4nan November 18, 2024 13:07
@tsangste
Copy link
Contributor Author

@B4nan updated based off your review

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 this pull request may close these issues.

2 participants