Transparently ensure foreign key source field in inclusion resolvers

[InvictusMB]

Split from #5592

Checklist

• DCO (Developer Certificate of Origin) signed in all commits
• npm test passes on your machine
• New tests added or existing tests modified to cover all changes
• Code conforms with the style guide
• API Documentation in code was updated
• Documentation in /docs/site was updated
• Affected artifact templates in packages/cli were updated
• Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

[InvictusMB]

@bajtos wrote:

Yeah, I agree the callback-based version is much more complex and thus difficult to understand & reason about.

Reviewing this after some time, I don't think the callback version is that bad. The impact on inclusion resolver implementation is minimal and all the madness is contained within includeRelatedModels helper. I also believe that callback contract is more explicit than exposing metadata on a resolver function.

I think the callback-based version will also not work when including more than one relation, e.g. when geting all TodoList instances and including both the related Todo items (via hasMany) and the related TodoListImage (via hasOne).

It did work with whatever I'd throw at it, including multiple relations. includeRelatedModels would accumulate all requirements from all resolvers before actually fetching the entities.

[bajtos]

I am struggling to understand what is this code doing and what is the purpose of captureResult and captureInvocation. Let's try to find a simpler (less clever/smart) solution please, one that's easier to understand even to casual readers.

[stale]

This pull request has been marked stale because it has not seen activity within two months. It will be closed within 14 days of being stale unless there is new activity.

[stale]

This pull request has been marked stale because it has not seen activity within two months. It will be closed within 14 days of being stale unless there is new activity.

[InvictusMB]

Not stale. I'll have time to refresh this soon.

[stale]

This pull request has been marked stale because it has not seen activity within two months. It will be closed within 14 days of being stale unless there is new activity.

[stale]

This pull request has been closed due to continued inactivity. If you are interested in finishing the proposed changes, then feel free to re-open this pull request or open a new one.