feat: add depth limiter optional parameter when loading nested trees #7926
Conversation
src/repository/FindTreesOptions.ts
Outdated
| /** | ||
| * When loading a tree from a TreeRepository, limits the depth of the descendents loaded | ||
| */ | ||
| depth: number; |
There was a problem hiding this comment.
make depth optional.
when depth is undefined then treat that as the unlimited case.
I think that would be a clearer API & would match other cases where we have optional options
There was a problem hiding this comment.
Thanks for the suggestion. I agree that it does indeed make more sense.
I have implemented the suggested change, and extended the test to include examples of using empty objects for the new arguments. Let me know if there's anything else you think can be improved here.
Thanks!
There was a problem hiding this comment.
@imnotjames I just rebased my branch to include the latest changes done to master. Assuming the tests (currently running) pass, is there anything else I can to do help move this PR forward?
There was a problem hiding this comment.
@imnotjames I've once again rebased my branch, and the tests are passing. Let me know if this PR is relevant to TypeORM, or if not, I can close it. thanks
tiagojsag
force-pushed
the
add-optinal-depth-limit-to-tree-repository-methods
branch
2 times, most recently
from
a737dd3
to
a2f8ef0
Compare
tiagojsag
force-pushed
the
add-optinal-depth-limit-to-tree-repository-methods
branch
from
a2f8ef0
to
2fd5f93
Compare
tiagojsag
force-pushed
the
add-optinal-depth-limit-to-tree-repository-methods
branch
from
2fd5f93
to
85ea143
Compare
|
@tiagojsag please rebase it once again and I'll merge it. Thank you! |
…using TreeRepository's findTrees() and findDescendantsTree() Modified TreeRepository's findTrees() and findDescendantsTree() public methods to now accept an optional object with config options. For now, said object contains a depth parameter that, if set, will limit how far down the tree those methods will crawl and retrieve nested entities. BREAKING CHANGE: TreeRepository's protected method buildChildrenEntityTree() now requires a 4th argument. Anyone affected by this break should also review and update their implementation, otherwise this feature will not work. Closes: typeorm#3909
tiagojsag
force-pushed
the
add-optinal-depth-limit-to-tree-repository-methods
branch
from
85ea143
to
4b7a520
Compare
|
👍 done |
|
Thank you for contribution! 🎉 |
|
Thanks for maintaining! |
tiagojsag
deleted the
add-optinal-depth-limit-to-tree-repository-methods
branch
…using TreeRepository's findTrees() and findDescendantsTree() (typeorm#7926) Modified TreeRepository's findTrees() and findDescendantsTree() public methods to now accept an optional object with config options. For now, said object contains a depth parameter that, if set, will limit how far down the tree those methods will crawl and retrieve nested entities. BREAKING CHANGE: TreeRepository's protected method buildChildrenEntityTree() now requires a 4th argument. Anyone affected by this break should also review and update their implementation, otherwise this feature will not work. Closes: typeorm#3909
| @@ -261,14 +266,18 @@ export class TreeRepository<Entity> extends Repository<Entity> { | |||
| }); | |||
| } | |||
|
|
|||
| protected buildChildrenEntityTree(entity: any, entities: any[], relationMaps: { id: any, parentId: any }[]): void { | |||
| protected buildChildrenEntityTree(entity: any, entities: any[], relationMaps: { id: any, parentId: any }[], options: (FindTreesOptions & { depth: number })): void { | |||
There was a problem hiding this comment.
@tiagojsag @pleerock this definition requires me to add depth property to the options argument.
Since FindTreesOptions only has depth as optional property, is there a reason why options can't be optional and just use FindTreesOptions as it's type and not cause a breaking change? As it is right now, it looks like I'll have to update buildChildrenEntityTree with { depth: -1 } everywhere I don't want changing functionality.
There was a problem hiding this comment.
I think you may be correct, and this can probably be modified to be optional.
Description of change
feat: add depth limiter optional parameter when loading nested trees when using TreeRepository's findTrees() and findDescendantsTree()
Modified TreeRepository's findTrees() and findDescendantsTree() public methods to now accept an optional object with config options.
For now, said object contains a depth parameter that, if set, will limit how far down the tree those methods will crawl and retrieve nested entities.
BREAKING CHANGE: TreeRepository's protected method buildChildrenEntityTree() now requires a 4th argument. Anyone affected by this break should also review and update their implementation, otherwise this feature will not work.
Closes: #3909
Pull-Request Checklist
masterbranchnpm run lintpasses with this changenpm run testpasses with this changeFixes #0000Unrelated tests are failing for me both on the branch and on master. I ran the individual tests I added to cover my changes, and hopefully CI will take care of the rest.