Remove the bluebird dependency from everywhere 🔥

[ErisDS]

Now that there are native promises in Node.js and with async/await, bluebird really is a blast from the past. However, it's embedded pretty hard across our dependencies and some of our code uses catch predicates, which means we have to be careful about the order in which we rip bluebird out, otherwise we could break our error handling in weird and unexpected ways.

Here's some notes on what we need to do, most importantly starting with finding everywhere we depend on catch predicates:

Native:

• Promise.all
• Promise.then
• Promise.reject
• Promise.resolve

Bluebird

• catch predicates -> replace with normal if statements
• Promise.promisify -> require('util').promisify
• Promise.props -> Promise.all example
• Promise.join -> Promise.all
• Promise.each -> Promise.all
• Promise.map -> Promise.all
• Promise.filter -> for and async/await
• Promise.mapSeries -> use our sequence util (which needs an implementation of reduce, but is our only usage of it)
• Promise.delay -> promisify getTimeout? Or get rid?

---

Refactor Approach

This refactor needs to be done in two phases:

1. ✅ Remove all the catch predicates

💡 The best way to find catch predicates is with this regex: /\.catch\([^)]+,/.

2. Remove bluebird from everywhere else:

• Ghost monorepo - bluebird is required 117 times 😱
• framework - bluebird required 5 times, but these are some of the most critical usages
• SDK - just one usage, easy win!
• gscan - 3 usages
• knex-migrator - 3 usages

Why did we have to do catch predicates first?

Catch predicates result in error handling that assume the Promise they are handling is a Bluebird promise. If that isn't the case, Ghost will error, and quite possibly crash.

Our code coverage isn't quite good enough to be certain that all the unhappy paths are tested and we'll find these, and it's too much cognitive load for anyone to figure out if removing Bluebird is safe whilst there are still catch predicates in the codebase.

---

How to work on this issue

The slow and steady approach to refactoring is the only thing that really works. Branches should be very short lived (<1 day if possible). Therefore, please do not attempt to do this entire refactor in a single PR. It is not possible for us to review!

Instead, please find a single file with many references, or a small subfolder of the codebase with a handful of usages, make the changes, and submit a PR. Then repeat.

Please follow our contribution guidelines as closely as you can, particularly being sure to reference this issue in your commit. Refactor commits should not use emojis as they are not user-facing changes.

Note: This refactor has almost no user facing impact and (hopefully) no tests should break or change.

A reference refactor commit can be found here: https://github.com/TryGhost/Ghost/commit/10aad8db7e12b6d08089cd7beee3ea9df64b3adb

Thank you 🙏 ❤️ 🙏

[Emmanuel-Melon]

I can work with this, I had a similar task at my previous workplace 😁

[ErisDS]

I've updated the OP here to be clearer that we need to remove catch predicates first, and why as there have been a couple of PRs that missed that detail.

[Emmanuel-Melon]

I've updated the OP here to be clearer that we need to remove catch predicates first, and why as there have been a couple of PRs that missed that detail.

Apologies for the delay, I've been away for a bit. Resuming working on this task today.

[ErisDS]

Hey all 👋 this issue was blocked on needing to remove the catch predicates first, so I've just merged a PR that removes all of them.

I've also updated the issue with the current state of things.

I'll go through the couple of blocked open PRs that remove some bits of bluebird asap. In the meantime, it'd be great to get some momentum on this and get rid of bluebird finally!

[danoseun]

@ErisDS what's left of this issue to be fixed?

[daniellockyer]

@danoseun There are still plenty of places where we're using Bluebird 🙂 https://github.com/search?q=repo%3ATryGhost%2FGhost%20bluebird&type=code

[vershwal]

With these three PRs for TryGhost/gscan Bluebird dependency can be completely removed from the gscan module 🤞 🎉

[chairulakmal]

Hello everyone,

I'm a new contributor here! I've created pull request #17831 to remove Bluebird from the ghost/core codebase. I found that only the raw-knex module was still using Bluebird, so I ~~refactored its code~~ made a minimal change to avoid using Bluebird Promise.props. Additionally, I removed Bluebird from ghost/core's package.json since no other part of the codebase needs it.

~~It's worth noting that the changes to the raw-knex plugin might not be fully covered by the current tests, as it seems only core/server/services/url/Resources uses raw-knex. I didn't realize there's a regression test specifically for core and my changes result in error. Will fix this today or tomorrow.~~

I ran yarn test:all for core and it yielded similar results to those of the current main branch. Your feedback would be greatly appreciated – looking forward to your thoughts on these updates!

Thanks, kai

[markstos]

I've submitted a PR to the nodemailer-mailgun-transport project to resolve this issue in that part of the dependency stack: https://github.com/orliesaurus/nodemailer-mailgun-transport/pull/126

[markstos]

I've reviewed the status of this, and after the open PRs linked above are merged, it appears that perhaps than can be closed after lerna is used for final new package releases that contain the fix and and dependencies on those packages are updated one more time.

• 🚧 Ghost monorepo - a yarn.lock mention remains which should get resolved per above.
• ✅ framework
• ✅ SDK
• ✅ gscan
• ✅ knex-migrator

[07tAnYa]

kindly assign this to me , I can work on this

[markstos]

@07tAnYa Per my comment above, there is really nothing left to do. What change are you planning to make?