chore(crud): Optimise bulk update previews (#5237)

* profile what happens when we update the previews * memoize the Document component * remove some logging * remove commented code * remove profiling * change assertion * change assertion * fix assertions
mongodb-js · Dec 15, 2023 · e934571 · e934571
1 parent 78d598a
commit e934571
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 21 deletions.
diff --git a/packages/compass-crud/src/components/change-view/change-view.tsx b/packages/compass-crud/src/components/change-view/change-view.tsx
@@ -1,4 +1,4 @@
-import React, { useState, useContext, createContext } from 'react';
+import React, { useState, useContext, createContext, useMemo } from 'react';
 import { type Document } from 'bson';
 import TypeChecker from 'hadron-type-checker';
 
@@ -569,7 +569,7 @@ export function ChangeView({
   before: Document;
   after: Document;
 }) {
-  const obj = unifyDocuments(before, after);
+  const obj = useMemo(() => unifyDocuments(before, after), [before, after]);
 
   const darkMode = useDarkMode();
 

diff --git a/packages/compass-crud/src/components/document.tsx b/packages/compass-crud/src/components/document.tsx
@@ -61,4 +61,4 @@ Document.propTypes = {
   isExpanded: PropTypes.bool,
 };
 
-export default Document;
+export default React.memo(Document);
diff --git a/packages/compass-crud/src/stores/crud-store.ts b/packages/compass-crud/src/stores/crud-store.ts
@@ -1127,14 +1127,6 @@ class CrudStoreImpl
       this.state.bulkUpdate.previewAbortController.abort();
     }
 
-    // immediately persist the state before any other state changes
-    this.setState({
-      bulkUpdate: {
-        ...this.state.bulkUpdate,
-        updateText,
-      },
-    });
-
     // Don't try and calculate the update preview if we know it won't work. Just
     // see if the update will parse.
     if (!this.state.isUpdatePreviewSupported) {
@@ -1144,6 +1136,7 @@ class CrudStoreImpl
         this.setState({
           bulkUpdate: {
             ...this.state.bulkUpdate,
+            updateText,
             preview: {
               changes: [],
             },
@@ -1159,6 +1152,7 @@ class CrudStoreImpl
       this.setState({
         bulkUpdate: {
           ...this.state.bulkUpdate,
+          updateText,
           preview: {
             changes: [],
           },
@@ -1173,6 +1167,8 @@ class CrudStoreImpl
 
     const abortController = new AbortController();
 
+    // set the abort controller in the state before we start doing anything so
+    // that other calls can see it
     this.setState({
       bulkUpdate: {
         ...this.state.bulkUpdate,
@@ -1192,6 +1188,7 @@ class CrudStoreImpl
       this.setState({
         bulkUpdate: {
           ...this.state.bulkUpdate,
+          updateText,
           preview: {
             changes: [],
           },
@@ -1200,6 +1197,12 @@ class CrudStoreImpl
           previewAbortController: undefined,
         },
       });
+
+      return;
+    }
+
+    if (abortController.signal.aborted) {
+      // don't kick off an expensive query if we're already aborted anyway
       return;
     }
 
@@ -1221,6 +1224,7 @@ class CrudStoreImpl
       this.setState({
         bulkUpdate: {
           ...this.state.bulkUpdate,
+          updateText,
           preview: {
             changes: [],
           },
@@ -1229,6 +1233,7 @@ class CrudStoreImpl
           previewAbortController: undefined,
         },
       });
+
       return;
     }
 
@@ -1240,6 +1245,7 @@ class CrudStoreImpl
     this.setState({
       bulkUpdate: {
         ...this.state.bulkUpdate,
+        updateText,
         preview,
         serverError: undefined,
         syntaxError: undefined,

diff --git a/packages/compass-e2e-tests/tests/collection-bulk-update.test.ts b/packages/compass-e2e-tests/tests/collection-bulk-update.test.ts
@@ -67,7 +67,7 @@ describe('Bulk Update', () => {
     // Check that the modal starts with the default update text
     expect(
       await browser.getCodemirrorEditorText(Selectors.BulkUpdateUpdate)
-    ).to.equal('{\n  $set: {\n\n  },\n}');
+    ).to.match(/{\s+\$set:\s+{\s+},?\s+}/);
 
     // Change the update text
     await browser.setCodemirrorEditorValue(
@@ -198,12 +198,9 @@ describe('Bulk Update', () => {
     ).to.equal('{ i: { $gt: 5 } }');
 
     // Check that the modal starts with the expected update text
-    expect(await browser.getCodemirrorEditorText(Selectors.BulkUpdateUpdate)).to
-      .equal(`{
- $set: {
-  k: 0
- }
-}`);
+    expect(
+      await browser.getCodemirrorEditorText(Selectors.BulkUpdateUpdate)
+    ).to.equal(`{ $set: { k: 0 } }`);
   });
 });
 

diff --git a/packages/compass-schema-validation/src/components/document-preview/document-preview.spec.tsx b/packages/compass-schema-validation/src/components/document-preview/document-preview.spec.tsx
@@ -1,5 +1,5 @@
 import React from 'react';
-import { mount, shallow } from 'enzyme';
+import { mount } from 'enzyme';
 import { expect } from 'chai';
 
 import DocumentPreview from '.';
@@ -21,10 +21,10 @@ describe('DocumentPreview [Component]', function () {
 
   context('when document loading state is success', function () {
     it('renders a document if there is one present', function () {
-      const component = shallow(
+      const component = mount(
         <DocumentPreview loadingState="success" document={{}} />
       );
-      expect(component.find('Document')).to.exist;
+      expect(component.find('HadronDocument')).to.exist;
     });
 
     it('renders a no preview text when there is no document', function () {