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

[Bug Fix] : Removal of cyclical dependencies due to action relationships being added to dependency map (#4342)

Browse Source
This commit is contained in:
Trisha Anand 2021-05-06 18:28:06 +05:30 committed by GitHub
parent f7d3beef50
commit ee14571ffc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 56 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/DisplayWidgets/Text_spec.js
View File

@ -61,33 +61,33 @@ describe("Text Widget Functionality", function() {
.should("have.css", "font-size", "18px"); .should("have.css", "font-size", "18px");
}); });
// it("Text widget depends on itself", function() { it("Text widget depends on itself", function() {
// cy.getCodeMirror().then(($cm) => { cy.getCodeMirror().then(($cm) => {
// if ($cm.val() !== "") { if ($cm.val() !== "") {
// cy.get(".CodeMirror textarea") cy.get(".CodeMirror textarea")
// .first() .first()
// .clear({ .clear({
// force: true, force: true,
// }); });
// } }
//
// cy.get(".CodeMirror textarea") cy.get(".CodeMirror textarea")
// .first() .first()
// .type(`{{${this.data.TextName}}}`, { .type(`{{${this.data.TextName}}}`, {
// force: true, force: true,
// parseSpecialCharSequences: false, parseSpecialCharSequences: false,
// }); });
// }); });
// cy.get(commonlocators.toastBody) cy.get(commonlocators.toastBody)
// .first() .first()
// .contains("Cyclic"); .contains("Cyclic");
//
// cy.PublishtheApp(); cy.PublishtheApp();
// cy.get(commonlocators.bodyTextStyle).should( cy.get(commonlocators.bodyTextStyle).should(
// "have.text", "have.text",
// `{{${this.data.TextName}}}`, `{{${this.data.TextName}}}`,
// ); );
// }); });
afterEach(() => { afterEach(() => {
cy.get(publishPage.backToEditor).click({ force: true }); cy.get(publishPage.backToEditor).click({ force: true });

54
app/client/src/workers/DataTreeEvaluator.ts
View File

@ -305,7 +305,7 @@ export default class DataTreeEvaluator {
Object.keys(dependencyMap).forEach((key) => { Object.keys(dependencyMap).forEach((key) => {
dependencyMap[key] = _.flatten( dependencyMap[key] = _.flatten(
dependencyMap[key].map((path) => dependencyMap[key].map((path) =>
extractReferencesFromBinding(key, path, this.allKeys), extractReferencesFromBinding(path, this.allKeys),
), ),
); );
}); });
@ -345,10 +345,21 @@ export default class DataTreeEvaluator {
} }
if (isAction(entity)) { if (isAction(entity)) {
Object.entries(entity.dependencyMap).forEach( Object.entries(entity.dependencyMap).forEach(
([dependent, entityDependencies]) => { ([path, entityDependencies]) => {
dependencies[`${entityName}.${dependent}`] = entityDependencies.map( const actionDependentPaths: Array<string> = [];
(propertyPath) => `${entityName}.${propertyPath}`, const mainPath = `${entityName}.${path}`;
); // Only add dependencies for paths which exist at the moment in appsmith world
if (this.allKeys.hasOwnProperty(mainPath)) {
// Only add dependent paths which exist in the data tree. Skip all the other paths to avoid creating
// a cyclical dependency.
entityDependencies.forEach((dependentPath) => {
const completePath = `${entityName}.${dependentPath}`;
if (this.allKeys.hasOwnProperty(completePath)) {
actionDependentPaths.push(completePath);
}
});
dependencies[mainPath] = actionDependentPaths;
}
}, },
); );
} }
@ -859,16 +870,23 @@ export default class DataTreeEvaluator {
const entityDependenciesName = entity.dependencyMap[ const entityDependenciesName = entity.dependencyMap[
entityPropertyPath entityPropertyPath
].map((dep) => `${entityName}.${dep}`); ].map((dep) => `${entityName}.${dep}`);
// Filter only the paths which exist in the appsmith world to avoid cyclical dependencies
const filteredEntityDependencies = entityDependenciesName.filter(
(path) => this.allKeys.hasOwnProperty(path),
);
// Now assign these existing dependent paths to the property path in dependencyMap
if (fullPropertyPath in this.dependencyMap) { if (fullPropertyPath in this.dependencyMap) {
this.dependencyMap[ this.dependencyMap[
fullPropertyPath fullPropertyPath
] = this.dependencyMap[fullPropertyPath].concat( ] = this.dependencyMap[fullPropertyPath].concat(
entityDependenciesName, filteredEntityDependencies,
); );
} else { } else {
this.dependencyMap[ this.dependencyMap[
fullPropertyPath fullPropertyPath
] = entityDependenciesName; ] = filteredEntityDependencies;
} }
} }
} }
@ -890,7 +908,7 @@ export default class DataTreeEvaluator {
this.dependencyMap[key] = _.uniq( this.dependencyMap[key] = _.uniq(
_.flatten( _.flatten(
this.dependencyMap[key].map((path) => this.dependencyMap[key].map((path) =>
extractReferencesFromBinding(key, path, this.allKeys), extractReferencesFromBinding(path, this.allKeys),
), ),
), ),
); );
@ -1027,7 +1045,7 @@ export default class DataTreeEvaluator {
const propertyBindings = entityPropertyBindings[path]; const propertyBindings = entityPropertyBindings[path];
const references = _.flatten( const references = _.flatten(
propertyBindings.map((binding) => propertyBindings.map((binding) =>
extractReferencesFromBinding(path, binding, this.allKeys), extractReferencesFromBinding(binding, this.allKeys),
), ),
); );
references.forEach((value) => { references.forEach((value) => {
@ -1089,7 +1107,6 @@ export default class DataTreeEvaluator {
} }
const extractReferencesFromBinding = ( const extractReferencesFromBinding = (
path: string,
dependentPath: string, dependentPath: string,
all: Record<string, true>, all: Record<string, true>,
): Array<string> => { ): Array<string> => {
@ -1100,7 +1117,7 @@ const extractReferencesFromBinding = (
identifiers.forEach((identifier: string) => { identifiers.forEach((identifier: string) => {
// If the identifier exists directly, add it and return // If the identifier exists directly, add it and return
if (all.hasOwnProperty(identifier)) { if (all.hasOwnProperty(identifier)) {
pushDependentsInSubDependencyArray(subDeps, path, identifier); subDeps.push(identifier);
return; return;
} }
const subpaths = _.toPath(identifier); const subpaths = _.toPath(identifier);
@ -1113,7 +1130,7 @@ const extractReferencesFromBinding = (
current = convertPathToString(subpaths); current = convertPathToString(subpaths);
// We've found the dep, add it and return // We've found the dep, add it and return
if (all.hasOwnProperty(current)) { if (all.hasOwnProperty(current)) {
pushDependentsInSubDependencyArray(subDeps, path, current); subDeps.push(current);
return; return;
} }
subpaths.pop(); subpaths.pop();
@ -1122,19 +1139,6 @@ const extractReferencesFromBinding = (
return _.uniq(subDeps); return _.uniq(subDeps);
}; };
const pushDependentsInSubDependencyArray = (
subDeps: string[],
path: string,
dependentPath: string,
): string[] => {
// Only add if path is not a child of dependentPath which ensures that cyclical dependency is not introduced
// when adding parent-child relationships later
if (!isChildPropertyPath(dependentPath, path)) {
subDeps.push(dependentPath);
}
return subDeps;
};
// TODO cryptic comment below. Dont know if we still need this. Duplicate function // TODO cryptic comment below. Dont know if we still need this. Duplicate function
// referencing DATA_BIND_REGEX fails for the value "{{Table1.tableData[Table1.selectedRowIndex]}}" if you run it multiple times and don't recreate // referencing DATA_BIND_REGEX fails for the value "{{Table1.tableData[Table1.selectedRowIndex]}}" if you run it multiple times and don't recreate
const isDynamicValue = (value: string): boolean => DATA_BIND_REGEX.test(value); const isDynamicValue = (value: string): boolean => DATA_BIND_REGEX.test(value);