Skip to content

Commit e934571

Browse files
authored
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
1 parent 78d598a commit e934571

File tree

5 files changed

+24
-21
lines changed

5 files changed

+24
-21
lines changed

packages/compass-crud/src/components/change-view/change-view.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useState, useContext, createContext } from 'react';
1+
import React, { useState, useContext, createContext, useMemo } from 'react';
22
import { type Document } from 'bson';
33
import TypeChecker from 'hadron-type-checker';
44

@@ -569,7 +569,7 @@ export function ChangeView({
569569
before: Document;
570570
after: Document;
571571
}) {
572-
const obj = unifyDocuments(before, after);
572+
const obj = useMemo(() => unifyDocuments(before, after), [before, after]);
573573

574574
const darkMode = useDarkMode();
575575

packages/compass-crud/src/components/document.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,4 @@ Document.propTypes = {
6161
isExpanded: PropTypes.bool,
6262
};
6363

64-
export default Document;
64+
export default React.memo(Document);

packages/compass-crud/src/stores/crud-store.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,14 +1127,6 @@ class CrudStoreImpl
11271127
this.state.bulkUpdate.previewAbortController.abort();
11281128
}
11291129

1130-
// immediately persist the state before any other state changes
1131-
this.setState({
1132-
bulkUpdate: {
1133-
...this.state.bulkUpdate,
1134-
updateText,
1135-
},
1136-
});
1137-
11381130
// Don't try and calculate the update preview if we know it won't work. Just
11391131
// see if the update will parse.
11401132
if (!this.state.isUpdatePreviewSupported) {
@@ -1144,6 +1136,7 @@ class CrudStoreImpl
11441136
this.setState({
11451137
bulkUpdate: {
11461138
...this.state.bulkUpdate,
1139+
updateText,
11471140
preview: {
11481141
changes: [],
11491142
},
@@ -1159,6 +1152,7 @@ class CrudStoreImpl
11591152
this.setState({
11601153
bulkUpdate: {
11611154
...this.state.bulkUpdate,
1155+
updateText,
11621156
preview: {
11631157
changes: [],
11641158
},
@@ -1173,6 +1167,8 @@ class CrudStoreImpl
11731167

11741168
const abortController = new AbortController();
11751169

1170+
// set the abort controller in the state before we start doing anything so
1171+
// that other calls can see it
11761172
this.setState({
11771173
bulkUpdate: {
11781174
...this.state.bulkUpdate,
@@ -1192,6 +1188,7 @@ class CrudStoreImpl
11921188
this.setState({
11931189
bulkUpdate: {
11941190
...this.state.bulkUpdate,
1191+
updateText,
11951192
preview: {
11961193
changes: [],
11971194
},
@@ -1200,6 +1197,12 @@ class CrudStoreImpl
12001197
previewAbortController: undefined,
12011198
},
12021199
});
1200+
1201+
return;
1202+
}
1203+
1204+
if (abortController.signal.aborted) {
1205+
// don't kick off an expensive query if we're already aborted anyway
12031206
return;
12041207
}
12051208

@@ -1221,6 +1224,7 @@ class CrudStoreImpl
12211224
this.setState({
12221225
bulkUpdate: {
12231226
...this.state.bulkUpdate,
1227+
updateText,
12241228
preview: {
12251229
changes: [],
12261230
},
@@ -1229,6 +1233,7 @@ class CrudStoreImpl
12291233
previewAbortController: undefined,
12301234
},
12311235
});
1236+
12321237
return;
12331238
}
12341239

@@ -1240,6 +1245,7 @@ class CrudStoreImpl
12401245
this.setState({
12411246
bulkUpdate: {
12421247
...this.state.bulkUpdate,
1248+
updateText,
12431249
preview,
12441250
serverError: undefined,
12451251
syntaxError: undefined,

packages/compass-e2e-tests/tests/collection-bulk-update.test.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ describe('Bulk Update', () => {
6767
// Check that the modal starts with the default update text
6868
expect(
6969
await browser.getCodemirrorEditorText(Selectors.BulkUpdateUpdate)
70-
).to.equal('{\n $set: {\n\n },\n}');
70+
).to.match(/{\s+\$set:\s+{\s+},?\s+}/);
7171

7272
// Change the update text
7373
await browser.setCodemirrorEditorValue(
@@ -198,12 +198,9 @@ describe('Bulk Update', () => {
198198
).to.equal('{ i: { $gt: 5 } }');
199199

200200
// Check that the modal starts with the expected update text
201-
expect(await browser.getCodemirrorEditorText(Selectors.BulkUpdateUpdate)).to
202-
.equal(`{
203-
$set: {
204-
k: 0
205-
}
206-
}`);
201+
expect(
202+
await browser.getCodemirrorEditorText(Selectors.BulkUpdateUpdate)
203+
).to.equal(`{ $set: { k: 0 } }`);
207204
});
208205
});
209206

packages/compass-schema-validation/src/components/document-preview/document-preview.spec.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from 'react';
2-
import { mount, shallow } from 'enzyme';
2+
import { mount } from 'enzyme';
33
import { expect } from 'chai';
44

55
import DocumentPreview from '.';
@@ -21,10 +21,10 @@ describe('DocumentPreview [Component]', function () {
2121

2222
context('when document loading state is success', function () {
2323
it('renders a document if there is one present', function () {
24-
const component = shallow(
24+
const component = mount(
2525
<DocumentPreview loadingState="success" document={{}} />
2626
);
27-
expect(component.find('Document')).to.exist;
27+
expect(component.find('HadronDocument')).to.exist;
2828
});
2929

3030
it('renders a no preview text when there is no document', function () {

0 commit comments

Comments
 (0)