Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/client/src/ce/workers/Evaluation/evaluationUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,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 &&
Expand Down
143 changes: 80 additions & 63 deletions app/client/src/entities/DependencyMap/DependencyMapUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@ import {
entityTypeCheckForPathDynamicTrigger,
getEntityNameAndPropertyPath,
IMMEDIATE_PARENT_REGEX,
isAction,
isJSAction,
} 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 =
| {
Expand All @@ -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) {
Expand All @@ -38,7 +45,7 @@ export class DependencyMapUtils {
.reverse()
.filter((edge) => !!edge);

if (configTree) {
if (configTree && isReactiveActionsEnabled) {
this.detectReactiveDependencyMisuse(dependencyMap, configTree);
}

Expand Down Expand Up @@ -163,81 +170,91 @@ 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 = isJSAction(nodeConfig);
const isActionEntity = isAction(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) {
const { entityName } = getEntityNameAndPropertyPath(dep);
const entityConfig = configTree[entityName];

if (!allDeps.has(dep)) {
allDeps.add(dep);
const depDeps = dependencyMap.getDirectDependencies(dep) || [];
if (entityConfig && entityConfig.ENTITY_TYPE === "ACTION") {
if (this.isTriggerPath(dep, configTree)) {
hasRun = true;
runPath = dep;
}

queue.push(...depDeps);
if (DependencyMapUtils.isDataPath(dep)) {
hasData = true;
dataPath = dep;
}

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);
}
}
Loading
Loading