[tl:dr] There is a significant change in the apps-area which imho puts the success of the whole apps-thing at risk.
Trigger
As per a recent commit to the Apps-Framework, the architectural foundation of the apps was shaken.
Motivation - and why this is important
The principle of dependency inversion (aka “dependency injection”, “DI”) was a foundation when designing the interfaces of the apps: The framework shall be in control (e. g. of where an implementer of the framework can do what). Each method should get access only to features and functions which the framework intends.
This makes it possible for the framework to determine the app’s capabilities.
Defined app responsibilities
E. g. a rocket.chat-app implementing only IPreMessageSentExtend
and IPreMessageSentPrevent
has some defined capabilities: It may prevent that a message is sent and it may extend this message (e. g. add an attachment), but for sure it’s not going to change the body of a message.
In contrast, a bad words filter app implementing IPreMessageSentModify
obviously has to do exactly this.
Why is this important? Think about a company installing an app from an app store: You want to be sure what the app can do, similarly to installing an app on mobile: “this app accesses your microphone” - or in your browser: “this extension is going to read all your traffic”.
By providing a global helper, this mechanism is bypassed. Unless you parse the code (and believe me, parsing is not enough) you will never know what an app might do.
From an enterprise-perspective, such an app-store is a no-go.
Development quality
Seen from a developer’s perspective, it’s about ease-of-implementation, static validation at build time and testing. We could argue about the first one (which I’ll do in the next paragraph), but static validation of the artifact consumed and mockability which heavily influences testability is undoubtly better with DI.
But there is simplicity first…
Currently, the apps’ design features a lot of interfaces with long names and long parameter lists. True. But is this really the bad part? Is it really more comfortable to have a constructor injected (“app global”) dependency?
In my opinion, the initial approach is way better. Ok, of course, I’m biased (contributed parts of it).
Thus, let me quickly explain why @bradley.hilton and I went for this approach back then
Signature driven
Often, a developer starting with an environment looks at the API first - and may look at the documentation later. A fundamental question when implementing something which runs in a bigger context is “where do I put my code”. For this, interfaces and their methods are not only a guideline, but since they are available at runtime, they are the truth.
The interfaces in the Apps are crafted so that each interface defines the timepoint while the method defines the permissions which are granted to the logic. This naturally splits logic. Do I really need to execute the potentially expensive logic? => put some validation code into the check{Timepoint}
method. This does not only improve overall quality, it also allows the framework to optimize.
Comfort
But the signatures are long
Yes, they are - kind of. But therefore, you don’t even need to import a dependency - the dependency is injected, it’s just there, use it. How more comfortable could it be?
Verdict
Alright, I hope I presented the motivations properly and you now at least know what my opinion is. There may be other opinions as well - and many of them are surely equally valid.
My opinion is coming from a now 15 year long (you may call me “old” - yack!) background, long time working with business software and frameworks at an enterprise grade in various languages. It may not be the right one, but believe me it’s not a shot into the blue.
My verdict given that there are other arguments
Variant 1: Be consequent
By allowing each method to do whatever you want with the context they run in, you basically can get rid of the signatures. So if you go down this route, also kill the signatures (incompatible change) and in the same step also kill the separated check
and execute
methods. This will have the framework in control of when an app is executed, but will result in potentially more messy app implementations.
Variant 2: Provide a good documentation
If you got feedback about difficulties kickstarting an app, provide more
- Boilderplate
- Samples
- Code-near documentation
Imho, the later one is more promising when it comes to maintenance - and when it comes to perceiving Rocket.Chat as a tool really made for enterprises.