Rocket.Chat.Apps-engine icon indicating copy to clipboard operation
Rocket.Chat.Apps-engine copied to clipboard

Order of the Apps' lifecycle hooks

Open cuonghuunguyen opened this issue 3 years ago • 9 comments

Different apps can register the same lifecycle hooks. In most cases, the order of execution does not matter, but sometimes it causes unpredictable results.

For example: If we have 2 Apps, 1 for Auto Translation and 1 for Badword filtering. Both use IPreMessageSent hook.

  • If the Bad word filter go first, the result will be correct
  • If Translation App go first, the Badword may not filter the bad words from the translation languages

There are many possible cases that can happen. I recommend we can have the same priority system for the lifecycle hooks just like the rocket chat callbacks. The values can be "LOW", "HIGH" or "MEDIUM"

cuonghuunguyen avatar Nov 24 '21 09:11 cuonghuunguyen

So, with an order like that, what would be executed first?

Would the lowest be executed first since it's not important and the result can be overwrote? Or would the highest be executed first even though the result could be overwrote?

graywolf336 avatar Nov 29 '21 17:11 graywolf336

Wow, that's a very good question. I didn't think about that. My idea is that just keep the execution order just like callbacks: HIGH -> MEDIUM -> LOW. The App will decide when the hook should run. Whether it must touch the data first or wait for all the other hooks to be done then finish up things

cuonghuunguyen avatar Nov 30 '21 03:11 cuonghuunguyen

I am thinking we could have five priority levels for the interfaces which destructively mutate the items.

  1. Highest
  2. High
  3. Medium
  4. Low
  5. Lowest

They would be executed highest importance to least important, meaning the highest priority would be executed first and the lowest priority would be executed last. If an App simply needs to listen for an event, then it should use the post events.

Having this would be a very good usage of decorators for the interface methods.

graywolf336 avatar Nov 30 '21 04:11 graywolf336

yes, decorators are exactly what I thought of when coming up to this idea :D

cuonghuunguyen avatar Nov 30 '21 04:11 cuonghuunguyen

@cuonghuunguyen I have a logical question for you.

Let's say two apps have their priority set at Highest and another two have their priority set to Lowest, how do we determine which one goes first of those?

graywolf336 avatar Dec 01 '21 13:12 graywolf336

In that case just keep the old order (aka non-predictable). Sounds not proper, but the z-index in HTML behaves the same, right?

cuonghuunguyen avatar Dec 01 '21 14:12 cuonghuunguyen

Yeah, that's exactly what I was thinking too. Just wanted to see what you had in mind :)

graywolf336 avatar Dec 01 '21 14:12 graywolf336

@cuonghuunguyen would love to get your opinion on the PR above :)

graywolf336 avatar Dec 07 '21 21:12 graywolf336

I'll check it after work, thanks for your effort on it

cuonghuunguyen avatar Dec 08 '21 02:12 cuonghuunguyen