GRAFX-3507 Support multiple event handlers

[pietervp]

Problem

When using the SDK, we expose a lot of event handlers to the integrator. For example, we can execute a callback whenever the selected frame changed. There is one big (not obvious) caveat, the handler can only be defined once. This means that each event can only be handled by 1 consumer. This is especially bad when using studio-ui, and you want to tap into some of these event yourself as integrator. Once you do that, you probably have broken the studio-ui interface by overriding it’s subscription.

Proposal

• Deprecate all properties in SDK.config.on and move logic to SDK.config.events where each property on will no longer be a simple callback, but a class to manage subscriptions.
• Remain backwards compatible, the events properties will by default use the legacy SDK.config.on callback when the event is triggered. We also keep supporting overriding that handler.
• Follow Up: Workspace and studio-ui should move to the events based approach when registering event handlers. (first we need SDK PR)

Code Highlights

• The SDK object constructor remains intact (ConfigType does not include events property). Although the SDK constructor starts with wrapping this config object to a RuntimeConfigType which adds the events. https://github.com/chili-publish/studio-sdk/pull/482/files#diff-04689b788dad65b6ce4b43e7344d3b854b1a95ad2c8692c6ece28ccd7d9a96b4R93
• EventHelper.ensureSubscriptions => this wraps the ConfigType to a RuntimeConfigType, automatically registering the legacy handlers already defined in ConfigType. https://github.com/chili-publish/studio-sdk/pull/482/files#diff-8cc56e41f48f0a52e82e4c279dba504747c37e1c76bf0d79b802505878eb56a7R69
• EventSubscription => controls the multiple callbacks (both registering and execution) https://github.com/chili-publish/studio-sdk/pull/482/files#diff-8cc56e41f48f0a52e82e4c279dba504747c37e1c76bf0d79b802505878eb56a7R23

Open Questions

• Naming: do we keep SDK.config.events or are there better naming options?
• Naming: do we keep SDK.config.handlers to indicate callbacks that require to return a result

[github-actions]

Coverage Report

Coverage report can be checked at https://stgrafxstudiodevpublic.blob.core.windows.net/sdk/coverage/482/coverage.html

Use PR sdk package

Tarball can be downloaded from https://stgrafxstudiodevpublic.blob.core.windows.net/sdk/dev-packages/482/studio-sdk.tgz

To use in local project, change package.json to use local tarball

"dependencies": {
    "@chili-publish/studio-sdk": "https://stgrafxstudiodevpublic.blob.core.windows.net/sdk/dev-packages/482/studio-sdk.tgz"
}

[github-actions]

Unit Test Results

    1 files    40 suites   44s ⏱️
439 tests 439 ✔️ 0 💤 0 ❌
444 runs  444 ✔️ 0 💤 0 ❌

Results for commit 65a5dca.

♻️ This comment has been updated with latest results.

[Matthiee]

I like this idea 💯

I do have some concerns regarding the naming.

[pietervp]

After receiving feedback from @Matthiee and discussing offline with @brapoprod :

• Added a configuration option to register a logger in the SDK (https://github.com/chili-publish/studio-sdk/pull/482/files#diff-a5080035ef7a9f26c66428b08da906fd0d75994ea367df52cae7e305db2c33d4R33-R36)
  • This allows us to avoid console.log in the code, and gives integrators a way to get to errors / warning / info generated by us.
  • This is needed because of the async nature of the event callbacks, there is no good way for an integration to handle these exceptions by default.
• Every callback registration can define how errors during callback execution should be handled. (log / throw / remove) (https://github.com/chili-publish/studio-sdk/pull/482/files#diff-a5080035ef7a9f26c66428b08da906fd0d75994ea367df52cae7e305db2c33d4R57-R72 and https://github.com/chili-publish/studio-sdk/pull/482/files#diff-9b49133c3aadcf3d8efbb67c197a89e4e8009cc91ed27042842f342818dc2d41R44-R58)
  • This allows each subscription to define its own rules on how to handle errors during callback
• On the feedback about the registration keys, we think keeping it a string is cleaner and simpler then requiring an object which need to be tracked by the integrator. Strings can be easily managed on integration side, no leaks, etc
• Other naming changes were applied