quill: Performance degradation with DOM mutation events

The usage of DOM mutation events in Quill (specifically, DOMNodeInserted from c4d897838a463d8396c9e93ec9faf5e9bca5566d) significantly degrades the performance of DOM node insertions and removals on the entire document. Additionally, the API is deprecated.

More information about the performance effects of DOM mutation events can be found on MDN: https://developer.mozilla.org/en-US/docs/Web/Guide/Events/Mutation_events In particular:

Adding DOM mutation listeners to a document profoundly degrades the performance of further DOM modifications to that document (making them 1.5 - 7 times slower!). Moreover, removing the listeners does not reverse the damage.

The listener was originally added to address #1437 — I’m not sure why adding mutation listeners would fix that bug, but it would be nice to find another solution.

I have tried substituting the mutation event listener with a MutationObserver, but using MutationObserver doesn’t fix the bug.

Steps for Reproduction

Visit https://quilljs.com
Run the following code in console:

document.body.addEventListener('click', () => {
    console.time();
    for (let i = 0; i < 1000; i++) {
        const div = document.createElement('div');
        document.body.appendChild(div);
    }
    console.timeEnd();
});

Click on the page and observe timing information
Run the same code on another site, to compare

Expected behavior: DOM node insertions are fast

Actual behavior: DOM node insertions are painfully slow

Platforms: This seems to affect all major browsers and platforms

Version: >= 1.2.5, when the listener was added

About this issue

Original URL
State: closed
Created 7 years ago
Reactions: 28
Comments: 35 (3 by maintainers)

Most upvoted comments

Any plans to do something about this?

+28

echan00 on Jul 29, 2019

Any update?

+18

vinzzzb on Apr 30, 2020

Most changes planned for version 2.0 have already been implemented in develop branch. I’m going to update the documentation/website for version 2.0. Don’t have a timeline to share but I hope we can release a RC version in two months.

+16

luin on Aug 14, 2023

Sorry for the late response. The listener has already been removed on develop branch and we are working on a new release for it. Closing for now.

+14

luin on Jan 5, 2023

any updates here to fix this ???

+12

21pg on Jun 17, 2021

Any news on when a new release will be available with this fix @luin ?

+11

Danielvandervelden on Aug 1, 2023

Some news here?

+11

rafa-suagu on Apr 14, 2019

Is there any update on this?

+10

nagavadla on Apr 22, 2022

@kevinhend @jhchen I propose we add a new boolean flag to the Quill constructor called compatibilityMode. When set to false, it bypasses adding the DOMNodeInserted listener. To preserve backwards-compatibility, the property should default to true, and require the developer to explicitly set it to false to regain that extra performance.

In the Quill source code:

if (config.compatibilityMode) {
  this.domNode.addEventListener('DOMNodeInserted', function() {});
}

When using Quill:

var editor = new Quill(document.querySelector('.editor'), {
  compatibilityMode: false,
  // ...
});

What do you think? If you’d like, I can submit a PR with my proposed implementation.

caleb531 on Nov 5, 2018

It would great of we can address this, I get this as warning:

[Violation] Added synchronous DOM mutation listener to a 'DOMNodeInserted' event. Consider using MutationObserver to make the page more responsive.

o0x2a on Oct 26, 2020

This was removed in 2018 but never made it into version 1.3.7 that is being served on NPM.

Removal PR in 2018: https://github.com/quilljs/quill/commit/41a60fbf7cc9d23f4d87c4a88c42bb56157e3432#diff-5f9943a0ada23637672023452b5f7e4c7d2a715487446dd346e4ebcd7c9d9ccbL16

1.3.7 that is being served on NPM (2017): https://github.com/quilljs/quill/blob/1.3.7/blots/scroll.js#L25

You can use develop branch instead of NPM package: https://github.com/quilljs/quill/blob/develop/blots/scroll.ts#L42 Or you can manually patch the library locally and remove the offending line.

abacaj on Oct 18, 2022

🤔 I think the difference is a lot more obvious when running the profiler as well – though that could very well be the overhead associated with profiling events.

However if I use finer timing tools (e.g. performance.mark) to time only the appendChild, I see a slowdown of 2 - 3x

albertxing on Oct 16, 2017

Sorry for the late response. The listener has already been removed on develop branch and we are working on a new release for it. Closing for now.

Epic comeback after PR open for 6 years 😃

marcbrame on Sep 20, 2023

Hello, I just started using quill and encountered the same warning. Any idea when this might get fixed?

MelkorNemesis on Aug 14, 2023

is there any solution?

mhraihan on Oct 18, 2022

I think @caleb531’s solution is the one to go for here 👍

For everybody else that is facing this and wants to fix this in the meanwhile: I deleted the line

this.domNode.addEventListener('dragstart', e => this.handleDragStart(e));

everywhere I could find it in the quill folder in the node_modules of my project and then used https://github.com/ds300/patch-package to create a patch of these changes. The only files you really need to touch are ./dist/quill.js and ./dist/quill.min.js, depending on what you use in your project.

EDIT: ironically, if you want to create a patch for quill using latest patch-package (6.2.2), you’ll first have to patch patch-package to be able to create the quill patch using this fix: https://github.com/ds300/patch-package/issues/166.

Timebutt on Apr 19, 2020