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

[feature] Populate all #160

Closed
lookfirst opened this issue Sep 24, 2019 · 12 comments
Closed

[feature] Populate all #160

lookfirst opened this issue Sep 24, 2019 · 12 comments
Labels
enhancement New feature or request
Milestone

Comments

@lookfirst
Copy link
Contributor

I'd love an option to just load a top level entity and have it fully populated without having to do anything. Maybe just {populate: true}.

@lookfirst lookfirst added the enhancement New feature or request label Sep 24, 2019
@B4nan B4nan added this to the 3.0 milestone Sep 24, 2019
@B4nan B4nan mentioned this issue Sep 24, 2019
59 tasks
@B4nan
Copy link
Member

B4nan commented Sep 24, 2019

Implemented in 3.0.0-alpha.15, as always, feel free to reopen.

@B4nan B4nan closed this as completed Sep 24, 2019
@lookfirst
Copy link
Contributor Author

sweet! totally makes my day. loading createdBy all the time sucks. ;-)

i know you have your own tests, but this also works:

https://github.com/lookfirst/mikro-example/blob/master/test/db/populate.spec.ts

@lookfirst
Copy link
Contributor Author

One other idea, what about using an annotation for this, like @load. This way, I could just specify fields that I always wanted populated...

@B4nan
Copy link
Member

B4nan commented Sep 24, 2019

Yeah, eager loading, that is also on the roadmap. Will probably do in as part of v3, as it should be pretty easy thanks to how populate support works now.

@lookfirst
Copy link
Contributor Author

lookfirst commented Sep 24, 2019

Oh interesting, populate string | true doesn't allow me to pass in a variable. it should be string | boolean i think.

@lookfirst
Copy link
Contributor Author

Apologies, I don't see a way to reopen this issue.

@B4nan
Copy link
Member

B4nan commented Sep 24, 2019

Its string[] | true, the array part is your problem I think. I could also support plain string.

@lookfirst
Copy link
Contributor Author

No, it is a problem passing a boolean variable.

	async getRig(id: string, populate: boolean = true): Promise<Rig | null> {
		console.log('in get rig!');
		return this.getOrm().em.findOne(Rig, id, {populate});
	}
Error:(20, 10) TS2769: No overload matches this call.
  Overload 1 of 2, '(entityName: EntityName<Rig>, where: string | number | Record<string, any> | { toString?(): string; toHexString?(): string; } | Partial<Rig>, options?: FindOneOptions | undefined): Promise<...>', gave the following error.
    Argument of type '{ populate: boolean; }' is not assignable to parameter of type 'FindOneOptions'.
      Types of property 'populate' are incompatible.
        Type 'boolean' is not assignable to type 'true | string[] | undefined'.
  Overload 2 of 2, '(entityName: EntityName<Rig>, where: string | number | Record<string, any> | { toString?(): string; toHexString?(): string; } | Partial<Rig>, populate?: true | string[] | undefined, orderBy?: Record<...> | undefined): Promise<...>', gave the following error.
    Argument of type '{ populate: boolean; }' is not assignable to parameter of type 'true | string[] | undefined'.
      Object literal may only specify known properties, and 'populate' does not exist in type 'string[]'.

@B4nan
Copy link
Member

B4nan commented Sep 24, 2019

Yeah well false is currently not valid value and it will probably cause an error. You could define your signature as:

async getRig(id: string, populate: string[] | true = true): Promise<Rig | null> { }

But I can support false as well, probably better to do so, as it can be used without TS at all so no type checks for literal types...

@B4nan B4nan reopened this Sep 24, 2019
@lookfirst
Copy link
Contributor Author

Only supporting one side of a boolean argument feels weird, but I think the JS argument is the most compelling.

@lookfirst
Copy link
Contributor Author

Feels weird in that I don't know of a single other API that does that.

@B4nan
Copy link
Member

B4nan commented Sep 24, 2019

Fixed in dev branch.

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

No branches or pull requests

2 participants