Content Model: Potential perf improvement in getFormatState (#2187)

microsoft · Nov 6, 2023 · 331b5a3 · 331b5a3
1 parent 095249e
commit 331b5a3
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 24 deletions.
diff --git a/packages-content-model/roosterjs-content-model-editor/lib/publicApi/format/getFormatState.ts b/packages-content-model/roosterjs-content-model-editor/lib/publicApi/format/getFormatState.ts
@@ -57,37 +57,34 @@ export function reducedModelChildProcessor(
     parent: ParentNode,
     context: FormatStateContext
 ) {
-    const selectionRootNode = getSelectionRootNode(context.selection);
-
-    if (selectionRootNode) {
-        if (!context.nodeStack) {
-            context.nodeStack = createNodeStack(parent, selectionRootNode);
-        }
+    if (!context.nodeStack) {
+        const selectionRootNode = getSelectionRootNode(context.selection);
+        context.nodeStack = selectionRootNode ? createNodeStack(parent, selectionRootNode) : [];
+    }
 
-        const stackChild = context.nodeStack.pop();
+    const stackChild = context.nodeStack.pop();
 
-        if (stackChild) {
-            const [nodeStartOffset, nodeEndOffset] = getRegularSelectionOffsets(context, parent);
+    if (stackChild) {
+        const [nodeStartOffset, nodeEndOffset] = getRegularSelectionOffsets(context, parent);
 
-            // If selection is not on this node, skip getting node index to save some time since we don't need it here
-            const index =
-                nodeStartOffset >= 0 || nodeEndOffset >= 0 ? getChildIndex(parent, stackChild) : -1;
+        // If selection is not on this node, skip getting node index to save some time since we don't need it here
+        const index =
+            nodeStartOffset >= 0 || nodeEndOffset >= 0 ? getChildIndex(parent, stackChild) : -1;
 
-            if (index >= 0) {
-                handleRegularSelection(index, context, group, nodeStartOffset, nodeEndOffset);
-            }
+        if (index >= 0) {
+            handleRegularSelection(index, context, group, nodeStartOffset, nodeEndOffset);
+        }
 
-            processChildNode(group, stackChild, context);
+        processChildNode(group, stackChild, context);
 
-            if (index >= 0) {
-                handleRegularSelection(index + 1, context, group, nodeStartOffset, nodeEndOffset);
-            }
-        } else {
-            // No child node from node stack, that means we have reached the deepest node of selection.
-            // Now we can use default child processor to perform full sub tree scanning for content model,
-            // So that all selected node will be included.
-            context.defaultElementProcessors.child(group, parent, context);
+        if (index >= 0) {
+            handleRegularSelection(index + 1, context, group, nodeStartOffset, nodeEndOffset);
         }
+    } else {
+        // No child node from node stack, that means we have reached the deepest node of selection.
+        // Now we can use default child processor to perform full sub tree scanning for content model,
+        // So that all selected node will be included.
+        context.defaultElementProcessors.child(group, parent, context);
     }
 }
 

diff --git a/...-content-model/roosterjs-content-model-editor/test/publicApi/format/getFormatStateTest.ts b/...-content-model/roosterjs-content-model-editor/test/publicApi/format/getFormatStateTest.ts
@@ -1,4 +1,5 @@
 import * as getPendingFormat from '../../../lib/modelApi/format/pendingFormat';
+import * as getSelectionRootNode from '../../../lib/modelApi/selection/getSelectionRootNode';
 import * as retrieveModelFormatState from '../../../lib/modelApi/common/retrieveModelFormatState';
 import { ContentModelFormatState } from '../../../lib/publicTypes/format/formatState/ContentModelFormatState';
 import { DomToModelContext } from 'roosterjs-content-model-types';
@@ -180,15 +181,22 @@ describe('getFormatState', () => {
         );
     });
 });
+
 describe('reducedModelChildProcessor', () => {
     let context: DomToModelContext;
+    let getSelectionRootNodeSpy: jasmine.Spy;
 
     beforeEach(() => {
         context = createDomToModelContext(undefined, {
             processorOverride: {
                 child: reducedModelChildProcessor,
             },
         });
+
+        getSelectionRootNodeSpy = spyOn(
+            getSelectionRootNode,
+            'getSelectionRootNode'
+        ).and.callThrough();
     });
 
     it('Empty DOM', () => {
@@ -201,6 +209,7 @@ describe('reducedModelChildProcessor', () => {
             blockGroupType: 'Document',
             blocks: [],
         });
+        expect(getSelectionRootNodeSpy).toHaveBeenCalledTimes(1);
     });
 
     it('Single child node, with selected Node in context', () => {
@@ -236,6 +245,7 @@ describe('reducedModelChildProcessor', () => {
                 },
             ],
         });
+        expect(getSelectionRootNodeSpy).toHaveBeenCalledTimes(1);
     });
 
     it('Multiple child nodes, with selected Node in context', () => {
@@ -277,6 +287,7 @@ describe('reducedModelChildProcessor', () => {
                 },
             ],
         });
+        expect(getSelectionRootNodeSpy).toHaveBeenCalledTimes(1);
     });
 
     it('Multiple child nodes, with selected Node in context, with more child nodes under selected node', () => {
@@ -340,6 +351,7 @@ describe('reducedModelChildProcessor', () => {
                 },
             ],
         });
+        expect(getSelectionRootNodeSpy).toHaveBeenCalledTimes(1);
     });
 
     it('Multiple layer with multiple child nodes, with selected Node in context, with more child nodes under selected node', () => {
@@ -399,6 +411,7 @@ describe('reducedModelChildProcessor', () => {
                 { blockType: 'Paragraph', segments: [], format: {}, isImplicit: true },
             ],
         });
+        expect(getSelectionRootNodeSpy).toHaveBeenCalledTimes(1);
     });
 
     it('With table, need to do format for all table cells', () => {
@@ -478,5 +491,6 @@ describe('reducedModelChildProcessor', () => {
                 },
             ],
         });
+        expect(getSelectionRootNodeSpy).toHaveBeenCalledTimes(1);
     });
 });