Fix Dataloader batching implementation in the Mongoose adapter

[nx-cloud[bot]]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 1991f3f01630e385a22d285de87bfa48fffdad99. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

---

✅ Successfully ran 5 targets

- [`nx run-many --target=e2e --all`](https://cloud.nx.app/runs/0budcGnwHh?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=lint --all`](https://cloud.nx.app/runs/02fcWOy6qI?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=build --all`](https://cloud.nx.app/runs/eN4zAk1Xd5?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=test --all`](https://cloud.nx.app/runs/PVsLIV4sxf?utm_source=pull-request&utm_medium=comment) - [`nx run workspace:version`](https://cloud.nx.app/runs/ZCvXtlJHzi?utm_source=pull-request&utm_medium=comment)

---

Sent with 💌 from NxCloud.

[mareksuscak]

Okay, this PR currently breaks nested collection pagination. I'll need to think about this a bit.

EDIT: Should be all resolved!

[smolinari]

Posting to watch.

Scott

[mareksuscak]

Here's a demo implementation that uses a patched version of the Mongoose adapter: https://github.com/mareksuscak/nestjs-query-bugs/pull/2

[smolinari]

Nope. I'm very happy Marek is taking this on.

Scott

[mareksuscak]

@smolinari @TriPSs I might have found a small bug, please hold on releasing a new version until I confirm. I'm testing this patch at a larger scale today.

[mareksuscak]

@smolinari @TriPSs it took a while to troubleshoot but I finally know what's happening. There's now a bug that affects certain versions of Mongoose - namely v6 and v7 because of incompatibility with class-transformer and specifically this bug which causes that values of type ObjectId that are not explicitly decorated with @ObjectId decorator are randomized during the conversion when their parent entity is being converted to a DTO. This was previously not an issue because findRelation and queryRelations were reloading the parent entity from the database here and here - I'd argue unnecessarily but what's more important is that the entity's fields were always correct despite the previous conversion. But now we only use the DTOs passed into findRelation and queryRelations and the DTOs all suffer from the class-transformer bug. I'm not sure what's the best way to fix this as it only affects certain versions of Mongoose. We're not seeing this behavior in tests because this repository already uses Mongoose v8 which isn't affected anymore.

EDIT: reported here: https://github.com/TriPSs/nestjs-query/issues/281

[smolinari]

I personally don't feel v6 or v7 of Mongoose should need to be supported. v8 is 8 months old. That is just me though.

@mareksuscak - I believe you mentioned only working in the Mongoose package, not the Typegoose. Is that correct? Are you also running the tests for the Typegoose package anyway? Is anything breaking?

Scott

[mareksuscak]

@smolinari I've only modified the ReferenceQueryService used by the mongoose adapter. Typegoose adapter is not affected by the changes in this PR whatsoever but the Dataloader is still broken in the Typegoose adapter. Because I've never used Typegoose before, I am not intending to port this change to that adapter but anyone should be able to fix it based on the changes in this PR.

[smolinari]

Ok. If I find time, I'll see if the changes can also be worked into the Typegoose package, and if they are needed. I bet they are.

Scott

[mareksuscak]

@smolinari I did take a quick look and structurally wise, Mongoose and Typegoose adapters are the same so I assume one was based on the other so it's safe to assume that Typegoose needs this patch as well.

[smolinari]

@mareksuscak - Yeah, for sure. I didn't write the Typegoose package, but did some work on it in the past. I'll have a look tomorrow, more than likely.

Scott