Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix: performance improvements for js editor (#21492)

Browse Source 
## Description

TL;DR performance improvements for js editor

- fix entityNavigationData generation (to prevent unnecessary component
updates)
- in codeEditor/index.ts (`addThisReference` was creating a new object
everytime)
- in navigationSelector.ts (use `getJSCollections` instead of
`getJSCollectionsForCurrentPage`, which created a new object everytime,
even if actions were not updated)
- combine markers for navigation and peek overlay to reduce the total
number of markers
- clear and add marks for only the edited lines instead of the whole
file

Note: once a js object is saved, it's still going to trigger a whole
file clear and marking.
Because, it's an entity update which needs a whole refresh of the
markers.

Fixes #21467


## Media
Case: Adding a blank space in js editor.

### Reduced un-necessary clears and marks:
#### Before: 

![image](https://user-images.githubusercontent.com/66776129/225681826-980e7ea6-5a9f-45ac-9de8-b6b5d73078d7.png)

####After:

![image](https://user-images.githubusercontent.com/66776129/225681997-064db06b-b208-4cdf-8ead-f67de8ea2d34.png)

---

### Reduced entity marker called count:
https://www.loom.com/share/23719f8dfde8457ea0a86f44500ec34a

---

### Reduced markers count:
#### Before:

![image](https://user-images.githubusercontent.com/66776129/225792984-1eb4082e-fbfe-4fa2-bad8-6797d7095673.png)

#### After:

![image](https://user-images.githubusercontent.com/66776129/225793028-04480724-8822-4934-8264-375ba7bd95cd.png)



## Type of change
- Bug fix (non-breaking change which fixes an issue)


## How Has This Been Tested?
- Manual

### Test Plan
> Add Testsmith test cases links that relate to this PR

### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)


## Checklist:
### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


### QA activity:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or
manual QA
- [ ] Organized project review call with relevant stakeholders after
Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
This commit is contained in:
Anand Srinivasan 2023-03-21 10:39:43 +05:30 committed by GitHub
parent 20b46ea48b
commit 76f22399e5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 247 additions and 214 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/client/src/components/editorComponents/CodeEditor/EditorConfig.ts
View File

@ -73,6 +73,8 @@ export type Hinter = {
export type MarkHelper = (
editor: CodeMirror.Editor,
entityNavigationData: EntityNavigationData,
from?: CodeMirror.Position,
to?: CodeMirror.Position,
) => void;
export enum CodeEditorBorder {

41
app/client/src/components/editorComponents/CodeEditor/MarkHelpers/bindingMarker.ts Normal file
View File

@ -0,0 +1,41 @@
import type CodeMirror from "codemirror";
import { AUTOCOMPLETE_MATCH_REGEX } from "constants/BindingsConstants";
import type { MarkHelper } from "components/editorComponents/CodeEditor/EditorConfig";
export const bindingMarker: MarkHelper = (editor: CodeMirror.Editor) => {
editor.eachLine((line: CodeMirror.LineHandle) => {
const lineNo = editor.getLineNumber(line) || 0;
let match;
while ((match = AUTOCOMPLETE_MATCH_REGEX.exec(line.text)) != null) {
const opening = {
start: match.index,
end: match.index + 2,
};
const ending = {
start: AUTOCOMPLETE_MATCH_REGEX.lastIndex - 2,
end: AUTOCOMPLETE_MATCH_REGEX.lastIndex,
};
editor.markText(
{ ch: ending.start, line: lineNo },
{ ch: ending.end, line: lineNo },
{
className: "binding-brackets",
},
);
editor.markText(
{ ch: opening.start, line: lineNo },
{ ch: opening.end, line: lineNo },
{
className: "binding-brackets",
},
);
editor.markText(
{ ch: opening.start, line: lineNo },
{ ch: ending.end, line: lineNo },
{
className: "binding-highlight",
},
);
}
});
};

148
app/client/src/components/editorComponents/CodeEditor/MarkHelpers/entityMarker.ts Normal file
View File

@ -0,0 +1,148 @@
import type {
EntityNavigationData,
NavigationData,
} from "selectors/navigationSelectors";
import type { MarkHelper } from "../EditorConfig";
export const NAVIGATE_TO_ATTRIBUTE = "data-navigate-to";
export const NAVIGATION_CLASSNAME = "navigable-entity-highlight";
const hasReference = (token: CodeMirror.Token) => {
const tokenString = token.string;
return token.type === "variable" || tokenString === "this";
};
export const PEEKABLE_CLASSNAME = "peekable-entity-highlight";
export const PEEKABLE_ATTRIBUTE = "peek-data";
export const PEEKABLE_LINE = "peek-line";
export const PEEKABLE_CH_START = "peek-ch-start";
export const PEEKABLE_CH_END = "peek-ch-end";
export const PEEK_STYLE_PERSIST_CLASS = "peek-style-persist";
export const entityMarker: MarkHelper = (
editor: CodeMirror.Editor,
entityNavigationData,
from,
to,
) => {
let markers: CodeMirror.TextMarker[] = [];
if (from && to) {
markers = editor.findMarks(
{
line: from.line,
ch: 0,
},
{
line: to.line,
// when a line is deleted?
ch: editor.getLine(to.line).length - 1,
},
);
clearMarkers(markers);
editor.eachLine(from.line, to.line, (line: CodeMirror.LineHandle) => {
addMarksForLine(editor, line, entityNavigationData);
});
} else {
markers = editor.getAllMarks();
clearMarkers(markers);
editor.eachLine((line: CodeMirror.LineHandle) => {
addMarksForLine(editor, line, entityNavigationData);
});
}
};
const addMarksForLine = (
editor: CodeMirror.Editor,
line: CodeMirror.LineHandle,
entityNavigationData: EntityNavigationData,
) => {
const lineNo = editor.getLineNumber(line) || 0;
const tokens = editor.getLineTokens(lineNo);
tokens.forEach((token) => {
const tokenString = token.string;
if (hasReference(token) && tokenString in entityNavigationData) {
const data = entityNavigationData[tokenString];
if (data.navigable || data.peekable) {
editor.markText(
{ ch: token.start, line: lineNo },
{ ch: token.end, line: lineNo },
getMarkOptions(data, token, lineNo),
);
}
addMarksForChildren(
entityNavigationData[tokenString],
lineNo,
token.end,
editor,
);
}
});
};
const addMarksForChildren = (
navigationData: NavigationData,
lineNo: number,
tokenEnd: number,
editor: CodeMirror.Editor,
) => {
const childNodes = navigationData.children || {};
if (Object.keys(childNodes).length) {
const token = editor.getTokenAt(
{
ch: tokenEnd + 2,
line: lineNo,
},
true,
);
if (token.string in childNodes) {
const childLink = childNodes[token.string];
if (childLink.navigable || childLink.peekable) {
editor.markText(
{ ch: token.start, line: lineNo },
{ ch: token.end, line: lineNo },
getMarkOptions(childLink, token, lineNo),
);
}
addMarksForChildren(childNodes[token.string], lineNo, token.end, editor);
}
}
};
const getMarkOptions = (
data: NavigationData,
token: CodeMirror.Token,
lineNo: number,
): CodeMirror.TextMarkerOptions => {
return {
className: `${data.navigable ? NAVIGATION_CLASSNAME : ""} ${
data.peekable ? PEEKABLE_CLASSNAME : ""
}`,
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
attributes: {
...(data.navigable && {
[NAVIGATE_TO_ATTRIBUTE]: `${data.name}`,
}),
...(data.peekable && {
[PEEKABLE_ATTRIBUTE]: data.name,
[PEEKABLE_CH_START]: token.start,
[PEEKABLE_CH_END]: token.end,
[PEEKABLE_LINE]: lineNo,
}),
},
atomic: false,
title: data.name,
};
};
const clearMarkers = (markers: CodeMirror.TextMarker[]) => {
markers.forEach((marker) => {
if (
marker.className?.includes(NAVIGATION_CLASSNAME) ||
marker.className?.includes(PEEKABLE_CLASSNAME)
)
marker.clear();
});
};

50
app/client/src/components/editorComponents/CodeEditor/index.tsx
View File

@ -55,8 +55,8 @@ import {
EditorWrapper,
IconContainer,
} from "components/editorComponents/CodeEditor/styledComponents";
import { bindingMarker } from "components/editorComponents/CodeEditor/MarkHelpers/bindingMarker";
import {
bindingMarker,
entityMarker,
NAVIGATE_TO_ATTRIBUTE,
PEEKABLE_ATTRIBUTE,
@ -64,7 +64,7 @@ import {
PEEKABLE_CH_START,
PEEKABLE_LINE,
PEEK_STYLE_PERSIST_CLASS,
} from "components/editorComponents/CodeEditor/markHelpers";
} from "components/editorComponents/CodeEditor/MarkHelpers/entityMarker";
import { bindingHint } from "components/editorComponents/CodeEditor/hintHelpers";
import BindingPrompt from "./BindingPrompt";
import { showBindingPrompt } from "./BindingPromptHelper";
@ -509,11 +509,16 @@ class CodeEditor extends Component<Props, State> {
this.setEditorInput("");
}
CodeEditor.updateMarkings(
this.editor,
this.props.marking,
this.props.entitiesForNavigation,
);
if (
this.props.entitiesForNavigation !== prevProps.entitiesForNavigation ||
this.props.marking !== prevProps.marking
) {
CodeEditor.updateMarkings(
this.editor,
this.props.marking,
this.props.entitiesForNavigation,
);
}
});
}
@ -962,11 +967,13 @@ class CodeEditor extends Component<Props, State> {
}
}
if (this.editor) {
if (this.editor && changeObj) {
CodeEditor.updateMarkings(
this.editor,
this.props.marking,
this.props.entitiesForNavigation,
changeObj.from,
changeObj.to,
);
}
};
@ -1127,8 +1134,10 @@ class CodeEditor extends Component<Props, State> {
editor: CodeMirror.Editor,
marking: Array<MarkHelper>,
entityNavigationData: EntityNavigationData,
from?: CodeMirror.Position,
to?: CodeMirror.Position,
) => {
marking.forEach((helper) => helper(editor, entityNavigationData));
marking.forEach((helper) => helper(editor, entityNavigationData, from, to));
};
updatePropertyValue(value: string, cursor?: number) {
@ -1362,12 +1371,10 @@ const mapStateToProps = (state: AppState, props: EditorProps) => ({
state,
getEditorIdentifier(props),
),
entitiesForNavigation: props.isJSObject
? addThisReference(
getEntitiesForNavigation(state),
props.dataTreePath?.split(".")[0],
)
: getEntitiesForNavigation(state),
entitiesForNavigation: getEntitiesForNavigation(
state,
props.dataTreePath?.split(".")[0],
),
});
const mapDispatchToProps = (dispatch: any) => ({
@ -1383,16 +1390,3 @@ const mapDispatchToProps = (dispatch: any) => ({
export default Sentry.withProfiler(
connect(mapStateToProps, mapDispatchToProps)(CodeEditor),
);
const addThisReference = (
navigationData: EntityNavigationData,
entityName?: string,
) => {
if (entityName && entityName in navigationData) {
return {
...navigationData,
this: navigationData[entityName],
};
}
return navigationData;
};

178
app/client/src/components/editorComponents/CodeEditor/markHelpers.ts
View File

@ -1,178 +0,0 @@
import type CodeMirror from "codemirror";
import { AUTOCOMPLETE_MATCH_REGEX } from "constants/BindingsConstants";
import type { MarkHelper } from "components/editorComponents/CodeEditor/EditorConfig";
import type { NavigationData } from "selectors/navigationSelectors";
export const bindingMarker: MarkHelper = (editor: CodeMirror.Editor) => {
editor.eachLine((line: CodeMirror.LineHandle) => {
const lineNo = editor.getLineNumber(line) || 0;
let match;
while ((match = AUTOCOMPLETE_MATCH_REGEX.exec(line.text)) != null) {
const opening = {
start: match.index,
end: match.index + 2,
};
const ending = {
start: AUTOCOMPLETE_MATCH_REGEX.lastIndex - 2,
end: AUTOCOMPLETE_MATCH_REGEX.lastIndex,
};
editor.markText(
{ ch: ending.start, line: lineNo },
{ ch: ending.end, line: lineNo },
{
className: "binding-brackets",
},
);
editor.markText(
{ ch: opening.start, line: lineNo },
{ ch: opening.end, line: lineNo },
{
className: "binding-brackets",
},
);
editor.markText(
{ ch: opening.start, line: lineNo },
{ ch: ending.end, line: lineNo },
{
className: "binding-highlight",
},
);
}
});
};
export const NAVIGATE_TO_ATTRIBUTE = "data-navigate-to";
export const NAVIGATION_CLASSNAME = "navigable-entity-highlight";
const hasReference = (token: CodeMirror.Token) => {
const tokenString = token.string;
return token.type === "variable" || tokenString === "this";
};
export const PEEKABLE_CLASSNAME = "peekaboo";
export const PEEKABLE_ATTRIBUTE = "peek-data";
export const PEEKABLE_LINE = "peek-line";
export const PEEKABLE_CH_START = "peek-ch-start";
export const PEEKABLE_CH_END = "peek-ch-end";
export const PEEK_STYLE_PERSIST_CLASS = "peek-style-persist";
export const entityMarker: MarkHelper = (
editor: CodeMirror.Editor,
entityNavigationData,
) => {
editor
.getAllMarks()
.filter(
(marker) =>
marker.className === NAVIGATION_CLASSNAME ||
marker.className === PEEKABLE_CLASSNAME,
)
.forEach((marker) => marker.clear());
editor.eachLine((line: CodeMirror.LineHandle) => {
const lineNo = editor.getLineNumber(line) || 0;
const tokens = editor.getLineTokens(lineNo);
tokens.forEach((token) => {
const tokenString = token.string;
if (hasReference(token) && tokenString in entityNavigationData) {
const data = entityNavigationData[tokenString];
editor.markText(
{ ch: token.start, line: lineNo },
{ ch: token.end, line: lineNo },
{
className: NAVIGATION_CLASSNAME,
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
attributes: {
[NAVIGATE_TO_ATTRIBUTE]: `${data.name}`,
},
atomic: false,
title: data.name,
},
);
if (data.peekable) {
editor.markText(
{ ch: token.start, line: lineNo },
{ ch: token.end, line: lineNo },
{
className: PEEKABLE_CLASSNAME,
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
attributes: {
[PEEKABLE_ATTRIBUTE]: data.name,
[PEEKABLE_CH_START]: token.start,
[PEEKABLE_CH_END]: token.end,
[PEEKABLE_LINE]: lineNo,
},
atomic: false,
title: data.name,
},
);
}
addMarksForChildren(
entityNavigationData[tokenString],
lineNo,
token.end,
editor,
);
}
});
});
};
const addMarksForChildren = (
navigationData: NavigationData,
lineNo: number,
tokenEnd: number,
editor: CodeMirror.Editor,
) => {
const childNodes = navigationData.children || {};
if (Object.keys(childNodes).length) {
const token = editor.getTokenAt(
{
ch: tokenEnd + 2,
line: lineNo,
},
true,
);
if (token.string in childNodes) {
const childLink = childNodes[token.string];
if (childLink.navigable) {
editor.markText(
{ ch: token.start, line: lineNo },
{ ch: token.end, line: lineNo },
{
className: NAVIGATION_CLASSNAME,
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
attributes: {
[NAVIGATE_TO_ATTRIBUTE]: `${childLink.name}`,
},
atomic: false,
title: childLink.name,
},
);
}
if (childLink.peekable) {
editor.markText(
{ ch: token.start, line: lineNo },
{ ch: token.end, line: lineNo },
{
className: PEEKABLE_CLASSNAME,
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
attributes: {
[PEEKABLE_ATTRIBUTE]: childLink.name,
[PEEKABLE_CH_START]: token.start,
[PEEKABLE_CH_END]: token.end,
[PEEKABLE_LINE]: lineNo,
},
atomic: false,
title: childLink.name,
},
);
}
addMarksForChildren(childNodes[token.string], lineNo, token.end, editor);
}
}
};

2
app/client/src/components/editorComponents/CodeEditor/styledComponents.ts
View File

@ -11,7 +11,7 @@ import {
NAVIGATION_CLASSNAME,
PEEKABLE_CLASSNAME,
PEEK_STYLE_PERSIST_CLASS,
} from "./markHelpers";
} from "./MarkHelpers/entityMarker";
const getBorderStyle = (
props: { theme: Theme } & {

7
app/client/src/components/editorComponents/form/fields/EmbeddedDatasourcePathField.tsx
View File

@ -22,10 +22,9 @@ import {
TabBehaviour,
EditorSize,
} from "components/editorComponents/CodeEditor/EditorConfig";
import {
bindingMarker,
entityMarker,
} from "components/editorComponents/CodeEditor/markHelpers";
import { bindingMarker } from "components/editorComponents/CodeEditor/MarkHelpers/bindingMarker";
import { entityMarker } from "components/editorComponents/CodeEditor/MarkHelpers/entityMarker";
import { bindingHint } from "components/editorComponents/CodeEditor/hintHelpers";
import StoreAsDatasource from "components/editorComponents/StoreAsDatasource";
import { urlGroupsRegexExp } from "constants/AppsmithActionConstants/ActionConstants";

1
app/client/src/selectors/entitiesSelector.ts
View File

@ -382,6 +382,7 @@ export const getActionsForCurrentPage = createSelector(
},
);
// Note: getJSCollectionsForCurrentPage (returns a new object everytime)
export const getJSCollectionsForCurrentPage = createSelector(
getCurrentPageId,
getJSCollections,

32
app/client/src/selectors/navigationSelectors.ts
View File

@ -6,7 +6,7 @@ import { ENTITY_TYPE } from "entities/DataTree/dataTreeFactory";
import { createSelector } from "reselect";
import {
getActionsForCurrentPage,
getJSCollectionsForCurrentPage,
getJSCollections,
getPlugins,
} from "selectors/entitiesSelector";
import { getWidgets } from "sagas/selectors";
@ -19,6 +19,7 @@ import { createNavData } from "utils/NavigationSelector/common";
import { getWidgetChildrenNavData } from "utils/NavigationSelector/WidgetChildren";
import { getJsChildrenNavData } from "utils/NavigationSelector/JsChildren";
import { getAppsmithNavData } from "utils/NavigationSelector/AppsmithNavData";
import { isJSObject } from "ce/workers/Evaluation/evaluationUtils";
export type NavigationData = {
name: string;
@ -36,11 +37,22 @@ export type EntityNavigationData = Record<string, NavigationData>;
export const getEntitiesForNavigation = createSelector(
getActionsForCurrentPage,
getPlugins,
getJSCollectionsForCurrentPage,
getJSCollections,
getWidgets,
getCurrentPageId,
getDataTree,
(actions, plugins, jsActions, widgets, pageId, dataTree: DataTree) => {
(_: any, entityName: string | undefined) => entityName,
(
actions,
plugins,
jsActions,
widgets,
pageId,
dataTree: DataTree,
entityName: string | undefined,
) => {
// data tree retriggers this
jsActions = jsActions.filter((a) => a.config.pageId === pageId);
const navigationData: EntityNavigationData = {};
if (!dataTree) return navigationData;
@ -49,6 +61,7 @@ export const getEntitiesForNavigation = createSelector(
(plugin) => plugin.id === action.config.pluginId,
);
const config = getActionConfig(action.config.pluginType);
// dataTree used to get entityDefinitions and peekData
const result = getActionChildrenNavData(action, dataTree);
if (!config) return;
navigationData[action.config.name] = createNavData({
@ -68,6 +81,7 @@ export const getEntitiesForNavigation = createSelector(
});
jsActions.forEach((jsAction) => {
// dataTree for null check and peekData
const result = getJsChildrenNavData(jsAction, pageId, dataTree);
navigationData[jsAction.config.name] = createNavData({
id: jsAction.config.id,
@ -81,6 +95,7 @@ export const getEntitiesForNavigation = createSelector(
});
Object.values(widgets).forEach((widget) => {
// dataTree to get entityDefinitions, for url (can use getWidgetByName?) and peekData
const result = getWidgetChildrenNavData(widget, dataTree, pageId);
navigationData[widget.widgetName] = createNavData({
id: widget.widgetId,
@ -92,9 +107,20 @@ export const getEntitiesForNavigation = createSelector(
children: result?.childNavData || {},
});
});
// dataTree to get entity definitions and peekData
navigationData["appsmith"] = getAppsmithNavData(
dataTree.appsmith as AppsmithEntity,
);
if (
entityName &&
isJSObject(dataTree[entityName]) &&
entityName in navigationData
) {
return {
...navigationData,
this: navigationData[entityName],
};
}
return navigationData;
},
);