Skip to content
Snippets Groups Projects
Unverified Commit 55dbe7b2 authored by Uladzimir Havenchyk's avatar Uladzimir Havenchyk Committed by GitHub
Browse files

Fix memoization on the table cell (#34093)

parent bfe1714b
Branches
Tags
No related merge requests found
......@@ -18,6 +18,16 @@ const memoized = new WeakMap();
const createMap = () => new Map();
/**
* This method implements memoization for class methods
* It creates a map where class itself, method and all the parameters are used as keys for a nested map
* map<class, map<method, map<param1, map<param2, map<param3...>>>>>
*
* If you use objects as parameters, make sure their references are stable as they will be used as keys
*
* @param keys - class methods to memoize
* @returns the same class with memoized methods
*/
export function memoizeClass<T>(
...keys: string[]
): (Class: Constructor<T>) => Constructor<T> {
......@@ -32,7 +42,7 @@ export function memoizeClass<T>(
const descriptor = descriptors[key];
const method = descriptor.value;
// If we don't get a decsriptor.value, it must have a getter (i.e., ES6 class properties)
// If we don't get a descriptor.value, it must have a getter (i.e., ES6 class properties)
if (!method) {
throw new TypeError(`Class properties cannot be memoized`);
}
......
......@@ -49,6 +49,8 @@ export class Mode {
return question.setDatasetQuery(Lib.toLegacyQuery(updatedQuery));
};
// TODO: those calculations are really expensive and must be memoized at some level
// check `_visualizationIsClickableCached` from TableInteractive
const availableDrillThrus = Lib.availableDrillThrus(
query,
stageIndex,
......@@ -58,7 +60,6 @@ export class Mode {
clicked?.dimensions,
);
// TODO: those calculations are really expensive and should be memoized at some level
drills = availableDrillThrus
.flatMap(drill => {
const drillDisplayInfo = Lib.displayInfo(query, stageIndex, drill);
......
......@@ -417,14 +417,9 @@ class TableInteractive extends Component {
);
}
getHeaderClickedObject(columnIndex) {
getHeaderClickedObject(data, columnIndex, isPivoted, query) {
try {
return getTableHeaderClickedObject(
this.props.data,
columnIndex,
this.props.isPivoted,
this.props.query,
);
return getTableHeaderClickedObject(data, columnIndex, isPivoted, query);
} catch (e) {
console.error(e);
}
......@@ -692,15 +687,19 @@ class TableInteractive extends Component {
hasMetadataPopovers,
getColumnTitle,
renderTableHeaderWrapper,
query,
} = this.props;
const { dragColIndex, showDetailShortcut } = this.state;
const { cols } = data;
const column = cols[columnIndex];
const columnTitle = getColumnTitle(columnIndex);
const clicked = this.getHeaderClickedObject(columnIndex);
const clicked = this.getHeaderClickedObject(
data,
columnIndex,
isPivoted,
query,
);
const isDraggable = !isPivoted;
const isDragging = dragColIndex === columnIndex;
const isClickable = this.visualizationIsClickable(clicked);
......@@ -1152,6 +1151,7 @@ export default _.compose(
"getCellBackgroundColor",
"getCellFormattedValue",
"getDimension",
"getHeaderClickedObject",
),
)(TableInteractive);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment