[Architecture] One MutationObserver should be registered, instead of one per component

[henrykuijpers]

Every core component could have a clientlibrary associated with it, that should contain code to instantiate the JS/dynamic part of the component.

I noticed that a MutationObserver pattern is introduced to handle the situation where a component is being edited through a dialog (and then saved), or when a component is inserted dynamically for whatever reason.

This pattern is a pretty good solution: It nicely fulfills all the use-cases where a component instantiation is required.

However, I have seen that per component a MutationObserver is registered, instead of just one MutationObserver that delegates the calls (for example through events?) to the various components. This has a performance impact: The more components we create that leverage this pattern, the more MutationObservers (that receive the exact same events, but handle different elements) get triggered!

So I propose to create a generic MutationObserver that can get information about which elements are added (and removed?!) from the page, allowing their components to handle those cases, by subscribing to events for them.

WDYT? I'd like to discuss a little bit about the implementation that we'd like to create here. I can provide a PR, together with my colleague, but then we first need to get the solution clear. :)

FYI: I'm talking specifically about the pattern being used here: https://github.com/adobe/aem-core-wcm-components/blob/657d593626c665042f675c3c7a579b3ba0db45ca/content/src/content/jcr_root/apps/core/wcm/components/form/text/v2/text/clientlibs/site/js/text.js#L131

[jckautzmann]

@henrykuijpers Thx for your feedback, much appreciated! First I would like to understand the motivation for improving the current mechanism. IIUC it is about improving the performance. Have you noticed a negative impact with an existing scenario or with a customer installation? Would you have numbers that you could share? Or a way how to measure it? If we decide to move ahead with a new logic, it should be part of a bigger refactoring to define a client-side component API that can be reused between our components and even be used by customers to write their own. It would need to be planned and prioritized accordingly and would probably not happen within the next 6 months.

[henrykuijpers]

Thanks for your reply.

h1. Performance I don't have any measures regarding performance, but since this would be triggered mainly in the authoring environment, but also when content is dynamically inserted in the DOM (i.e. on a website where content is loaded through ajax), these MutationObservers (especially with their current configuration) get triggered a lot.

It should definitely be a whole lot better to let only one MutationObserver get registered and let it handle all it's listeners in an efficient way.

The browsers nowadays are probably fast enough, but imagine having a lot more components leverage this feature in the future, and also imagine a lot of custom components defined in website implementations.

h1. Reusability Reusability with the current solution is another issue: while it is very easy to create these MutationObservers from code, it could be that we want to enhance things in the future. It would be very nice if those enhancements could immediately apply to custom code, instead of having to copy paste new features in our custom code.

h1. Avoid repeating The current code is repeated 5x (with only the selector checking using a different regex) right now. Not a good example of code reuse.

Btw, I see a bug: https://github.com/adobe/aem-core-wcm-components/blob/657d593626c665042f675c3c7a579b3ba0db45ca/content/src/content/jcr_root/apps/core/wcm/components/form/text/v2/text/clientlibs/site/js/text.js#L166 This line is accidentally executing the event listener already, assigning "undefined" as an event listener.

[jckautzmann]

@henrykuijpers I agree that it would make sense to refactor the MutationObserver logic, you listed valid reasons to do it. We see it as a broader effort to define a client-side component API. Now if you have a solution in mind that would move us in the right direction, please feel free to discuss or contribute here.

Could you please explain in more details what issue you see in line#166? This line registers an event listener to execute onDocumentReady() after the DOM has loaded.

[henrykuijpers]

The problem with line 166 is that it indeed registers an eventlistener, but instead of passing a reference to that function, it actually executes the function and passes the return value (probably undefined) as the value of the event listener.

For the MutationObserver issue, I'll discuss this week with my colleague regarding a good approach.

[jckautzmann]

Thx for clarifying the issue in line 166. I created an issue and a PR to fix it: https://github.com/adobe/aem-core-wcm-components/issues/461 https://github.com/adobe/aem-core-wcm-components/pull/462

[jckautzmann]

(CQ-4264381)