Skip to content

Commit

Permalink
fix(schema): prevent application from hanging when selecting ranges t…
Browse files Browse the repository at this point in the history
…oo fast on the "Schema" tab COMPASS-8048 (#5980)

fix(schema): debounce onChange calls to avoid application main thread locking
  • Loading branch information
gribnoysup authored Jul 1, 2024
1 parent d39bd0c commit 0472a06
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 16 deletions.
2 changes: 1 addition & 1 deletion packages/compass-query-bar/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const QueryBarPlugin = registerHadronPlugin(
}
);

export type ChangeQueryBar = typeof useChangeQueryBarQuery;
export type ChangeQueryFn = ReturnType<typeof useChangeQueryBarQuery>;

// Rendering query bar only makes sense if query bar store is in the rendering
// tree. If it's not, `useQueryBarComponent` will return a `null` component
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import GeoscatterMapItem from './marker';
import { LIGHTMODE_TILE_URL, DARKMODE_TILE_URL } from './constants';
import { getHereAttributionMessage } from './utils';
import { debounce } from 'lodash';
import { useChangeQueryBarQuery } from '@mongodb-js/compass-query-bar';

// TODO: Disable boxZoom handler for circle lasso.
//
Expand Down Expand Up @@ -308,14 +307,13 @@ class UnthemedCoordinatesMinichart extends PureComponent {
}
}

const CoordinatesMinichart = ({ ...props }) => {
const CoordinatesMinichart = ({ onQueryChanged, ...props }) => {
const darkMode = useDarkMode();
const changeQuery = useChangeQueryBarQuery();
const onChange = useCallback(
(geoQuery) => {
changeQuery('mergeGeoQuery', geoQuery);
onQueryChanged('mergeGeoQuery', geoQuery);
},
[changeQuery]
[onQueryChanged]
);
return (
<UnthemedCoordinatesMinichart
Expand All @@ -326,5 +324,9 @@ const CoordinatesMinichart = ({ ...props }) => {
);
};

CoordinatesMinichart.propTypes = {
onQueryChanged: PropTypes.func,
};

export default CoordinatesMinichart;
export { CoordinatesMinichart };
22 changes: 19 additions & 3 deletions packages/compass-schema/src/components/field.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useMemo, useState } from 'react';
import React, { useEffect, useMemo, useState } from 'react';
import {
Subtitle,
Icon,
Expand All @@ -10,7 +10,7 @@ import {
SignalPopover,
PerformanceSignals,
} from '@mongodb-js/compass-components';
import { find } from 'lodash';
import { debounce, find } from 'lodash';
import { withPreferences } from 'compass-preferences-model/provider';
import type {
ArraySchemaType,
Expand Down Expand Up @@ -208,6 +208,22 @@ function Field({ actions, name, path, types, enableMaps }: FieldProps) {
: undefined;
});

// In some cases charts can call the change callback too often (for examlpe
// when selecting from a very tightly rendered list of timeline items, like
// ObjectIds), to avoid application hanging trying to process them all
// syncronously and sequentially causing non-stop re-renders, we're debouncing
// change call a bit and only applying the last selected change to the actual
// query bar
const debouncedChangeQuery = useMemo(() => {
return debounce(changeQuery, 100);
}, [changeQuery]);

useEffect(() => {
return () => {
debouncedChangeQuery.cancel();
};
}, [debouncedChangeQuery]);

const activeShownTypes = useMemo(() => sortTypes(types), [types]);
const nestedDocType = useMemo(() => getNestedDocType(types), [types]);

Expand Down Expand Up @@ -293,7 +309,7 @@ function Field({ actions, name, path, types, enableMaps }: FieldProps) {
type={activeType}
nestedDocType={nestedDocType}
actions={actions}
onQueryChanged={changeQuery}
onQueryChanged={debouncedChangeQuery}
/>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class MiniChart extends PureComponent {
queryValue={fieldValue}
type={this.props.type}
width={width}
onQueryChanged={this.props.onQueryChanged}
/>
);
}
Expand All @@ -97,6 +98,7 @@ class MiniChart extends PureComponent {
actions={this.props.actions}
fieldName={fieldName}
type={this.props.type}
onQueryChanged={this.props.onQueryChanged}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class UniqueMiniChart extends Component {
queryValue: PropTypes.oneOfType([PropTypes.string, PropTypes.object]),
type: PropTypes.object.isRequired,
width: PropTypes.number,
onQueryChanged: PropTypes.func,
};

constructor(props) {
Expand Down Expand Up @@ -46,6 +47,7 @@ class UniqueMiniChart extends Component {
value={value}
queryValue={this.props.queryValue}
fieldName={this.props.fieldName}
onQueryChanged={this.props.onQueryChanged}
/>
);
});
Expand Down
15 changes: 10 additions & 5 deletions packages/compass-schema/src/components/value-bubble.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
palette,
useDarkMode,
} from '@mongodb-js/compass-components';
import { useChangeQueryBarQuery } from '@mongodb-js/compass-query-bar';
import type { ChangeQueryFn } from '@mongodb-js/compass-query-bar';

import constants from '../constants/schema';

Expand Down Expand Up @@ -63,21 +63,26 @@ type ValueBubbleProps = {
fieldName: string;
queryValue: any;
value: any;
onQueryChanged: ChangeQueryFn;
};

function ValueBubble({ fieldName, queryValue, value }: ValueBubbleProps) {
function ValueBubble({
fieldName,
queryValue,
value,
onQueryChanged,
}: ValueBubbleProps) {
const darkMode = useDarkMode();
const changeQuery = useChangeQueryBarQuery();

const onBubbleClicked = useCallback(
(e: React.MouseEvent<HTMLButtonElement>) => {
changeQuery(e.shiftKey ? 'toggleDistinctValue' : 'setValue', {
onQueryChanged(e.shiftKey ? 'toggleDistinctValue' : 'setValue', {
field: fieldName,
value,
unsetIfSet: true,
});
},
[changeQuery, fieldName, value]
[onQueryChanged, fieldName, value]
);

const extractedStringValue = useMemo(
Expand Down
4 changes: 2 additions & 2 deletions packages/compass-schema/src/stores/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export type SchemaStore = StoreWithStateMixin<SchemaState> & {
onDeleted: (geoQuery: ReturnType<typeof generateGeoQuery>) => void
): void;
stopAnalysis(): void;
_trackSchemaAnalyzed(analysisTimeMS: number): void;
_trackSchemaAnalyzed(analysisTimeMS: number, query: any): void;
startAnalysis(): void;
};

Expand Down Expand Up @@ -317,7 +317,7 @@ export function activateSchemaPlugin(
resultId: resultId(),
});

this._trackSchemaAnalyzed(analysisTime);
this._trackSchemaAnalyzed(analysisTime, query);

this.onSchemaSampled();
} catch (err: any) {
Expand Down

0 comments on commit 0472a06

Please sign in to comment.