Skip to content

Conversation

@alecgibson
Copy link
Collaborator

mongodb@5 drops support for callbacks, which breaks this library, which is all written with callbacks.

This is a BREAKING change which drops callback support from this library as well, and fully embraces promises through async/await syntax in both the library code and test code.

This allows us to support both mongodb@4 and mongodb@5.

@alecgibson alecgibson requested a review from dawidreedsy March 1, 2023 15:50
mongodb-queue.js Outdated
})
})
Queue.prototype.createIndexes = async function() {
var indexname = await this.col.createIndex({ deleted : 1, visible : 1 })
Copy link

@dawidreedsy dawidreedsy Mar 1, 2023

Choose a reason for hiding this comment

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

Any reason we use var

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mostly to stay consistent with what was there before, but maybe I can upgrade this too while we're here

mongodb-queue.js Outdated
Queue.prototype.createIndexes = async function() {
var indexname = await this.col.createIndex({ deleted : 1, visible : 1 })
await this.col.createIndex({ack: 1}, {unique: true, sparse: true})
await this.col.createIndex({deleted: 1}, {sparse: true})

Choose a reason for hiding this comment

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

Shouldn't we have just

const [indexname] = await Promise.all([
  this.col.createIndex({ deleted : 1, visible : 1 }),
  this.col.createIndex({ack: 1}, {unique: true, sparse: true})
  this.col.createIndex({deleted: 1}, {sparse: true})
]) 

mongodb-queue.js Outdated
// So:
// 1) add this message to the deadQueue
// 2) ack this message from the regular queue
// 3) call ourthis to return a new message (if exists)

Choose a reason for hiding this comment

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

ourself not ourthis

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ha bad find and replace 😅

mongodb-queue.js Outdated
Comment on lines 118 to 120
if (this.deadQueue) {
// check the tries
if (msg.tries > this.maxRetries) {

Choose a reason for hiding this comment

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

Suggested change
if (this.deadQueue) {
// check the tries
if (msg.tries > this.maxRetries) {
if (this.deadQueue && msg.tries > this.maxRetries) {

[`mongodb@5`][1] drops support for callbacks, which breaks this library,
which is all written with callbacks.

This is a **BREAKING** change which drops callback support from this
library as well, and fully embraces promises through `async`/`await`
syntax in both the library code and test code.

This allows us to support both `mongodb@4` and `mongodb@5`.

[1]: https://github.com/mongodb/node-mongodb-native/releases/tag/v5.0.0
@alecgibson alecgibson merged commit 05d4a6b into main Mar 2, 2023
@alecgibson alecgibson deleted the promisify branch March 2, 2023 09:19
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.

3 participants