DEV: Add plugin API to extend search results (PR #12966)

GitHub

Is this ready for review @udan11 ? I love that we can use async now, makes the change much easier to digest.

A tricky thing though is that we are in an inbetween world where some stuff is async and some stuff just returns promises, it is hard to decide when we are comfortable living with a mix vs a full cleanup.

I left this as a draft because I had the same question. Is it alright if we combine promises and async? Normally, I would have used only promise because I like being consistent and we did not have support for async. However, in this case using promises makes a mess of the whole code. Given how close we were to release, I did not want to risk it.

I will resume work on this, but the same question stands. I like a good experiment so I might just switch everything to async, but I am a bit nervous given that it is search, a vital part of Discourse.

Is it alright if we combine promises and async? Normally, I would have used only promise because I like being consistent and we did not have support for async. However, in this case using promises makes a mess of the whole code. Given how close we were to release, I did not want to risk it.

IMO it’s absolutely fine to combine them. Any method that returns a promise should work with async as log as you use exceptions to catch any errors. Over time I imagine more and more of our base will end up with async since it’s simpler. There are cases though where promises are clearer.

Given we are safe to combine, can you get this into a mergeable state @udan11 ? I would love to merge it in combined with the encrypt search which is awesome.