diff --git a/packages/ckeditor5-engine/src/model/differ.ts b/packages/ckeditor5-engine/src/model/differ.ts index 41e31682b69..bb6b8bb0d3e 100644 --- a/packages/ckeditor5-engine/src/model/differ.ts +++ b/packages/ckeditor5-engine/src/model/differ.ts @@ -1520,7 +1520,17 @@ function _generateDiffInstructionsFromChanges( oldChildrenLength: number, change // We removed `howMany` old nodes, update `oldChildrenHandled`. oldChildrenHandled += change.howMany; } else { - diff.push( ...'a'.repeat( change.howMany ).split( '' ) ); + // Total maximum amount of arguments that can be passed to `Array.prototype.push` may be limited so we need to + // add them manually one by one to avoid this limit. However loop might be a bit slower than `push` method on + // smaller changesets so we need to decide which method to use based on the size of the change. + // See: https://github.com/ckeditor/ckeditor5/issues/16819 + if ( change.howMany > 1500 ) { + for ( let i = 0; i < change.howMany; i++ ) { + diff.push( 'a' ); + } + } else { + diff.push( ...'a'.repeat( change.howMany ).split( '' ) ); + } // The last handled offset is at the position after the changed range. offset = change.offset + change.howMany; diff --git a/packages/ckeditor5-engine/tests/model/differ.js b/packages/ckeditor5-engine/tests/model/differ.js index e47e3bc14a3..cc7b388288c 100644 --- a/packages/ckeditor5-engine/tests/model/differ.js +++ b/packages/ckeditor5-engine/tests/model/differ.js @@ -1707,6 +1707,62 @@ describe( 'Differ', () => { expectChanges( [] ); } ); } ); + + // See: https://github.com/ckeditor/ckeditor5/issues/16819 + it( 'on element with very long text should have batched instructions together during generating diff from changes', () => { + const MAX_PUSH_CALL_STACK_ARGS = 1500; + + const p = root.getChild( 0 ); + const veryLongString = 'a'.repeat( 300 ); + + model.change( () => { + insert( veryLongString, Position._createAt( p, 0 ) ); + } ); + + const pushSpy = sinon.spy( Array.prototype, 'push' ); + + model.change( writer => { + writer.setAttribute( attributeKey, true, writer.createRangeIn( p ) ); + } ); + + // Let's check if appended instructions has been batched together in single `.push()` call. + const instructionsDiff = pushSpy.args.find( args => ( + args.length >= 300 && + args.length < MAX_PUSH_CALL_STACK_ARGS && + args.every( ch => ch === 'a' ) + ) ); + + expect( instructionsDiff ).not.to.be.undefined; + pushSpy.restore(); + } ); + + // See: https://github.com/ckeditor/ckeditor5/issues/16819 + it( 'on element with very long text should not have batched instructions together during generating diff from changes ' + + 'that are larger than max push call stack args count', () => { + const MAX_PUSH_CALL_STACK_ARGS = 1500; + + const p = root.getChild( 0 ); + const veryLongString = 'a'.repeat( MAX_PUSH_CALL_STACK_ARGS + 10 ); + + model.change( () => { + insert( veryLongString, Position._createAt( p, 0 ) ); + } ); + + const pushSpy = sinon.spy( Array.prototype, 'push' ); + + model.change( writer => { + writer.setAttribute( attributeKey, true, writer.createRangeIn( p ) ); + } ); + + // Let's check if appended instructions has been NOT batched together in single `.push()` call. + const instructionsDiff = pushSpy.args.find( args => ( + args.length > MAX_PUSH_CALL_STACK_ARGS && + args.every( ch => ch === 'a' ) + ) ); + + expect( instructionsDiff ).to.be.undefined; + pushSpy.restore(); + } ); } ); describe( 'split', () => {