Delta uses clone deep now and hits ERROR RangeError: Maximum call stack size exceeded

[phillipwildhirt]

The Delta.push method has been changed from

Delta.prototype.push = function (newOp) {
  var index = this.ops.length;
  var lastOp = this.ops[index - 1];
  newOp = extend(true, {}, newOp);

to use cloneDeep which when cloneDeep is called it hits the maximum call stack error, ERROR RangeError: Maximum call stack size exceeded, anytime you pass a complicated/injectable class into the created Element.

  push(newOp: Op): this {
    let index = this.ops.length;
    let lastOp = this.ops[index - 1];
    newOp = cloneDeep(newOp); // error here. At lodash.clonedeep.index.js line 897

There's even a comment in lodash "Recursively populate clone (susceptible to call stack limits)."

---

We pass an Angular Injectable Service instance into an Embed Blot, the blot is actually an Angular Element, and because it's a complicated objected being passed through Quill's code, this clone deep causes the max call stack error.

This was not an issue in 1.3.7.

Angular 17.3.7
Quill v2.0.1

[luin]

All properties inside an op should be serializable (e.g. plain object) without and circular structures because those data would be returned when you call quill.getContents(), and the result is expected to be able to be serialized and stored in the database.

Not sure about your use case, but a potential solution could be creating/getting the service instance in the constructor of the blot based on the value.

[phillipwildhirt]

We pass in a service instance when the blot is created and then destroy the blot before before it's ever saved to the database. This blot is the anchor for a number menu's that pop-up and allow a final saveable blot to be created. The service allows us to minimize the amount of input/outputs that are required inside the blot... It's probably better, as you say, to serialize it, but it worked for us in 1.3.7. I guess it will be one of those some-day-refactors.

Our ops look something like this:

{"ops":[
  {"insert":{"at-role-people-tag":{"role":"Agent","names":["John Smith"],"autoCollapse":true}}},
  {"insert":"\n"},
  {"insert":{"anchor-tag":{dropdownService: DropdownService, itemType: 'order'}}}, 
  {"insert":"\n\n\n\n"},
  {"attributes":{"color":"#cce0f5","italic":true},"insert":"--"},
  {"insert":"\n"}
]}

where the anchor-tag is temporary, and replaced with something like the at-role-people-tag before database save.

---

I discovered that 1.3.7 code was not ACTUALLY using what is in [email protected]. It didn't use cloneDeep in it's push method. First, few lines of 1.3.7's Delta.push:

Delta.prototype.push = function (newOp) {
  var index = this.ops.length;
  var lastOp = this.ops[index - 1];
  newOp = extend(true, {}, newOp);

And [email protected]:

  push(newOp: Op): this {
    let index = this.ops.length;
    let lastOp = this.ops[index - 1];
    newOp = cloneDeep(newOp);

Since the code in question: Delta.push() is only looking at the insert, attributes, etc, and never looks at or uses any of the nested items in the insert, I forked and changed the method to a lodash.clone (shallow) instead, and it works great. I think the cloneDeep may be overkill, and something to consider.

[luin]

Delta inserts can be arbitrary objects as well (e.g. { insert: { image: 'https://' } }) so that's the reason we use cloneDeep. I understand that in your case you don't need to save the blot value to databases but as a best practice, every blot value should be serializable.

To solve that, I would maintain a map (Record<string, DropdownService>) where the keys are unique ids generated randomly per session and in the delta you only store the id.