Skip to content

Commit

Permalink
feat: Linting in trigger fields (#7638)
Browse files Browse the repository at this point in the history
  • Loading branch information
@hetunandu
hetunandu authored Oct 5, 2021
1 parent 08e3775 commit 94e3ffe
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 123 deletions.
7 changes: 3 additions & 4 deletions app/client/src/components/editorComponents/CodeEditor/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,9 +480,7 @@ class CodeEditor extends Component<Props, State> {
[],
) as EvaluationError[];

let annotations: Annotation[] = [];

annotations = getLintAnnotations(editor.getValue(), errors);
const annotations = getLintAnnotations(editor.getValue(), errors);

this.updateLintingCallback(editor, annotations);
}
Expand Down Expand Up @@ -593,7 +591,8 @@ class CodeEditor extends Component<Props, State> {
this.state.isFocused &&
!hideEvaluatedValue &&
("evaluatedValue" in this.props ||
("dataTreePath" in this.props && !!this.props.dataTreePath));
("dataTreePath" in this.props && !!dataTreePath));

return (
<DynamicAutocompleteInputWrapper
className="t--code-editor-wrapper"
Expand Down
1 change: 0 additions & 1 deletion app/client/src/pages/Editor/PropertyPane/PropertyControl.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,6 @@ const PropertyControl = memo((props: Props) => {
type: "Function",
autocompleteDataType: AutocompleteDataType.FUNCTION,
};
delete config.dataTreePath;
delete config.evaluatedValue;
}

Expand Down
50 changes: 45 additions & 5 deletions app/client/src/workers/DataTreeEvaluator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,15 @@ import {
EXECUTION_PARAM_REFERENCE_REGEX,
} from "constants/AppsmithActionConstants/ActionConstants";
import { DATA_BIND_REGEX } from "constants/BindingsConstants";
import evaluate, { EvalResult } from "workers/evaluate";
import evaluate, {
createGlobalData,
EvalResult,
EvaluationScriptType,
getScriptToEval,
} from "workers/evaluate";
import { substituteDynamicBindingWithValues } from "workers/evaluationSubstitution";
import { Severity } from "entities/AppsmithConsole";
import { getLintingErrors } from "workers/lint";

export default class DataTreeEvaluator {
dependencyMap: DependencyMap = {};
Expand Down Expand Up @@ -373,7 +379,8 @@ export default class DataTreeEvaluator {
}
}
if (isWidget(entity)) {
// Set default property dependency
// Make property dependant on the default property as any time the default changes
// the property needs to change
const defaultProperties = this.widgetConfigMap[entity.type]
.defaultProperties;
Object.entries(defaultProperties).forEach(
Expand All @@ -383,6 +390,13 @@ export default class DataTreeEvaluator {
];
},
);
// Adding the dynamic triggers in the dependency list as they need linting whenever updated
// we dont make it dependant on anything else
if (entity.dynamicTriggerPathList) {
Object.values(entity.dynamicTriggerPathList).forEach(({ key }) => {
dependencies[`${entityName}.${key}`] = [];
});
}
}
if (isAction(entity)) {
Object.entries(entity.dependencyMap).forEach(
Expand Down Expand Up @@ -442,9 +456,13 @@ export default class DataTreeEvaluator {
const isABindingPath =
(isAction(entity) || isWidget(entity)) &&
isPathADynamicBinding(entity, propertyPath);
const isATriggerPath =
isWidget(entity) && isPathADynamicTrigger(entity, propertyPath);
let evalPropertyValue;
const requiresEval =
isABindingPath && isDynamicValue(unEvalPropertyValue);
isABindingPath &&
!isATriggerPath &&
isDynamicValue(unEvalPropertyValue);
if (propertyPath) {
_.set(currentTree, getEvalErrorPath(fullPropertyPath), []);
}
Expand Down Expand Up @@ -475,8 +493,7 @@ export default class DataTreeEvaluator {
} else {
evalPropertyValue = unEvalPropertyValue;
}

if (isWidget(entity)) {
if (isWidget(entity) && !isATriggerPath) {
const widgetEntity = entity;
const defaultPropertyMap = this.widgetConfigMap[widgetEntity.type]
.defaultProperties;
Expand Down Expand Up @@ -508,6 +525,10 @@ export default class DataTreeEvaluator {
return _.set(currentTree, fullPropertyPath, parsedValue);
}
return _.set(currentTree, fullPropertyPath, evalPropertyValue);
} else if (isATriggerPath) {
const errors = this.lintTriggerPath(evalPropertyValue, entity);
addErrorToEntityProperty(errors, currentTree, fullPropertyPath);
return currentTree;
} else if (isAction(entity)) {
const safeEvaluatedValue = removeFunctions(evalPropertyValue);
_.set(
Expand Down Expand Up @@ -1319,6 +1340,25 @@ export default class DataTreeEvaluator {
clearLogs() {
this.logs = [];
}

private lintTriggerPath(userScript: string, entity: DataTreeEntity) {
const { jsSnippets } = getDynamicBindings(userScript, entity);
const script = getScriptToEval(
jsSnippets[0],
EvaluationScriptType.TRIGGERS,
);
const GLOBAL_DATA = createGlobalData(
this.evalTree,
this.resolvedFunctions,
true,
);
return getLintingErrors(
script,
GLOBAL_DATA,
jsSnippets[0],
EvaluationScriptType.TRIGGERS,
);
}
}

const extractReferencesFromBinding = (
Expand Down
161 changes: 49 additions & 112 deletions app/client/src/workers/evaluate.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@ import {
unsafeFunctionForEval,
} from "utils/DynamicBindingUtils";
import unescapeJS from "unescape-js";
import { JSHINT as jshint } from "jshint";
import { Severity } from "entities/AppsmithConsole";
import { Position } from "codemirror";
import { AppsmithPromise, enhanceDataTreeWithFunctions } from "./Actions";
import { ActionDescription } from "entities/DataTree/actionTriggers";
import { isEmpty, last } from "lodash";
import { isEmpty } from "lodash";
import { getLintingErrors } from "workers/lint";

export type EvalResult = {
result: any;
Expand All @@ -25,7 +24,7 @@ export enum EvaluationScriptType {
TRIGGERS = "TRIGGERS",
}

const evaluationScriptsPos: Record<EvaluationScriptType, string> = {
export const EvaluationScripts: Record<EvaluationScriptType, string> = {
[EvaluationScriptType.EXPRESSION]: `
function closedFunction () {
const result = <<script>>
Expand All @@ -50,19 +49,6 @@ const evaluationScriptsPos: Record<EvaluationScriptType, string> = {
`,
};

const getPositionInEvaluationScript = (
type: EvaluationScriptType,
): Position => {
const script = evaluationScriptsPos[type];

const index = script.indexOf("<<script>>");
const substr = script.substr(0, index);
const lines = substr.split("\n");
const lastLine = last(lines) || "";

return { line: lines.length, ch: lastLine.length };
};

const getScriptType = (
evalArguments?: Array<any>,
isTriggerBased = false,
Expand All @@ -76,73 +62,49 @@ const getScriptType = (
return scriptType;
};

const getScriptToEval = (
export const getScriptToEval = (
userScript: string,
type: EvaluationScriptType,
): string => {
return evaluationScriptsPos[type].replace("<<script>>", userScript);
return EvaluationScripts[type].replace("<<script>>", userScript);
};

const getLintingErrors = (
script: string,
data: Record<string, unknown>,
originalBinding: string,
scriptPos: Position,
): EvaluationError[] => {
const globalData: Record<string, boolean> = {};
Object.keys(data).forEach((datum) => (globalData[datum] = true));
// Jshint shouldn't throw errors for additional libraries
extraLibraries.forEach((lib) => (globalData[lib.accessor] = true));

globalData.console = true;

const options = {
indent: 2,
esversion: 8, // For async/await support
eqeqeq: false, // Not necessary to use ===
curly: false, // Blocks can be added without {}, eg if (x) return true
freeze: true, // Overriding inbuilt classes like Array is not allowed
undef: true, // Undefined variables should be reported as error
forin: false, // Doesn't require filtering for..in loops with obj.hasOwnProperty()
noempty: false, // Empty blocks are allowed
strict: false, // We won't force strict mode
unused: false, // Unused variables are allowed
asi: true, // Tolerate Automatic Semicolon Insertion (no semicolons)
boss: true, // Tolerate assignments where comparisons would be expected
evil: false, // Use of eval not allowed
sub: true, // Don't force dot notation
funcscope: true, // Tolerate variable definition inside control statements
// environments
browser: true,
worker: true,
mocha: false,
// global values
globals: globalData,
};

jshint(script, options);
const beginsWithLineBreakRegex = /^\s+|\s+$/;

return jshint.errors.map((lintError) => {
const ch = lintError.character;
return {
errorType: PropertyEvaluationErrorType.LINT,
raw: script,
// We are forcing warnings to errors and removing unwanted JSHint checks
severity: Severity.ERROR,
errorMessage: lintError.reason,
errorSegment: lintError.evidence,
originalBinding,
// By keeping track of these variables we can highlight the exact text that caused the error.
variables: [lintError.a, lintError.b, lintError.c, lintError.d],
code: lintError.code,
line: lintError.line - scriptPos.line,
ch: lintError.line === scriptPos.line ? ch - scriptPos.ch : ch,
};
});
export const createGlobalData = (
dataTree: DataTree,
resolvedFunctions: Record<string, any>,
isTriggerBased: boolean,
evalArguments?: Array<any>,
) => {
const GLOBAL_DATA: Record<string, any> = {};
///// Adding callback data
GLOBAL_DATA.ARGUMENTS = evalArguments;
///// Mocking Promise class
GLOBAL_DATA.Promise = AppsmithPromise;
if (isTriggerBased) {
//// Add internal functions to dataTree;
const dataTreeWithFunctions = enhanceDataTreeWithFunctions(dataTree);
///// Adding Data tree with functions
Object.keys(dataTreeWithFunctions).forEach((datum) => {
GLOBAL_DATA[datum] = dataTreeWithFunctions[datum];
});
} else {
Object.keys(dataTree).forEach((datum) => {
GLOBAL_DATA[datum] = dataTree[datum];
});
}
if (!isEmpty(resolvedFunctions)) {
Object.keys(resolvedFunctions).forEach((datum: any) => {
const resolvedObject = resolvedFunctions[datum];
Object.keys(resolvedObject).forEach((key: any) => {
GLOBAL_DATA[datum][key] = resolvedObject[key];
});
});
}
return GLOBAL_DATA;
};

const beginsWithLineBreakRegex = /^\s+|\s+$/;

export default function evaluate(
js: string,
data: DataTree,
Expand All @@ -157,55 +119,30 @@ export default function evaluate(
const scriptType = getScriptType(evalArguments, isTriggerBased);
const script = getScriptToEval(unescapedJS, scriptType);
// We are linting original js binding,
// This will make sure that the characted count is not messed up when we do unescapejs
// This will make sure that the character count is not messed up when we do unescapejs
const scriptToLint = getScriptToEval(js, scriptType);
return (function() {
let errors: EvaluationError[] = [];
let result;
let triggers: any[] = [];
/**** Setting the eval context ****/
const GLOBAL_DATA: Record<string, any> = {};
///// Adding callback data
GLOBAL_DATA.ARGUMENTS = evalArguments;
GLOBAL_DATA.Promise = AppsmithPromise;
if (isTriggerBased) {
//// Add internal functions to dataTree;
const dataTreeWithFunctions = enhanceDataTreeWithFunctions(data);
///// Adding Data tree with functions
Object.keys(dataTreeWithFunctions).forEach((datum) => {
GLOBAL_DATA[datum] = dataTreeWithFunctions[datum];
});
} else {
///// Adding Data tree
Object.keys(data).forEach((datum) => {
GLOBAL_DATA[datum] = data[datum];
});
}
const GLOBAL_DATA: Record<string, any> = createGlobalData(
data,
resolvedFunctions,
isTriggerBased,
evalArguments,
);

// Set it to self so that the eval function can have access to it
// as global data. This is what enables access all appsmith
// entity properties from the global context

Object.keys(GLOBAL_DATA).forEach((key) => {
for (const entity in GLOBAL_DATA) {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore: No types available
self[key] = GLOBAL_DATA[key];
});

if (!isEmpty(resolvedFunctions)) {
Object.keys(resolvedFunctions).forEach((datum: any) => {
const resolvedObject = resolvedFunctions[datum];
Object.keys(resolvedObject).forEach((key: any) => {
self[datum][key] = resolvedObject[key];
});
});
self[entity] = GLOBAL_DATA[entity];
}
errors = getLintingErrors(
scriptToLint,
GLOBAL_DATA,
js,
getPositionInEvaluationScript(scriptType),
);

errors = getLintingErrors(scriptToLint, GLOBAL_DATA, js, scriptType);

///// Adding extra libraries separately
extraLibraries.forEach((library) => {
Expand Down
5 changes: 4 additions & 1 deletion app/client/src/workers/evaluationUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,10 @@ export const isDynamicLeaf = (unEvalTree: DataTree, propertyPath: string) => {
if (!isAction(entity) && !isWidget(entity) && !isJSAction(entity))
return false;
const relativePropertyPath = convertPathToString(propPathEls);
return relativePropertyPath in entity.bindingPaths;
return (
relativePropertyPath in entity.bindingPaths ||
(isWidget(entity) && relativePropertyPath in entity.triggerPaths)
);
};

/*
Expand Down
Loading

0 comments on commit 94e3ffe

Please sign in to comment.