Performance: change detection is unnecessarily triggered

[adhirajsinghchauhan]

What is the current behavior? The editor automatically forwards all events to the corresponding EventEmitters: https://github.com/tinymce/tinymce-angular/blob/54a841030f493a85ab1ff4d8795a347f3d6af048/tinymce-angular-component/src/main/ts/utils/Utils.ts#L13-L18 The problem with that is, a lot of the events defined by validEvents are redundant + don't need to be run inside Angular.

https://github.com/tinymce/tinymce-angular/blob/54a841030f493a85ab1ff4d8795a347f3d6af048/tinymce-angular-component/src/main/ts/editor/Events.ts#L111-L117

• onMouseDown & onMouseUp are redundancies for onClick. In the demo, click inside the editor once, and see how CD is triggered 4 times for the same action. Yes, onClick does not cover all events captured by onMouseDown or onMouseUp, but for the most part they're still pretty redundant
• Why CD needs to be triggered for enter, leave, move, out, and over is beyond me

Demo codesandbox.io

What is the expected behavior? If CD is run multiple times for no reason, Angular needs to recheck views. In a complex project, this results in massive performance slowdowns, which multiplies if the page has multiple active editors.

• Ideal: change detection should be triggered only when necessary, and an effort should be made to reduce the possibility of duplicate CD runs as much as possible. For example, if the client component isn't using any of the non-crucial event outputs, then there's no point to fire events for majority of them.
• Minimum: validEvents should be bisected into two different arrays: validEvents and eventsToFireInsideAngular

Which versions of TinyMCE/TinyMCE-Angular, and which browser / OS are affected by this issue?

        TinyMCE: 5.4.2
TinyMCE-Angular: 4.1.0
     Browser/OS: Chrome 85.0.4183.83/Windows 10.0.19041.450

Did this work in previous versions of TinyMCE or TinyMCE-Angular? NA

[jscasca]

Hi @adhirajsinghchauhan

I understand that you are not using any of those events but the reason the wrapper forward those are to allow users to handle different events in their own way. Change detection was not the concern when passing the events, it was for the user to address particular use cases that you might not have.

In an effort to address this "massive" performance issue perhaps we can add a new property to either black-list or white-list the events being bound so that users can have more control over this.

Let me know your thoughts since I don't clearly see the objective behind bisecting the validEvents array

[adhirajsinghchauhan]

perhaps we can add a new property to either black-list or white-list the events being bound

If this is how it will work, then yes it's what I outlined in the "ideal" behaviour:

• Allowlist will be used instead of validEvents (if passed, otherwise default to validEvents)
• Blocklist is used simply to filter out from whatever the allowlist is

Let me know if the TinyMCE team will work on this themselves, or if a PR is required.

[jscasca]

Hi @adhirajsinghchauhan

In v4.2.0, we added the properties allowedEvents and ignoreEvents to whitelist or blacklist events from emitting on the angular component. We will document this in the Integration docs shortly.

Both properties take a comma separated string of selected events to allow/ignore. E.G.:

  allowedEvents="onMouseEnter,onMouseLeave"
  ignoreEvents="onMouseLeave"
></editor>```

[adhirajsinghchauhan]

I ended up setting allowedEvents="" and responsiveness surely improved. See this demo on codesandbox.io - the editor still functions as expected, but without the extraneous CD triggers.

@jscasca It's great this is now resolved, but for the sake of completeness I'd recommend changing default behaviour of the editor component to only emit events if the user has explicitly subscribed to them. Example, if the user does something like this:

<editor (click)="..." (mouseover)="..."></editor>

Only onClick and onMouseOver should be emitted. Right now the default behaviour is still to emit all events regardless of whether or not the user has subscribed, which might lead unfamiliar developers to wonder why are things not performing as expected.

See this sort-of relevant perf commit in Angular Components: https://github.com/angular/components/commit/f7e0f31. Even though that one is at a much smaller scale, it's still considered a performance improvement. Imagine how much of an impact would tinymce-angular's default behaviour have in comparison.

There are many ways to check if the user has subscribed: event.observers.length > 0, custom host binding (with setters), etc.

Thanks for resolving this quickly otherwise 😄