Skip to content

BREAKING CHANGE: rely on transforms over middleware to apply virtuals and add findOneAndReplace support#76

Merged
vkarpov15 merged 8 commits intomasterfrom
vkarpov15/gh-75
May 14, 2025
Merged

BREAKING CHANGE: rely on transforms over middleware to apply virtuals and add findOneAndReplace support#76
vkarpov15 merged 8 commits intomasterfrom
vkarpov15/gh-75

Conversation

@vkarpov15
Copy link
Copy Markdown
Member

Fix #75

Summary

#75 points out a significant issue in this plugin's functionality: Query transform()s run before middleware, so transform that changes the query shape causes this plugin to break. Fix the issue by making this plugin unshift() a transform to the front of the transforms array in pre(), which will ensure this plugin's transform will run before any user-specified transforms.

This change will require a 2.0.0 release because it may break some existing code.

@hasezoey can you please take a look at this one?

Examples

@vkarpov15 vkarpov15 requested a review from Copilot May 1, 2025 14:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses a critical issue where query transforms run before middleware, which has been causing the plugin to misbehave. The changes include:

  • Adding support for the findOneAndReplace operation in tests.
  • Updating the middleware to unshift the transform across multiple query operations (including findOneAndReplace) so it executes prior to user-supplied transforms.
  • Removing redundant post hooks in favor of the new pre hook approach.

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
test/index.test.js Added a new test case for the findOneAndReplace operation.
index.js Modified middleware to switch to an unshifted pre hook for multiple query types.
Files not reviewed (1)
  • package.json: Language not supported
Comments suppressed due to low confidence (1)

test/index.test.js:100

  • [nitpick] It may be beneficial to extend the test case for findOneAndReplace by considering edge cases, such as scenarios where an empty dataset is returned, to ensure the transform behavior remains consistent.
return model.findOneAndReplace({ _id: docId }, { ...baseObj }).lean(leanOptions).exec();

Comment thread index.js Outdated
Copy link
Copy Markdown
Member

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though i have not much experience with this plugin.

Comment thread test/index.test.js Outdated
vkarpov15 and others added 2 commits May 13, 2025 13:53
@vkarpov15 vkarpov15 merged commit 913d105 into master May 14, 2025
10 checks passed
@vkarpov15 vkarpov15 deleted the vkarpov15/gh-75 branch May 14, 2025 16:53
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.

Unexpected result when executing lean({ virtuals: true }) with a transform function in a query

3 participants