fix: Updating logic for reactive actions to fix cyclic dependency issue with App templates and Generate page flow of a DB (#41144)
## Description Updating logic for reactive actions to fix cyclic dependency issue with App templates and Generate page flow of a DB. Currently, both flows were leading to cyclic dependency errors which shouldn't show up. App template used - Order Fulfilment Tracker DB used - Mock DB Movies Fixes [#41125](https://github.com/appsmithorg/appsmith/issues/41125) [41113](https://github.com/appsmithorg/appsmith/issues/41113) EE PR for tests: https://github.com/appsmithorg/appsmith-ee/pull/8029 ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/16590882275> > Commit: 907935727e70fac4dc1a8efe70cd8a3cd5da262b > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=16590882275&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Tue, 29 Jul 2025 09:46:35 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit * **Refactor** * Improved dependency detection logic for actions and JS actions, refining how data paths are identified and handled. * Unified data path detection by using a shared utility function across the application. * Enhanced filtering of entities during dependency calculations for greater accuracy. * **Bug Fixes** * Corrected detection of reactive dependency misuse to reduce false positives for certain entity types. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
9930224212
commit
38ae03bf17
|
|
@ -1197,7 +1197,7 @@ export function getExternalChangedDependencies(
|
|||
}
|
||||
|
||||
export const isDataPath = (
|
||||
entity: DataTreeEntity,
|
||||
entity: DataTreeEntity | Partial<DataTreeEntityConfig>,
|
||||
fullPropertyPath: string,
|
||||
) => {
|
||||
if (isWidget(entity)) {
|
||||
|
|
|
|||
|
|
@ -132,16 +132,6 @@ export function getJSDependencies(
|
|||
dependencies = { ...dependencies, [fullPropertyPath]: newDeps };
|
||||
}
|
||||
|
||||
for (const funcName of jsActionConfig.actionNames) {
|
||||
const func = jsEntity[funcName];
|
||||
|
||||
if (func) {
|
||||
dependencies[`${jsObjectName}.${funcName}.data`] = [
|
||||
`${jsObjectName}.${funcName}`,
|
||||
];
|
||||
}
|
||||
}
|
||||
|
||||
return dependencies;
|
||||
}
|
||||
export function getActionDependencies(
|
||||
|
|
|
|||
|
|
@ -5,7 +5,9 @@ import {
|
|||
getEntityNameAndPropertyPath,
|
||||
IMMEDIATE_PARENT_REGEX,
|
||||
isActionConfig,
|
||||
isDataPath,
|
||||
isJSActionConfig,
|
||||
isWidget,
|
||||
} from "ee/workers/Evaluation/evaluationUtils";
|
||||
import type { ConfigTree } from "entities/DataTree/dataTreeTypes";
|
||||
import { isPathDynamicTrigger } from "utils/DynamicBindingUtils";
|
||||
|
|
@ -167,10 +169,6 @@ export class DependencyMapUtils {
|
|||
return false;
|
||||
}
|
||||
|
||||
static isDataPath(path: string) {
|
||||
return path.endsWith(".data");
|
||||
}
|
||||
|
||||
static detectReactiveDependencyMisuse(
|
||||
dependencyMap: DependencyMap,
|
||||
configTree: ConfigTree,
|
||||
|
|
@ -213,27 +211,34 @@ export class DependencyMapUtils {
|
|||
);
|
||||
|
||||
for (const dep of transitiveDeps) {
|
||||
if (this.isTriggerPath(dep, configTree)) {
|
||||
hasRun = true;
|
||||
runPath = dep;
|
||||
}
|
||||
const { entityName: depName } = getEntityNameAndPropertyPath(dep);
|
||||
const entity = configTree[depName];
|
||||
|
||||
if (this.isDataPath(dep)) {
|
||||
hasData = true;
|
||||
dataPath = dep;
|
||||
}
|
||||
// to show cyclic dependency errors only for Action calls and not JSObject.body or JSObject
|
||||
if (entity && entity.ENTITY_TYPE) {
|
||||
if (this.isTriggerPath(dep, configTree)) {
|
||||
hasRun = true;
|
||||
runPath = dep;
|
||||
}
|
||||
|
||||
if (
|
||||
hasRun &&
|
||||
hasData &&
|
||||
runPath.split(".")[0] === dataPath.split(".")[0]
|
||||
) {
|
||||
throw Object.assign(
|
||||
new Error(
|
||||
`Reactive dependency misuse: '${node}' depends on both trigger path '${runPath}' and data path '${dataPath}' from the same entity. This can cause unexpected reactivity.`,
|
||||
),
|
||||
{ node, triggerPath: runPath, dataPath },
|
||||
);
|
||||
// using the isDataPath function from evalUtils to calculate data paths based on entity type
|
||||
if (isDataPath(entity, dep)) {
|
||||
hasData = true;
|
||||
dataPath = dep;
|
||||
}
|
||||
|
||||
if (
|
||||
hasRun &&
|
||||
hasData &&
|
||||
runPath.split(".")[0] === dataPath.split(".")[0]
|
||||
) {
|
||||
throw Object.assign(
|
||||
new Error(
|
||||
`Reactive dependency misuse: '${node}' depends on both trigger path '${runPath}' and data path '${dataPath}' from the same entity. This can cause unexpected reactivity.`,
|
||||
),
|
||||
{ node, triggerPath: runPath, dataPath },
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -256,7 +261,15 @@ export class DependencyMapUtils {
|
|||
|
||||
if (!entityConfig) return;
|
||||
|
||||
if (!isActionConfig(entityConfig) && !isJSActionConfig(entityConfig)) {
|
||||
if (isWidget(entityConfig)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// to not calculate transitive dependencies for JSObject.body and JSObject
|
||||
if (
|
||||
isJSActionConfig(entityConfig) &&
|
||||
(current.includes(".body") || !current.includes("."))
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user