Skip to content

Commit

Permalink
fix: proposal to keep before.find sync
Browse files Browse the repository at this point in the history
  • Loading branch information
vparpoil committed Nov 3, 2024
1 parent 5761671 commit 4cec3d1
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 36 deletions.
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,9 @@ test.before.find(function (userId, selector, options) {
});
```

__Important:__ This hook does not get called for `after.update` hooks (see https://github.com/Meteor-Community-Packages/meteor-collection-hooks/pull/297).
__Important:__
- The function used as `before.find` hook cannot be async
- This hook does not get called for `after.update` hooks (see https://github.com/Meteor-Community-Packages/meteor-collection-hooks/pull/297).

--------------------------------------------------------------------------------

Expand Down
10 changes: 3 additions & 7 deletions find.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ CollectionHooks.defineWrapper('find', function (userId, _super, instance, hooks,
hooks.before.forEach(hook => {
if (!hook.hook.constructor.name.includes('Async')) {
hook.hook.call(this, userId, selector, options)
} else {
throw new Error('Cannot use async function as before.find hook')
}
})

Expand All @@ -27,13 +29,7 @@ CollectionHooks.defineWrapper('find', function (userId, _super, instance, hooks,
if (cursor[method]) {
const originalMethod = cursor[method]
cursor[method] = async function (...args) {
// Apply asynchronous before hooks
for (const hook of hooks.before) {
if (hook.hook.constructor.name.includes('Async')) {
await hook.hook.call(this, userId, selector, options)
}
}

// Do not try to apply asynchronous before hooks here because they act on the cursor which is already defined
const result = await originalMethod.apply(this, args)

// Apply after hooks
Expand Down
3 changes: 2 additions & 1 deletion tests/find.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ Tinytest.addAsync('find - selector should be {} when called without arguments',
const collection = new Mongo.Collection(null)

let findSelector = null
collection.before.find(async function (userId, selector, options) {
collection.before.find(function (userId, selector, options) {
findSelector = selector
return true
})

// hooks won't be triggered on find() alone, we must call fetchAsync()
Expand Down
33 changes: 6 additions & 27 deletions tests/find_after_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,28 +37,9 @@ Tinytest.addAsync('issue #296 - after insert hook always finds all inserted', as
return true
})

let beforeCalled = false
collection.before.insert(() => {
beforeCalled = true
})

let afterCalled = false
collection.after.insert(() => {
afterCalled = true
})

await collection.insertAsync({ removedAt: new Date() })

test.equal(beforeCalled, true)
test.equal(afterCalled, true)
})

Tinytest.addAsync('async find hook - after insert hook always finds all inserted', async function (test, next) {
const collection = new Mongo.Collection(null)

collection.before.find(async (userId, selector) => {
await new Promise(resolve => setTimeout(resolve, 10)) // Simulate async operation
collection.before.findOne((userId, selector) => {
selector.removedAt = { $exists: false }

return true
})

Expand All @@ -74,15 +55,13 @@ Tinytest.addAsync('async find hook - after insert hook always finds all inserted

await collection.insertAsync({ removedAt: new Date() })

// Wait for a short time to ensure async find hook has completed
await new Promise(resolve => setTimeout(resolve, 20))

test.equal(beforeCalled, true)
test.equal(afterCalled, true)

// Verify that the find hook is working
const result = await collection.findOneAsync({})
test.isUndefined(result, 'Document should not be found due to async find hook')
const result = await collection.find({}).fetchAsync()
test.equal(result.length, 0, 'Document should not be found due to before find hook modifying cursor')

const result2 = await collection.findOneAsync({})
test.isUndefined(result2, 'Document should not be found due to before findOne hook modifying cursor')
next()
})

0 comments on commit 4cec3d1

Please sign in to comment.