Merge pull request #41060 from appsmithorg/release

30/06 Daily Promotion

Author: btsgh

app/client/src/ce/workers/Evaluation/evaluationUtils.ts

@@ -29,6 +29,7 @@ import type {
   WidgetEntity,
   DataTreeEntityConfig,
   WidgetEntityConfig,
+  ActionEntityConfig,
 } from "ee/entities/DataTree/types";
 import type { EvalProps } from "workers/common/DataTreeEvaluator";
 import { validateWidgetProperty } from "workers/common/DataTreeEvaluator/validationUtils";
@@ -403,6 +404,16 @@ export function isAction(
   );
 }
 
+export function isActionConfig(
+  entity: DataTreeEntityConfig,
+): entity is ActionEntityConfig {
+  return (
+    typeof entity === "object" &&
+    "ENTITY_TYPE" in entity &&
+    entity.ENTITY_TYPE === ENTITY_TYPE.ACTION
+  );
+}
+
 export function isAppsmithEntity(
   entity: DataTreeEntity,
 ): entity is AppsmithEntity {
@@ -413,7 +424,9 @@ export function isAppsmithEntity(
   );
 }
 
-export function isJSAction(entity: DataTreeEntity): entity is JSActionEntity {
+export function isJSAction(
+  entity: Partial<DataTreeEntity>,
+): entity is JSActionEntity {
   return (
     typeof entity === "object" &&
     "ENTITY_TYPE" in entity &&

app/client/src/entities/DependencyMap/DependencyMapUtils.ts

@@ -4,9 +4,13 @@ import {
   entityTypeCheckForPathDynamicTrigger,
   getEntityNameAndPropertyPath,
   IMMEDIATE_PARENT_REGEX,
+  isActionConfig,
+  isJSActionConfig,
 } from "ee/workers/Evaluation/evaluationUtils";
 import type { ConfigTree } from "entities/DataTree/dataTreeTypes";
 import { isPathDynamicTrigger } from "utils/DynamicBindingUtils";
+import { WorkerEnv } from "workers/Evaluation/handlers/workerEnv";
+import { ActionRunBehaviour } from "PluginActionEditor/types/PluginActionTypes";
 
 type SortDependencies =
   | {
@@ -23,6 +27,9 @@ export class DependencyMapUtils {
   ): SortDependencies {
     const dependencyTree: Array<[string, string | undefined]> = [];
     const dependencies = dependencyMap.rawDependencies;
+    const featureFlags = WorkerEnv.getFeatureFlags();
+    const isReactiveActionsEnabled =
+      featureFlags.release_reactive_actions_enabled;
 
     for (const [node, deps] of dependencies.entries()) {
       if (deps.size) {
@@ -38,7 +45,7 @@ export class DependencyMapUtils {
         .reverse()
         .filter((edge) => !!edge);
 
-      if (configTree) {
+      if (configTree && isReactiveActionsEnabled) {
         this.detectReactiveDependencyMisuse(dependencyMap, configTree);
       }
 
@@ -163,81 +170,86 @@ export class DependencyMapUtils {
   ) {
     const dependencies = dependencyMap.rawDependencies;
 
-    // Helper function to get all transitive dependencies
-    const getAllTransitiveDependencies = (node: string): Set<string> => {
-      const allDeps = new Set<string>();
-      const queue = [node];
+    for (const node of dependencies.keys()) {
+      const { entityName: nodeName } = getEntityNameAndPropertyPath(node);
+      const nodeConfig = configTree[nodeName];
 
-      while (queue.length > 0) {
-        const current = queue.shift()!;
-        const deps = dependencyMap.getDirectDependencies(current) || [];
+      const isJSActionEntity = isJSActionConfig(nodeConfig);
+      const isActionEntity = isActionConfig(nodeConfig);
 
-        for (const dep of deps) {
-          if (!allDeps.has(dep)) {
-            allDeps.add(dep);
-            queue.push(dep);
-          }
-        }
+      if (isJSActionEntity) {
+        // Only continue if at least one function is automatic
+        const hasAutomaticFunc = Object.values(nodeConfig.meta).some(
+          (jsFunction) =>
+            jsFunction.runBehaviour === ActionRunBehaviour.AUTOMATIC,
+        );
+
+        if (!hasAutomaticFunc) continue;
+      } else if (isActionEntity) {
+        // Only continue if runBehaviour is AUTOMATIC
+        if (nodeConfig.runBehaviour !== ActionRunBehaviour.AUTOMATIC) continue;
+      } else {
+        // If not a JSAction, or Action, skip
+        continue;
       }
 
-      return allDeps;
-    };
+      // For each entity, check if both .run and a .data path are present
+      let hasRun = false;
+      let hasData = false;
+      let dataPath = "";
+      let runPath = "";
 
-    for (const [node, deps] of dependencies.entries()) {
-      // Get all dependencies including transitive ones
-      const allDeps = new Set<string>();
-      const queue = Array.from(deps);
+      const transitiveDeps = this.getAllTransitiveDependencies(
+        dependencyMap,
+        node,
+      );
 
-      while (queue.length > 0) {
-        const dep = queue.shift()!;
+      for (const dep of transitiveDeps) {
+        if (this.isTriggerPath(dep, configTree)) {
+          hasRun = true;
+          runPath = dep;
+        }
 
-        if (!allDeps.has(dep)) {
-          allDeps.add(dep);
-          const depDeps = dependencyMap.getDirectDependencies(dep) || [];
+        if (this.isDataPath(dep)) {
+          hasData = true;
+          dataPath = dep;
+        }
 
-          queue.push(...depDeps);
+        if (hasRun && hasData) {
+          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 },
+          );
         }
       }
+    }
+  }
 
-      // Separate dependencies into trigger paths and data paths
-      const triggerPaths = Array.from(deps).filter((dep) =>
-        this.isTriggerPath(dep, configTree),
-      );
-      const dataPaths = Array.from(deps).filter((dep) => this.isDataPath(dep));
-
-      // For each trigger path, check if there's a data path from the same entity
-      for (const triggerPath of triggerPaths) {
-        const triggerEntity = triggerPath.split(".")[0];
-
-        // Find data paths from the same entity
-        const sameEntityDataPaths = dataPaths.filter((dataPath) => {
-          const dataEntity = dataPath.split(".")[0];
-
-          return dataEntity === triggerEntity;
-        });
-
-        if (sameEntityDataPaths.length > 0) {
-          // Check if any of these data paths depend on the trigger path (directly or indirectly)
-          for (const dataPath of sameEntityDataPaths) {
-            const dataPathTransitiveDeps =
-              getAllTransitiveDependencies(dataPath);
-
-            if (dataPathTransitiveDeps.has(triggerPath)) {
-              const error = new Error(
-                `Reactive dependency misuse: '${node}' depends on both trigger path '${triggerPath}' and data path '${dataPath}' from the same entity, and '${dataPath}' depends on '${triggerPath}' (directly or indirectly). This can cause unexpected reactivity.`,
-              );
-
-              // Add custom properties
-              // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-              // @ts-ignore
-              // eslint-disable-next-line @typescript-eslint/no-explicit-any
-              (error as any).node = node;
-
-              throw error;
-            }
-          }
+  /**
+   * Returns all transitive dependencies (direct and indirect, no duplicates) for a given node.
+   */
+  static getAllTransitiveDependencies(
+    dependencyMap: DependencyMap,
+    node: string,
+  ): string[] {
+    const dependencies = dependencyMap.rawDependencies;
+    const visited = new Set<string>();
+
+    function traverse(current: string) {
+      const directDeps = dependencies.get(current) || new Set<string>();
+
+      for (const dep of directDeps) {
+        if (!visited.has(dep)) {
+          visited.add(dep);
+          traverse(dep);
         }
       }
     }
+
+    traverse(node);
+
+    return Array.from(visited);
   }
 }