Provide Icinga to ipl/web behaviors
#5238
Conversation
sukhwinder33445
force-pushed
the
provide-icinga-to-ipl-behaviors
branch
from
6cbb1ee
to
80eb42d
Compare
| @@ -86,6 +86,8 @@ | |||
| return false; | |||
| } | |||
|
|
|||
| define('Icinga', [], () => Icinga); | |||
There was a problem hiding this comment.
I struggle to find a suitable location when to define Icinga. This is too early. At the end, it's too late. Right before the behavior initialization, it's somewhat arbitrary...
So, looking at this from another angle, I'd propose to solve this differently:
The linked PR in ipl-web only requires this because it needs Icinga.EventListener. So why don't we define just this class? In eventlistener.js, at the end of the factory: define("icinga/legacy-app/EventListener", [], () => EventListener)
Why icinga/legacy-app/? icinga/ is the global namespace. legacy-app/ is the library name. The reason for legacy- is that I don't want to use up just app/ at this time.
| @@ -238,7 +238,7 @@ public static function optimizeDefine($js, $filePath, $basePath, $packageName) | |||
| try { | |||
| $dependencies = $match[2] ? Json::decode($match[2]) : []; | |||
| foreach ($dependencies as &$dependencyName) { | |||
| if ($dependencyName === 'exports') { | |||
| if ($dependencyName === 'exports' || $dependencyName === 'Icinga') { | |||
There was a problem hiding this comment.
just check whether the name already begins with icinga/legacy-app/.
No description provided.