Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Perf]Prevent resizable component rerenders #5889

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 60 additions & 40 deletions app/client/src/components/editorComponents/DropTargetComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import React, {
Context,
createContext,
useEffect,
useCallback,
memo,
useMemo,
} from "react";
import styled from "styled-components";
import { useDrop, XYCoord, DropTargetMonitor } from "react-dnd";
Expand All @@ -30,8 +32,9 @@ import {
useShowPropertyPane,
useCanvasSnapRowsUpdateHook,
} from "utils/hooks/dragResizeHooks";
import { getOccupiedSpaces } from "selectors/editorSelectors";
import { getMemoizedOccupiedSpaces } from "selectors/editorSelectors";
import { useWidgetSelection } from "utils/hooks/useWidgetSelection";
import { OccupiedSpace } from "constants/editorConstants";

type DropTargetComponentProps = WidgetProps & {
children?: ReactNode;
Expand Down Expand Up @@ -64,6 +67,7 @@ function Onboarding() {
export const DropTargetContext: Context<{
updateDropTargetRows?: (widgetId: string, row: number) => boolean;
persistDropTargetRows?: (widgetId: string, row: number) => void;
occupiedSpaces?: OccupiedSpace[];
}> = createContext({});

export function DropTargetComponent(props: DropTargetComponentProps) {
Expand All @@ -72,7 +76,6 @@ export function DropTargetComponent(props: DropTargetComponentProps) {
const snapRows = getCanvasSnapRows(props.bottomRow, props.canExtend);

const { updateWidget } = useContext(EditorContext);
const occupiedSpaces = useSelector(getOccupiedSpaces);
const selectedWidget = useSelector(
(state: AppState) => state.ui.widgetDragResize.lastSelectedWidget,
);
Expand All @@ -83,17 +86,16 @@ export function DropTargetComponent(props: DropTargetComponentProps) {
(state: AppState) => state.ui.widgetDragResize.isDragging,
);

const spacesOccupiedBySiblingWidgets =
occupiedSpaces && occupiedSpaces[props.widgetId]
? occupiedSpaces[props.widgetId]
: undefined;

const childWidgets = useSelector(
(state: AppState) => state.entities.canvasWidgets[props.widgetId].children,
);

const occupiedSpacesByChildren =
occupiedSpaces && occupiedSpaces[props.widgetId];
const selectOccupiedSpaces = useMemo(
() => getMemoizedOccupiedSpaces(props.widgetId),
[props.widgetId],
);

const occupiedSpacesByChildren = useSelector(selectOccupiedSpaces);

const [dropTargetOffset, setDropTargetOffset] = useState({ x: 0, y: 0 });
const [rows, setRows] = useState(snapRows);
Expand All @@ -107,40 +109,51 @@ export function DropTargetComponent(props: DropTargetComponentProps) {
setRows(snapRows);
}, [props.bottomRow, props.canExtend]);

const persistDropTargetRows = (widgetId: string, widgetBottomRow: number) => {
const newRows = calculateDropTargetRows(
widgetId,
widgetBottomRow,
props.minHeight / GridDefaults.DEFAULT_GRID_ROW_HEIGHT - 1,
occupiedSpacesByChildren,
);
const rowsToPersist = Math.max(
props.minHeight / GridDefaults.DEFAULT_GRID_ROW_HEIGHT - 1,
newRows,
);
setRows(rowsToPersist);
if (canDropTargetExtend) {
updateCanvasSnapRows(props.widgetId, rowsToPersist);
}
};

/* Update the rows of the main container based on the current widget's (dragging/resizing) bottom row */
const updateDropTargetRows = (widgetId: string, widgetBottomRow: number) => {
if (canDropTargetExtend) {
const persistDropTargetRows = useCallback(
(widgetId: string, widgetBottomRow: number) => {
const newRows = calculateDropTargetRows(
widgetId,
widgetBottomRow,
props.minHeight / GridDefaults.DEFAULT_GRID_ROW_HEIGHT - 1,
occupiedSpacesByChildren,
);
if (rows < newRows) {
setRows(newRows);
return true;
const rowsToPersist = Math.max(
props.minHeight / GridDefaults.DEFAULT_GRID_ROW_HEIGHT - 1,
newRows,
);
setRows(rowsToPersist);
if (canDropTargetExtend) {
updateCanvasSnapRows(props.widgetId, rowsToPersist);
}
},
[
props.minHeight,
props.widgetId,
occupiedSpacesByChildren,
canDropTargetExtend,
],
);

/* Update the rows of the main container based on the current widget's (dragging/resizing) bottom row */
const updateDropTargetRows = useCallback(
(widgetId: string, widgetBottomRow: number) => {
if (canDropTargetExtend) {
const newRows = calculateDropTargetRows(
widgetId,
widgetBottomRow,
props.minHeight / GridDefaults.DEFAULT_GRID_ROW_HEIGHT - 1,
occupiedSpacesByChildren,
);
if (rows < newRows) {
setRows(newRows);
return true;
}
return false;
}
return false;
}
return false;
};
},
[props.minHeight, occupiedSpacesByChildren, canDropTargetExtend, rows],
);

const isChildFocused =
!!childWidgets &&
Expand Down Expand Up @@ -215,7 +228,7 @@ export function DropTargetComponent(props: DropTargetComponentProps) {
props.snapRowSpace,
widget,
dropTargetOffset,
spacesOccupiedBySiblingWidgets,
occupiedSpacesByChildren,
rows,
GridDefaults.DEFAULT_GRID_COLUMNS,
);
Expand Down Expand Up @@ -258,10 +271,17 @@ export function DropTargetComponent(props: DropTargetComponentProps) {
: "0px 0px 0px 1px transparent";
const dropRef = !props.dropDisabled ? drop : undefined;

// memoizing context values
const contextValue = useMemo(() => {
return {
updateDropTargetRows,
persistDropTargetRows,
occupiedSpaces: occupiedSpacesByChildren,
};
}, [updateDropTargetRows, persistDropTargetRows, occupiedSpacesByChildren]);

return (
<DropTargetContext.Provider
value={{ updateDropTargetRows, persistDropTargetRows }}
>
<DropTargetContext.Provider value={contextValue}>
<StyledDropTarget
className={"t--drop-target"}
onClick={handleFocus}
Expand All @@ -281,7 +301,7 @@ export function DropTargetComponent(props: DropTargetComponentProps) {
isOver={isExactlyOver}
isResizing={isChildResizing}
noPad={props.noPad || false}
occupiedSpaces={spacesOccupiedBySiblingWidgets}
occupiedSpaces={occupiedSpacesByChildren}
onBoundsUpdate={handleBoundsUpdate}
parentCols={props.snapColumns}
parentColumnWidth={props.snapColumnSpace}
Expand Down
18 changes: 8 additions & 10 deletions app/client/src/components/editorComponents/ResizableComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import {
import AnalyticsUtil from "utils/AnalyticsUtil";
import { scrollElementIntoParentCanvasView } from "utils/helpers";
import { getNearestParentCanvas } from "utils/generators";
import { getOccupiedSpaces } from "selectors/editorSelectors";
import { commentModeSelector } from "selectors/commentsSelectors";
import { useWidgetSelection } from "utils/hooks/useWidgetSelection";

Expand All @@ -51,11 +50,12 @@ export const ResizableComponent = memo(function ResizableComponent(
const resizableRef = useRef<HTMLDivElement>(null);
// Fetch information from the context
const { updateWidget } = useContext(EditorContext);
const occupiedSpaces = useSelector(getOccupiedSpaces);

const { persistDropTargetRows, updateDropTargetRows } = useContext(
DropTargetContext,
);
const {
occupiedSpaces: occupiedSpacesBySiblingWidgets,
persistDropTargetRows,
updateDropTargetRows,
} = useContext(DropTargetContext);

const isCommentMode = useSelector(commentModeSelector);

Expand All @@ -79,11 +79,6 @@ export const ResizableComponent = memo(function ResizableComponent(
(state: AppState) => state.ui.widgetDragResize.isResizing,
);

const occupiedSpacesBySiblingWidgets =
occupiedSpaces && props.parentId && occupiedSpaces[props.parentId]
? occupiedSpaces[props.parentId]
: undefined;

// isFocused (string | boolean) -> isWidgetFocused (boolean)
const isWidgetFocused =
focusedWidget === props.widgetId ||
Expand Down Expand Up @@ -303,4 +298,7 @@ export const ResizableComponent = memo(function ResizableComponent(
</Resizable>
);
});

ResizableComponent.displayName = "ResizableComponent";

export default ResizableComponent;
25 changes: 21 additions & 4 deletions app/client/src/reducers/evaluationReducers/treeReducer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createImmerReducer } from "utils/AppsmithUtils";
import { DataTree } from "entities/DataTree/dataTreeFactory";
import { ReduxAction, ReduxActionTypes } from "constants/ReduxActionConstants";
import { applyChange, Diff } from "deep-diff";
import { DataTree } from "entities/DataTree/dataTreeFactory";
import { createImmerReducer } from "utils/AppsmithUtils";

export type EvaluatedTreeState = DataTree;

Expand All @@ -9,8 +10,24 @@ const initialState: EvaluatedTreeState = {};
const evaluatedTreeReducer = createImmerReducer(initialState, {
[ReduxActionTypes.SET_EVALUATED_TREE]: (
state: EvaluatedTreeState,
action: ReduxAction<DataTree>,
) => action.payload,
action: ReduxAction<{
dataTree: DataTree;
updates: Diff<DataTree, DataTree>[];
removedPaths: [string];
}>,
) => {
const { dataTree, updates } = action.payload;
if (Object.keys(dataTree).length) {
return dataTree;
}
for (const update of updates) {
// Null check for typescript
if (!Array.isArray(update.path) || update.path.length === 0) {
continue;
}
applyChange(state, undefined, update);
}
},
[ReduxActionTypes.FETCH_PAGE_INIT]: () => initialState,
});

Expand Down
54 changes: 32 additions & 22 deletions app/client/src/sagas/EvaluationsSaga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ import {
ReduxActionTypes,
ReduxActionWithoutPayload,
} from "constants/ReduxActionConstants";
import { getUnevaluatedDataTree } from "selectors/dataTreeSelectors";
import {
getDataTree,
getUnevaluatedDataTree,
} from "selectors/dataTreeSelectors";
import WidgetFactory, { WidgetTypeConfigMap } from "../utils/WidgetFactory";
import { GracefulWorkerService } from "utils/WorkerUtil";
import Worker from "worker-loader!../workers/evaluation.worker";
Expand Down Expand Up @@ -312,7 +315,6 @@ function* logSuccessfulBindings(
function* updateTernDefinitions(
dataTree: DataTree,
evaluationOrder: string[],
removedPaths: string[],
isFirstEvaluation: boolean,
) {
const updatedEntities: Set<string> = new Set();
Expand All @@ -333,12 +335,12 @@ function* updateTernDefinitions(
TernServer.updateDef(name, def);
}
});
removedPaths.forEach((path) => {
// No '.' means that the path is an entity name
if (path.split(".").length === 1) {
TernServer.removeDef(path);
}
});
// removedPaths.forEach((path) => {
// // No '.' means that the path is an entity name
// if (path.split(".").length === 1) {
// TernServer.removeDef(path);
// }
// });
}

function* postEvalActionDispatcher(
Expand All @@ -355,6 +357,7 @@ function* evaluateTreeSaga(
) {
const unevalTree = yield select(getUnevaluatedDataTree);
log.debug({ unevalTree });
console.log(unevalTree);
PerformanceTracker.startAsyncTracking(
PerformanceTransactionName.DATA_TREE_EVALUATION,
);
Expand All @@ -372,33 +375,40 @@ function* evaluateTreeSaga(
errors,
evaluationOrder,
logs,
removedPaths,
updates,
} = workerResponse;
PerformanceTracker.stopAsyncTracking(
PerformanceTransactionName.DATA_TREE_EVALUATION,
);
log.debug({ dataTree: dataTree });
logs.forEach((evalLog: any) => log.debug(evalLog));
yield call(evalErrorHandler, errors, dataTree, evaluationOrder);
yield fork(logSuccessfulBindings, unevalTree, dataTree, evaluationOrder);
yield fork(
updateTernDefinitions,
dataTree,
evaluationOrder,
removedPaths,
isFirstEvaluation,
);

PerformanceTracker.startAsyncTracking(
PerformanceTransactionName.SET_EVALUATED_TREE,
);
yield put({
type: ReduxActionTypes.SET_EVALUATED_TREE,
payload: dataTree,
payload: { dataTree, updates },
});
PerformanceTracker.stopAsyncTracking(
PerformanceTransactionName.SET_EVALUATED_TREE,
);

const updatedDataTree = yield select(getDataTree);

log.debug({ dataTree: updatedDataTree });
logs.forEach((evalLog: any) => log.debug(evalLog));
yield call(evalErrorHandler, errors, updatedDataTree, evaluationOrder);
yield fork(
logSuccessfulBindings,
unevalTree,
updatedDataTree,
evaluationOrder,
);
yield fork(
updateTernDefinitions,
updatedDataTree,
evaluationOrder,
isFirstEvaluation,
);

yield put({
type: ReduxActionTypes.SET_EVALUATION_INVERSE_DEPENDENCY_MAP,
payload: { inverseDependencyMap: dependencies },
Expand Down
Loading