fix: trigger fields linting issue (#11875)

* initial fix

* fix warnings

* log evaluation order length

* log time

* logs

* re-order

* rename

* onclick vs release logs

* unit test for loading properties

* clean up

* ends with .tableData

* add comment
This commit is contained in:
Anand Srinivasan 2022-03-31 17:07:18 +05:30 committed by GitHub
parent e013c562a0
commit 186a97c699
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 175 additions and 58 deletions

View File

@ -121,6 +121,7 @@ class WidgetFactory {
WidgetType,
readonly PropertyPaneConfig[]
> = new Map();
static loadingProperties: Map<WidgetType, Array<RegExp>> = new Map();
static widgetConfigMap: Map<
WidgetType,
@ -134,6 +135,7 @@ class WidgetFactory {
defaultPropertiesMap: Record<string, string>,
metaPropertiesMap: Record<string, any>,
propertyPaneConfig?: PropertyPaneConfig[],
loadingProperties?: Array<RegExp>,
) {
if (!this.widgetTypes[widgetType]) {
this.widgetTypes[widgetType] = widgetType;
@ -141,6 +143,8 @@ class WidgetFactory {
this.derivedPropertiesMap.set(widgetType, derivedPropertiesMap);
this.defaultPropertiesMap.set(widgetType, defaultPropertiesMap);
this.metaPropertiesMap.set(widgetType, metaPropertiesMap);
loadingProperties &&
this.loadingProperties.set(widgetType, loadingProperties);
if (propertyPaneConfig) {
const validatedPropertyPaneConfig = validatePropertyPaneConfig(
@ -235,6 +239,10 @@ class WidgetFactory {
return map;
}
static getLoadingProperties(type: WidgetType): Array<RegExp> | undefined {
return this.loadingProperties.get(type);
}
static getWidgetTypeConfigMap(): WidgetTypeConfigMap {
const typeConfigMap: WidgetTypeConfigMap = {};
WidgetFactory.getWidgetTypes().forEach((type) => {

View File

@ -7,9 +7,10 @@ import {
} from "entities/DataTree/dataTreeFactory";
import {
findLoadingEntities,
getEntityDependants,
getEntityDependantPaths,
groupAndFilterDependantsMap,
} from "utils/WidgetLoadingStateUtils";
import WidgetFactory from "./WidgetFactory";
const JS_object_tree: DataTreeJSAction = {
pluginType: PluginType.JS,
@ -68,14 +69,61 @@ const Query_tree: DataTreeAction = {
isLoading: false,
};
const Api_tree: DataTreeAction = {
data: {},
actionId: "",
config: {},
pluginType: PluginType.API,
pluginId: "",
name: "",
run: {},
clear: {},
dynamicBindingPathList: [],
bindingPaths: {},
ENTITY_TYPE: ENTITY_TYPE.ACTION,
dependencyMap: {},
logBlackList: {},
datasourceUrl: "",
responseMeta: {
isExecutionSuccess: true,
},
isLoading: false,
};
const Table_tree: DataTreeWidget = {
ENTITY_TYPE: ENTITY_TYPE.WIDGET,
bindingPaths: {},
triggerPaths: {},
validationPaths: {},
logBlackList: {},
propertyOverrideDependency: {},
overridingPropertyPaths: {},
privateWidgets: {},
widgetId: "",
type: "TABLE_WIDGET",
widgetName: "",
renderMode: "CANVAS",
version: 0,
parentColumnSpace: 0,
parentRowSpace: 0,
leftColumn: 0,
rightColumn: 0,
topRow: 0,
bottomRow: 0,
isLoading: false,
animateLoading: true,
};
const baseDataTree = {
JS_file: { ...JS_object_tree, name: "JS_file" },
Select1: { ...Select_tree, name: "Select1" },
Select2: { ...Select_tree, name: "Select2" },
Select3: { ...Select_tree, name: "Select3" },
Table1: { ...Table_tree, name: "Table1" },
Query1: { ...Query_tree, name: "Query1" },
Query2: { ...Query_tree, name: "Query2" },
Query3: { ...Query_tree, name: "Query3" },
Api1: { ...Api_tree, name: "Api1" },
};
describe("Widget loading state utils", () => {
@ -116,6 +164,22 @@ describe("Widget loading state utils", () => {
],
};
beforeAll(() => {
// mock WidgetFactory.getLoadingProperties
const loadingPropertiesMap = new Map<string, RegExp[]>();
loadingPropertiesMap.set("TABLE_WIDGET", [/.tableData$/]);
jest
.spyOn(WidgetFactory, "getLoadingProperties")
.mockImplementation((widgetType) =>
loadingPropertiesMap.get(widgetType),
);
});
afterAll(() => {
jest.restoreAllMocks();
});
// Select1.options -> JS_file.func1 -> Query1.data
it("handles linear dependencies", () => {
const loadingEntites = findLoadingEntities(
@ -247,6 +311,20 @@ describe("Widget loading state utils", () => {
});
expect(loadingEntites).toStrictEqual(new Set(["Select2"]));
});
it("includes loading properties", () => {
const loadingEntites = findLoadingEntities(["Api1"], baseDataTree, {
"Api1.data": ["Table1.tableData"],
});
expect(loadingEntites).toStrictEqual(new Set(["Table1"]));
});
it("ignores non-loading properties", () => {
const loadingEntites = findLoadingEntities(["Api1"], baseDataTree, {
"Api1.run": ["Table1.primaryColumns.action.onClick"],
});
expect(loadingEntites).toStrictEqual(new Set());
});
});
describe("groupAndFilterDependantsMap", () => {
@ -327,10 +405,10 @@ describe("Widget loading state utils", () => {
});
});
describe("getEntityDependants", () => {
describe("getEntityDependantPaths", () => {
// Select1.options -> JS_file.func1 -> Query1.data
it("handles simple dependency", () => {
const dependants = getEntityDependants(
const dependants = getEntityDependantPaths(
["Query1"],
{
Query1: {
@ -342,16 +420,15 @@ describe("Widget loading state utils", () => {
},
new Set<string>(),
);
expect(dependants).toStrictEqual({
names: new Set(["JS_file", "Select1"]),
fullPaths: new Set(["JS_file.func1", "Select1.options"]),
});
expect(dependants).toStrictEqual(
new Set(["JS_file.func1", "Select1.options"]),
);
});
// Select1.options -> JS_file.func1 -> Query1.data
// Select2.options -> JS_file.func2 -> Query1.data
it("handles multiple dependencies", () => {
const dependants = getEntityDependants(
const dependants = getEntityDependantPaths(
["Query1"],
{
Query1: {
@ -364,19 +441,18 @@ describe("Widget loading state utils", () => {
},
new Set<string>(),
);
expect(dependants).toStrictEqual({
names: new Set(["JS_file", "Select1", "Select2"]),
fullPaths: new Set([
expect(dependants).toStrictEqual(
new Set([
"JS_file.func1",
"Select1.options",
"JS_file.func2",
"Select2.options",
]),
});
);
});
it("handles specific entity paths", () => {
const dependants = getEntityDependants(
const dependants = getEntityDependantPaths(
["JS_file.func2"], // specific path
{
Query1: {
@ -392,15 +468,12 @@ describe("Widget loading state utils", () => {
},
new Set<string>(),
);
expect(dependants).toStrictEqual({
names: new Set(["Select2"]),
fullPaths: new Set(["Select2.options"]),
});
expect(dependants).toStrictEqual(new Set(["Select2.options"]));
});
// Select1.options -> JS_file.func1 -> JS_file.internalFunc -> Query1.data
it("handles JS self-dependencies", () => {
const dependants = getEntityDependants(
const dependants = getEntityDependantPaths(
["Query1"],
{
Query1: {
@ -413,19 +486,14 @@ describe("Widget loading state utils", () => {
},
new Set<string>(),
);
expect(dependants).toStrictEqual({
names: new Set(["JS_file", "Select1"]),
fullPaths: new Set([
"JS_file.internalFunc",
"JS_file.func1",
"Select1.options",
]),
});
expect(dependants).toStrictEqual(
new Set(["JS_file.internalFunc", "JS_file.func1", "Select1.options"]),
);
});
// Select1.options -> JS_file.func -> JS_file.internalFunc1 -> JS_file.internalFunc2 -> Query1.data
it("handles nested JS self-dependencies", () => {
const dependants = getEntityDependants(
const dependants = getEntityDependantPaths(
["Query1"],
{
Query1: {
@ -439,15 +507,14 @@ describe("Widget loading state utils", () => {
},
new Set<string>(),
);
expect(dependants).toStrictEqual({
names: new Set(["JS_file", "Select1"]),
fullPaths: new Set([
expect(dependants).toStrictEqual(
new Set([
"JS_file.internalFunc1",
"JS_file.internalFunc2",
"JS_file.func",
"Select1.options",
]),
});
);
});
/* Select1.options -> JS.func1 -> Query1.data,
@ -458,7 +525,7 @@ describe("Widget loading state utils", () => {
Only Select2 should be listed, not Select1.
*/
it("handles selective dependencies in same JS file", () => {
const dependants = getEntityDependants(
const dependants = getEntityDependantPaths(
["Query2"],
{
Query1: {
@ -474,10 +541,9 @@ describe("Widget loading state utils", () => {
},
new Set<string>(),
);
expect(dependants).toStrictEqual({
names: new Set(["JS_file", "Select2"]),
fullPaths: new Set(["JS_file.func2", "Select2.options"]),
});
expect(dependants).toStrictEqual(
new Set(["JS_file.func2", "Select2.options"]),
);
});
});
});

View File

@ -2,6 +2,7 @@ import { DataTree } from "entities/DataTree/dataTreeFactory";
import { get, set } from "lodash";
import { isJSObject } from "workers/evaluationUtils";
import { DependencyMap } from "./DynamicBindingUtils";
import WidgetFactory from "./WidgetFactory";
type GroupedDependencyMap = Record<string, DependencyMap>;
@ -57,14 +58,13 @@ export const groupAndFilterDependantsMap = (
return entitiesDepMap;
};
// get entities that depend on a given list of entites
// e.g. widgets that depend on a list of actions
export const getEntityDependants = (
// get entity paths that depend on a given list of entites
// e.g. widget paths that depend on a list of actions
export const getEntityDependantPaths = (
fullEntityPaths: string[],
allEntitiesDependantsmap: GroupedDependencyMap,
visitedPaths: Set<string>,
): { names: Set<string>; fullPaths: Set<string> } => {
const dependantEntityNames = new Set<string>();
): Set<string> => {
const dependantEntityFullPaths = new Set<string>();
fullEntityPaths.forEach((fullEntityPath) => {
@ -87,25 +87,20 @@ export const getEntityDependants = (
// goes through dependants of a property
dependants.forEach((dependantPath) => {
const dependantEntityName = dependantPath.split(".")[0];
// Marking visited paths to avoid infinite recursion.
if (visitedPaths.has(dependantPath)) {
return;
}
visitedPaths.add(dependantPath);
dependantEntityNames.add(dependantEntityName);
dependantEntityFullPaths.add(dependantPath);
const childDependants = getEntityDependants(
const childDependants = getEntityDependantPaths(
[dependantPath],
allEntitiesDependantsmap,
visitedPaths,
);
childDependants.names.forEach((childDependantName) => {
dependantEntityNames.add(childDependantName);
});
childDependants.fullPaths.forEach((childDependantPath) => {
childDependants.forEach((childDependantPath) => {
dependantEntityFullPaths.add(childDependantPath);
});
});
@ -113,7 +108,7 @@ export const getEntityDependants = (
);
});
return { names: dependantEntityNames, fullPaths: dependantEntityFullPaths };
return dependantEntityFullPaths;
};
export const findLoadingEntities = (
@ -125,15 +120,29 @@ export const findLoadingEntities = (
inverseMap,
dataTree,
);
const loadingEntitiesDetails = getEntityDependants(
const loadingEntityPaths = getEntityDependantPaths(
isLoadingActions,
entitiesDependantsMap,
new Set<string>(),
);
// check animateLoading is active on current widgets and set
const filteredLoadingEntityNames = new Set<string>();
loadingEntitiesDetails.names.forEach((entityName) => {
loadingEntityPaths.forEach((entityPath) => {
const entityPathArray = entityPath.split(".");
const entityName = entityPathArray[0];
const widgetType = get(dataTree, [entityName, "type"]);
const loadingProperties = WidgetFactory.getLoadingProperties(widgetType);
// check if propertyPath is listed in widgetConfig
if (
entityPathArray.length > 1 &&
loadingProperties &&
!loadingProperties.find((propRegExp) => propRegExp.test(entityPath))
) {
return;
}
// check animateLoading is active on current widgets and set
get(dataTree, [entityName, "animateLoading"]) === true &&
filteredLoadingEntityNames.add(entityName);
});

View File

@ -26,6 +26,7 @@ export interface WidgetConfiguration {
default: Record<string, string>;
meta: Record<string, any>;
derived: DerivedPropertiesMap;
loadingProperties?: Array<RegExp>;
};
}
@ -55,6 +56,7 @@ export const registerWidget = (Widget: any, config: WidgetConfiguration) => {
config.properties.default,
config.properties.meta,
config.properties.config,
config.properties.loadingProperties,
);
configureWidget(config);
};

View File

@ -75,6 +75,20 @@ abstract class BaseWidget<
return {};
}
/**
* getLoadingProperties returns a list of regexp's used to specify bindingPaths,
* which can set the isLoading prop of the widget.
* When:
* 1. the path is bound to an action (API/Query)
* 2. the action is currently in-progress
*
* if undefined, all paths can set the isLoading state
* if empty array, no paths can set the isLoading state
*/
static getLoadingProperties(): Array<RegExp> | undefined {
return;
}
/**
* Widget abstraction to register the widget type
* ```javascript

View File

@ -180,6 +180,7 @@ export const CONFIG = {
default: Widget.getDefaultPropertiesMap(),
meta: Widget.getMetaPropertiesMap(),
config: Widget.getPropertyPaneConfig(),
loadingProperties: Widget.getLoadingProperties(),
},
};

View File

@ -109,6 +109,10 @@ class TableWidget extends BaseWidget<TableWidgetProps, WidgetState> {
};
}
static getLoadingProperties(): Array<RegExp> | undefined {
return [/.tableData$/];
}
getTableColumns = () => {
let columns: ReactTableColumnProps[] = [];
const hiddenColumns: ReactTableColumnProps[] = [];

View File

@ -519,12 +519,22 @@ export default class DataTreeEvaluator {
if (isWidget(entity)) {
// Adding the dynamic triggers in the dependency list as they need linting whenever updated
// we don't make it dependent on anything else
if (entity.dynamicTriggerPathList) {
Object.values(entity.dynamicTriggerPathList).forEach(({ key }) => {
dependencies[`${entityName}.${key}`] = [];
// To keep linting in trigger fields in sync, nodes they depend on need to be added to their dependencies
const dynamicTriggerPathlist = entity.dynamicTriggerPathList;
if (dynamicTriggerPathlist && dynamicTriggerPathlist.length) {
dynamicTriggerPathlist.forEach((dynamicPath) => {
const propertyPath = dynamicPath.key;
const unevalPropValue = _.get(entity, propertyPath);
const { jsSnippets } = getDynamicBindings(unevalPropValue);
const existingDeps =
dependencies[`${entityName}.${propertyPath}`] || [];
dependencies[`${entityName}.${propertyPath}`] = existingDeps.concat(
jsSnippets.filter((jsSnippet) => !!jsSnippet),
);
});
}
const widgetDependencies = addWidgetPropertyDependencies({
entity,
entityName,
@ -1333,7 +1343,10 @@ export default class DataTreeEvaluator {
entity,
entityPropertyPath,
);
if (isABindingPath) {
const isATriggerPath =
isWidget(entity) &&
isPathADynamicTrigger(entity, entityPropertyPath);
if (isABindingPath || isATriggerPath) {
didUpdateDependencyMap = true;
const { jsSnippets } = getDynamicBindings(