react-quill icon indicating copy to clipboard operation
react-quill copied to clipboard

Crash when more than one instance is controlled by the same state

Open Emilios1995 opened this issue 5 years ago • 2 comments

Reproducible example in CodeSandbox

This only happens when the initial value is an HTML string that has whitespace between tags, and in need of other similar cleaning changes. When react-quill fires the first update which includes the cleaning changes, an infinite loop starts until React crashes with:

Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.

I understand that at first both components are trying to update themselves, an in turn trigger an update on the other component, but the onChange callback should stop being called once the sanitization has been applied.

For anyone encountering this, in the same Code Sandbox linked above, there are two files Fix1.js and Fix2.js with a couple of work arounds.

React-Quill version

2.0.0-beta.2

This doesn't happen in v1

Emilios1995 avatar May 13 '20 19:05 Emilios1995

This is fixed in my fork where a beforeChange function is introduced which allows to validate what qualifies as a change, it's not just whitespaces, but quill will often reoder the HTML attributes; it's very hard given how quill itself works, however all these changes have a source that is valued as "api", so it can be relatively cheap to check for these value changes with the beforeChange function.

public beforeChange(value: string, delta: any, source: string, editor: any) {
    if (source === "api") {
      const customElem = document.createElement("div");
      customElem.innerHTML = value;
      const customElem2 = document.createElement("div");
      const editorValue = this.props.currentValue as string || "";
      customElem2.innerHTML = editorValue;

      return !customElem.isEqualNode(customElem2);
    }
    return true;
  }

onzag avatar Jun 23 '20 19:06 onzag

Because styles can be rerranged as well the isEqualNode was not good enough check. I ended up with this from Stack Overflow plus some tweaking. isEqualElement will even check if two styles are rearranged.

    /**
     * Get the attribute names for an element and sorts them alpha for comparison
     */
    static getAttributeNames(node) {
        let index, rv, attrs;
    
        rv = [];
        attrs = node.attributes;
        for (index = 0; index < attrs.length; ++index) {
            rv.push(attrs[index].nodeName);
        }
        rv.sort();
        return rv;
    }
    
    /**
     * Compare two elements for equality.  Even will compare if the style element
     * is out of order for example:
     *
     * elem1 = style="color: red; font-size: 28px"
     * elem2 = style="font-size: 28px; color: red"
     */
    static isEqualElement(elm1, elm2) {
        let attrs1, attrs2, name, node1, node2;
    
        // Compare attributes without order sensitivity
        attrs1 = DomHandler.getAttributeNames(elm1);
        attrs2 = DomHandler.getAttributeNames(elm2);
        if (attrs1.join(",") !== attrs2.join(",")) {
            // console.log("Found nodes with different sets of attributes; not equiv");
            return false;
        }
    
        // ...and values
        // unless you want to compare DOM0 event handlers
        // (onclick="...")
        for (let index = 0; index < attrs1.length; ++index) {
            name = attrs1[index];
            if (name === 'style') {
                const astyle = elm1.style;
                const bstyle = elm2.style;
                const rexDigitsOnly = /^\d+$/;
                for (const key of Object.keys(astyle)) {
                    if (!rexDigitsOnly.test(key) && astyle[key] !== bstyle[key]) {
                        // Not equivalent, stop
                        //console.log("Found nodes with mis-matched values for attribute '" + name + "'; not equiv");
                        return false;
                    }
                }
            } else {
                if (elm1.getAttribute(name) !== elm2.getAttribute(name)) {
                    // console.log("Found nodes with mis-matched values for attribute '" + name + "'; not equiv");
                    return false;
                }
            }
        }
    
        // Walk the children
        for (node1 = elm1.firstChild, node2 = elm2.firstChild; node1 && node2; node1 = node1.nextSibling, node2 = node2.nextSibling) {
            if (node1.nodeType !== node2.nodeType) {
                // display("Found nodes of different types; not equiv");
                return false;
            }
            if (node1.nodeType === 1) { // Element
                if (!DomHandler.isEqualElement(node1, node2)) {
                    return false;
                }
            } else if (node1.nodeValue !== node2.nodeValue) {
                // console.log("Found nodes with mis-matched nodeValues; not equiv");
                return false;
            }
        }
        if (node1 || node2) {
            // One of the elements had more nodes than the other
            // console.log("Found more children of one element than the other; not equivalent");
            return false;
        }
    
        // Seem the same
        return true;
    }

melloware avatar Nov 08 '21 13:11 melloware