fix: Updating updateDependencies map to execute newly binded queries in reactive flow#41014
Conversation
WalkthroughThis update introduces new Redux action types for on-load action execution and reactive queries, enhances dependency and trigger path tracking for actions and JS actions, and implements saga logic to execute reactive queries based on run behavior. Numerous files are refactored to standardize the Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant Redux
participant Saga
participant Evaluator
participant DataTree
UI->>Redux: Dispatch EXECUTE_REACTIVE_QUERIES
Redux->>Saga: Saga listens for EXECUTE_REACTIVE_QUERIES
Saga->>Evaluator: Get dataTree, configTree, executeReactiveActions
loop For each reactive action
Saga->>DataTree: Find entity and property path
alt JS Action
Saga->>Redux: Dispatch START_EXECUTE_JS_FUNCTION
Saga->>Saga: Wait for function execution completion
else Regular Action
Saga->>Redux: Dispatch RUN_ACTION
Saga->>Saga: Wait for action success/error
end
Saga->>Redux: Dispatch SET_ONLOAD_ACTION_EXECUTED
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
app/client/src/utils/DynamicBindingUtils.ts (1)
243-256: Excellent type safety improvementGood refactoring from
anytype to specific union types. However, address the static analysis hint about optional chaining.Apply this diff to use optional chaining as suggested by static analysis:
- if ( - entity && - entity.dynamicTriggerPathList && - Array.isArray(entity.dynamicTriggerPathList) - ) { + if (entity?.dynamicTriggerPathList && Array.isArray(entity.dynamicTriggerPathList)) {
🧹 Nitpick comments (13)
app/client/src/workers/common/DependencyMap/index.ts (1)
331-354: Code duplication detected - consider refactoring for maintainability.The newly added logic in the EDIT case duplicates the dependency processing logic from the NEW case (lines 239-261). While the functional improvement is correct and aligns with the reactive query execution objectives, this duplication creates a maintenance burden.
Consider extracting the common dependency processing logic into a helper function:
+const processEntityDependencies = ( + entity: ActionEntity | WidgetEntity | JSActionEntity, + entityName: string, + configTree: ConfigTree, + allKeys: Record<string, true>, + setDependenciesToDepedencyMapFn: (path: string, references: string[]) => void, + dataTreeEvalErrors: any[] +) => { + const entityDependencyMap = getEntityDependencies( + entity, + configTree[entityName], + allKeys, + ); + + if (!isEmpty(entityDependencyMap)) { + Object.entries(entityDependencyMap).forEach( + ([path, pathDependencies]) => { + const { errors: extractDependencyErrors, references } = + extractInfoFromBindings(pathDependencies, allKeys); + + setDependenciesToDepedencyMapFn(path, references); + dataTreeEvalErrors.push(...extractDependencyErrors); + }, + ); + return true; + } + return false; +};Then use this helper in both NEW and EDIT cases to reduce duplication.
app/client/src/sagas/EvaluationsSaga.ts (1)
238-251: Improve safety with optional chaining.The reactive query dispatch logic is well-structured, but consider using optional chaining for better safety.
Apply this diff to use optional chaining:
- if ( - executeReactiveActions && - executeReactiveActions.length && - isReactiveActionsEnabled - ) { + if ( + executeReactiveActions?.length && + isReactiveActionsEnabled + ) {app/client/src/ce/sagas/moduleInstanceSagaUtils.ts (1)
14-20: Add TODO comment for placeholder implementation.The empty generator function appears to be an intentional placeholder. Consider adding a TODO comment to clarify the intended implementation.
+// TODO: Implement module instance execution logic export function* executionForJSModuleInstance({ configTree, dataTree, entity, entityName, propertyPath, }: ParamsForJSModuleInstance) {}app/client/src/ce/entities/DataTree/dataTreeAction.ts (1)
59-75: Consider immutability for action configurationThe view mode logic correctly handles the scenario, but directly mutating
action.config.actionConfigurationcould cause side effects. Consider creating a new object instead.- action.config.actionConfiguration = { - ...action.config.actionConfiguration, - body: result, - }; + const updatedActionConfiguration = { + ...action.config.actionConfiguration, + body: result, + };app/client/src/ce/entities/DataTree/types/DataTreeSeed.ts (1)
14-34: LGTM: Comprehensive data tree seed interfaceWell-structured interface that centralizes the data tree evaluation context. The TODO comments appropriately acknowledge the technical debt around
anytypes.Consider prioritizing the replacement of
anytypes with proper interfaces in future iterations.app/client/src/ce/workers/Evaluation/evaluationUtils.ts (2)
1200-1205: Strengthen JSAction data path validation.The current check could match unintended paths. Consider validating that the path follows the expected pattern
<functionName>.data.if (isJSAction(entity)) { // Check if propertyPath matches <function>.data (not just 'data') - if (propertyPath.endsWith(".data") && propertyPath !== "data") { + const pathParts = propertyPath.split("."); + if (pathParts.length === 2 && pathParts[1] === "data") { return true; } }
1210-1213: Add TODO comment for stub implementation.This stub function should have a TODO comment explaining its purpose and when it will be implemented.
/* eslint-disable-next-line @typescript-eslint/no-unused-vars */ +// TODO: Implement JS module instance detection logic export function isJSModuleInstance(entity: DataTreeEntity) { return false; }app/client/src/workers/Evaluation/evalTreeWithChanges.ts (1)
77-77: Consider performance impact of deep cloning.Using
klonato deep clone the entire evaluation tree could be expensive for large applications. Consider if a shallow clone or selective cloning would suffice.#!/bin/bash # Check the typical size of eval trees by looking for tree size logging or metrics rg -A 5 "evalTree.*size|tree.*nodes|dataTree.*count" --type tsapp/client/src/sagas/PostEvaluationSagas.ts (1)
395-395: Fix static analysis issues.Address the redundant double-negation and missing optional chaining.
-if (!!entityConfig.moduleInstanceId) { +if (entityConfig.moduleInstanceId) { -const runBehaviour = - entityConfig?.meta && entityConfig.meta[action.name]?.runBehaviour; +const runBehaviour = entityConfig?.meta?.[action.name]?.runBehaviour;Also applies to: 408-408
app/client/src/entities/DependencyMap/DependencyMapUtils.ts (2)
61-61: Fix static context 'this' usage.Using
thisin static methods can be confusing. Use the class name instead.-this.detectReactiveDependencyMisuse(dependencyMap, configTree); +DependencyMapUtils.detectReactiveDependencyMisuse(dependencyMap, configTree); -this.isTriggerPath(dep, configTree), +DependencyMapUtils.isTriggerPath(dep, configTree), -const dataPaths = Array.from(deps).filter((dep) => this.isDataPath(dep)); +const dataPaths = Array.from(deps).filter((dep) => DependencyMapUtils.isDataPath(dep));Also applies to: 223-223, 225-225
250-252: Avoid using ts-ignore.Instead of using ts-ignore, properly type the error object.
-// 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; +interface ReactiveDependencyError extends Error { + node?: string; +} +const typedError = error as ReactiveDependencyError; +typedError.node = node;app/client/src/workers/common/DataTreeEvaluator/index.ts (2)
1153-1154: Performance concern with value change trackingThe value change tracking adds comparison overhead for every evaluated property. For large data trees, this could impact performance.
Consider implementing a more efficient change detection mechanism:
- Use a Set for O(1) lookups instead of an object
- Consider lazy evaluation of changes only for paths with reactive dependencies
- Add performance metrics to monitor the impact
Also applies to: 1192-1193, 1347-1349, 1406-1408, 1460-1464, 1496-1498
1612-1614: Simplify error message handlingThe ternary operator for error message handling can be simplified.
- message: (result.error as Error).message - ? (result.error as Error).message - : "Cyclic dependency found while evaluating.", + message: (result.error as Error).message || "Cyclic dependency found while evaluating.",
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (93)
app/client/src/ce/constants/ReduxActionConstants.tsx(1 hunks)app/client/src/ce/entities/DataTree/dataTreeAction.ts(3 hunks)app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts(4 hunks)app/client/src/ce/entities/DataTree/dataTreeJSAction.ts(6 hunks)app/client/src/ce/entities/DataTree/types.ts(3 hunks)app/client/src/ce/entities/DataTree/types/DataTreeSeed.ts(1 hunks)app/client/src/ce/reducers/uiReducers/editorReducer.tsx(8 hunks)app/client/src/ce/sagas/ActionExecution/ActionExecutionSagas.ts(1 hunks)app/client/src/ce/sagas/index.tsx(2 hunks)app/client/src/ce/sagas/moduleInstanceSagaUtils.ts(1 hunks)app/client/src/ce/selectors/moduleInstanceSelectors.ts(1 hunks)app/client/src/ce/utils/actionExecutionUtils.ts(1 hunks)app/client/src/ce/workers/Evaluation/evaluationUtils.test.ts(2 hunks)app/client/src/ce/workers/Evaluation/evaluationUtils.ts(1 hunks)app/client/src/ce/workers/common/DependencyMap/utils/getEntityDependenciesByType.ts(1 hunks)app/client/src/components/editorComponents/CodeEditor/EvaluatedValuePopup.tsx(1 hunks)app/client/src/components/editorComponents/CodeEditor/index.tsx(1 hunks)app/client/src/components/formControls/DynamicTextFieldControl.tsx(1 hunks)app/client/src/constants/EvaluationConstants.ts(1 hunks)app/client/src/constants/PropertyControlConstants.tsx(1 hunks)app/client/src/ee/entities/DataTree/types/DataTreeSeed.ts(1 hunks)app/client/src/ee/sagas/moduleInstanceSagaUtils.ts(1 hunks)app/client/src/entities/Action/actionProperties.test.ts(7 hunks)app/client/src/entities/Action/actionProperties.ts(2 hunks)app/client/src/entities/DataTree/dataTreeFactory.ts(1 hunks)app/client/src/entities/DataTree/dataTreeWidget.test.ts(1 hunks)app/client/src/entities/DependencyMap/DependencyMapUtils.ts(5 hunks)app/client/src/entities/DependencyMap/__tests__/index.test.ts(2 hunks)app/client/src/entities/Widget/utils.test.ts(1 hunks)app/client/src/entities/Widget/utils.ts(1 hunks)app/client/src/layoutSystems/common/DSLConversions/fixedToAutoLayout.ts(2 hunks)app/client/src/plugins/Linting/utils/entityParser.ts(1 hunks)app/client/src/sagas/ActionExecution/PluginActionSaga.ts(4 hunks)app/client/src/sagas/EvaluationsSaga.ts(3 hunks)app/client/src/sagas/JSPaneSagas.ts(1 hunks)app/client/src/sagas/PostEvaluationSagas.ts(4 hunks)app/client/src/selectors/editorSelectors.tsx(1 hunks)app/client/src/utils/DynamicBindingUtils.ts(1 hunks)app/client/src/utils/FilterInternalProperties/__tests__/index.test.ts(2 hunks)app/client/src/utils/autocomplete/__tests__/dataTreeTypeDefCreator.test.ts(2 hunks)app/client/src/utils/editor/EditorBindingPaths.ts(1 hunks)app/client/src/utils/helpers.test.ts(1 hunks)app/client/src/widgets/CategorySliderWidget/widget/propertyConfig/contentConfig.ts(1 hunks)app/client/src/widgets/ChartWidget/widget/propertyConfig.ts(1 hunks)app/client/src/widgets/CheckboxGroupWidget/widget/index.tsx(1 hunks)app/client/src/widgets/DropdownWidget/widget/index.tsx(1 hunks)app/client/src/widgets/FilePickerWidgetV2/widget/index.tsx(1 hunks)app/client/src/widgets/FilepickerWidget/widget/index.tsx(1 hunks)app/client/src/widgets/JSONFormWidget/widget/propertyConfig.ts(1 hunks)app/client/src/widgets/JSONFormWidget/widget/propertyConfig/properties/common.ts(1 hunks)app/client/src/widgets/JSONFormWidget/widget/propertyConfig/properties/multiSelect.ts(1 hunks)app/client/src/widgets/JSONFormWidget/widget/propertyConfig/properties/radioGroup.ts(1 hunks)app/client/src/widgets/JSONFormWidget/widget/propertyConfig/properties/select.ts(1 hunks)app/client/src/widgets/ListWidget/widget/propertyConfig.ts(1 hunks)app/client/src/widgets/ListWidgetV2/widget/propertyConfig.ts(1 hunks)app/client/src/widgets/MapChartWidget/widget/index.tsx(1 hunks)app/client/src/widgets/MapWidget/widget/index.tsx(1 hunks)app/client/src/widgets/MenuButtonWidget/widget/propertyConfig/contentConfig.ts(1 hunks)app/client/src/widgets/MetaHOC.tsx(1 hunks)app/client/src/widgets/MultiSelectTreeWidget/widget/index.tsx(1 hunks)app/client/src/widgets/MultiSelectWidget/widget/index.tsx(1 hunks)app/client/src/widgets/MultiSelectWidgetV2/widget/index.tsx(1 hunks)app/client/src/widgets/RadioGroupWidget/widget/index.tsx(1 hunks)app/client/src/widgets/SelectWidget/widget/index.tsx(1 hunks)app/client/src/widgets/SingleSelectTreeWidget/widget/index.tsx(1 hunks)app/client/src/widgets/SwitchGroupWidget/widget/index.tsx(1 hunks)app/client/src/widgets/TableWidget/widget/propertyConfig.ts(1 hunks)app/client/src/widgets/TableWidgetV2/widget/index.tsx(1 hunks)app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Basic.ts(1 hunks)app/client/src/widgets/TableWidgetV2/widget/propertyConfig/contentConfig.ts(1 hunks)app/client/src/widgets/wds/WDSCheckboxGroupWidget/config/propertyPaneConfig/contentConfig.ts(1 hunks)app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/contentConfig.ts(1 hunks)app/client/src/widgets/wds/WDSMultiSelectWidget/config/propertyPaneConfig/contentConfig.tsx(1 hunks)app/client/src/widgets/wds/WDSRadioGroupWidget/config/propertyPaneConfig/contentConfig.ts(1 hunks)app/client/src/widgets/wds/WDSSelectWidget/config/propertyPaneConfig/contentConfig.tsx(1 hunks)app/client/src/widgets/wds/WDSSwitchGroupWidget/config/propertyPaneConfig/contentConfig.ts(1 hunks)app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/contentConfig.ts(1 hunks)app/client/src/workers/Evaluation/JSObject/JSObject.test.ts(4 hunks)app/client/src/workers/Evaluation/JSObject/utils.ts(3 hunks)app/client/src/workers/Evaluation/__tests__/evaluation.test.ts(15 hunks)app/client/src/workers/Evaluation/__tests__/evaluationSubstitution.test.ts(1 hunks)app/client/src/workers/Evaluation/evalTreeWithChanges.test.ts(13 hunks)app/client/src/workers/Evaluation/evalTreeWithChanges.ts(8 hunks)app/client/src/workers/Evaluation/evaluationSubstitution.ts(1 hunks)app/client/src/workers/Evaluation/handlers/evalTree.ts(7 hunks)app/client/src/workers/Evaluation/handlers/evalTrigger.ts(3 hunks)app/client/src/workers/Evaluation/types.ts(1 hunks)app/client/src/workers/common/DataTreeEvaluator/dataTreeEvaluator.test.ts(15 hunks)app/client/src/workers/common/DataTreeEvaluator/index.ts(24 hunks)app/client/src/workers/common/DataTreeEvaluator/mockData/ArrayAccessorTree.ts(1 hunks)app/client/src/workers/common/DataTreeEvaluator/mockData/NestedArrayAccessorTree.ts(1 hunks)app/client/src/workers/common/DataTreeEvaluator/mockData/mockConfigTree.ts(1 hunks)app/client/src/workers/common/DependencyMap/index.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/src/utils/DynamicBindingUtils.ts
[error] 248-249: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/client/src/sagas/EvaluationsSaga.ts
[error] 239-240: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/client/src/sagas/PostEvaluationSagas.ts
[error] 395-395: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 408-408: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/client/src/entities/DependencyMap/DependencyMapUtils.ts
[error] 61-61: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 223-223: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 225-225: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
🔇 Additional comments (119)
app/client/src/constants/EvaluationConstants.ts (1)
1-5: Define centralized EvaluationSubstitutionType enum
Creates a single source of truth for substitution types, removing redundant definitions inee/entities.app/client/src/widgets/wds/WDSMultiSelectWidget/config/propertyPaneConfig/contentConfig.tsx (1)
18-18: Update import path for EvaluationSubstitutionType
Switching to the new centralized enum path ensures consistency across widget configs.app/client/src/widgets/MapChartWidget/widget/index.tsx (1)
5-5: Switch EvaluationSubstitutionType import to constants module
Aligns with the refactor to use a shared constants file for substitution types.app/client/src/widgets/wds/WDSRadioGroupWidget/config/propertyPaneConfig/contentConfig.ts (1)
8-8: Point import to new EvaluationConstants location
Ensures all widgets import the enum from the same source.app/client/src/utils/editor/EditorBindingPaths.ts (1)
2-2: Align import of EvaluationSubstitutionType with centralized definition
Replace legacy import to maintain consistency in editor utility paths.app/client/src/widgets/TableWidgetV2/widget/propertyConfig/PanelConfig/Basic.ts (1)
13-13: Align import to centralized constants
Switch to importingEvaluationSubstitutionTypefromconstants/EvaluationConstantsaligns with the project-wide consolidation.app/client/src/workers/common/DataTreeEvaluator/mockData/mockConfigTree.ts (1)
2-2: Use new constants module for substitution type
ImportingEvaluationSubstitutionTypefromconstants/EvaluationConstantskeeps mock data in sync with the refactor.app/client/src/widgets/ChartWidget/widget/propertyConfig.ts (1)
2-2: Standardize import for EvaluationSubstitutionType
This update centralizes the import, matching the refactored constant location.app/client/src/widgets/SwitchGroupWidget/widget/index.tsx (1)
5-5: Centralize EvaluationSubstitutionType import
Good catch—updating to the new constants path ensures consistency across widgets.app/client/src/widgets/CategorySliderWidget/widget/propertyConfig/contentConfig.ts (1)
4-4: Update substitution type import location
Switching toconstants/EvaluationConstantsaligns with the consolidated enum.app/client/src/widgets/JSONFormWidget/widget/propertyConfig/properties/common.ts (1)
3-3: Import path updated forEvaluationSubstitutionType
Consistent with the centralized enum location.app/client/src/widgets/MultiSelectWidgetV2/widget/index.tsx (1)
7-7: Import path updated forEvaluationSubstitutionType
Matches the new constants module and aligns with the broader refactor.app/client/src/components/editorComponents/CodeEditor/EvaluatedValuePopup.tsx (1)
11-11: Import path updated forEvaluationSubstitutionType
Aligns this component with the centralized constants.app/client/src/widgets/wds/WDSSwitchGroupWidget/config/propertyPaneConfig/contentConfig.ts (1)
2-2: Import path updated forEvaluationSubstitutionType
Ensures consistency with the new enum definition inEvaluationConstants.app/client/src/entities/DataTree/dataTreeFactory.ts (1)
22-22: Update import forDataTreeSeedtype
Switching to the dedicatedDataTreeSeedpath improves clarity and centralizes the type definition.app/client/src/utils/helpers.test.ts (1)
3-3: Correct import source for EvaluationSubstitutionType
Updated the import path to the centralizedconstants/EvaluationConstantsmodule to align with the global refactor.app/client/src/widgets/wds/WDSMenuButtonWidget/config/propertyPaneConfig/contentConfig.ts (1)
3-3: Updated import for EvaluationSubstitutionType
Switched the import toconstants/EvaluationConstantsto reflect the new central enum location.app/client/src/workers/Evaluation/evaluationSubstitution.ts (1)
3-3: Refactored import path for EvaluationSubstitutionType
Adjusted the import to the newconstants/EvaluationConstantsfile as part of the centralized substitution-type migration.app/client/src/widgets/wds/WDSSelectWidget/config/propertyPaneConfig/contentConfig.tsx (1)
3-3: Align import with centralized constants
Changed theEvaluationSubstitutionTypeimport toconstants/EvaluationConstantsto maintain consistency across widgets.app/client/src/widgets/JSONFormWidget/widget/propertyConfig/properties/multiSelect.ts (1)
1-1: Centralized EvaluationSubstitutionType import
Switched the import path to the sharedconstants/EvaluationConstantsmodule in line with the broader refactor.app/client/src/widgets/JSONFormWidget/widget/propertyConfig/properties/radioGroup.ts (1)
3-3: Consistent import path update. TheEvaluationSubstitutionTypeimport now correctly references the centralizedconstants/EvaluationConstantsmodule.app/client/src/widgets/MenuButtonWidget/widget/propertyConfig/contentConfig.ts (1)
3-3: Centralize import of EvaluationSubstitutionType. Path updated to useconstants/EvaluationConstants, aligning with the recent refactor.app/client/src/plugins/Linting/utils/entityParser.ts (1)
7-7: Update import source for EvaluationSubstitutionType. Aligned to the newconstants/EvaluationConstantslocation for consistency.app/client/src/widgets/wds/WDSCheckboxGroupWidget/config/propertyPaneConfig/contentConfig.ts (1)
2-2: Align EvaluationSubstitutionType import. Changed to the centralizedconstants/EvaluationConstantsmodule.app/client/src/widgets/TableWidgetV2/widget/propertyConfig/contentConfig.ts (1)
7-7: Consistent import refactor for EvaluationSubstitutionType. Now imports fromconstants/EvaluationConstantsas per standardized pattern.app/client/src/widgets/FilepickerWidget/widget/index.tsx (1)
14-14: Consistent import refactor
ImportingEvaluationSubstitutionTypefrom"constants/EvaluationConstants"aligns with the centralized enum location and maintains consistency across the codebase.app/client/src/widgets/ListWidgetV2/widget/propertyConfig.ts (1)
5-5: Consistent import refactor
Updating the source ofEvaluationSubstitutionTypeto"constants/EvaluationConstants"matches the standardized enum usage in other modules.app/client/src/widgets/CheckboxGroupWidget/widget/index.tsx (1)
11-11: Consistent import refactor
Switching theEvaluationSubstitutionTypeimport to"constants/EvaluationConstants"keeps the widget in sync with the refactored constant definitions.app/client/src/widgets/ListWidget/widget/propertyConfig.ts (1)
6-6: Consistent import refactor
Refactoring the import ofEvaluationSubstitutionTypeto the centralized constants file preserves uniformity in evaluation substitution types.app/client/src/widgets/MultiSelectTreeWidget/widget/index.tsx (1)
9-9: Consistent import refactor
Updating the import path forEvaluationSubstitutionTypeto"constants/EvaluationConstants"ensures alignment with the global enum source.app/client/src/entities/Widget/utils.ts (1)
5-5: Unified import path for EvaluationSubstitutionType is correctThe enum is properly centralized in
constants/EvaluationConstants, and no functional code changes are introduced.app/client/src/widgets/RadioGroupWidget/widget/index.tsx (1)
11-11: Centralized EvaluationSubstitutionType import appliedImporting from
constants/EvaluationConstantsaligns with the global refactor. Downstream usage ofEvaluationSubstitutionType.SMART_SUBSTITUTEremains unchanged.app/client/src/widgets/SelectWidget/widget/index.tsx (1)
6-6: Standardized EvaluationSubstitutionType importThe import update reflects the consolidated
EvaluationConstantsmodule, preserving existing evaluation substitution behavior.app/client/src/widgets/DropdownWidget/widget/index.tsx (1)
15-15: EvaluationSubstitutionType import path refactoredSwitching to the centralized
constants/EvaluationConstantsensures consistency across widgets. No logic or behavior alterations.app/client/src/widgets/FilePickerWidgetV2/widget/index.tsx (1)
8-8: Refactored EvaluationSubstitutionType importUpdating the import aligns with the new
constants/EvaluationConstantsenum. Functionality remains intact as this is purely an import change.app/client/src/widgets/MapWidget/widget/index.tsx (1)
10-10: Centralize EvaluationSubstitutionType import. Aligns this widget with the newconstants/EvaluationConstantsenum source.app/client/src/widgets/SingleSelectTreeWidget/widget/index.tsx (1)
9-9: Consistent import path for EvaluationSubstitutionType. Updated to reference the centralizedEvaluationConstantsmodule.app/client/src/widgets/TableWidget/widget/propertyConfig.ts (1)
4-4: Refactored import of EvaluationSubstitutionType. Now imported fromconstants/EvaluationConstantsto match the consolidated enum location.app/client/src/constants/PropertyControlConstants.tsx (1)
13-13: Centralize EvaluationSubstitutionType import
Switching the import ofEvaluationSubstitutionTypetoconstants/EvaluationConstantsaligns with the global refactor and keeps enum definitions in one place.app/client/src/workers/Evaluation/__tests__/evaluationSubstitution.test.ts (1)
2-2: Update test import for EvaluationSubstitutionType
The test now correctly imports the enum from the consolidatedconstants/EvaluationConstantsmodule.app/client/src/widgets/JSONFormWidget/widget/propertyConfig/properties/select.ts (1)
8-8: Align import path for EvaluationSubstitutionType
ImportingEvaluationSubstitutionTypefrom the new constants file maintains consistency across widget configs.app/client/src/widgets/wds/WDSTableWidget/config/propertyPaneConfig/contentConfig.ts (1)
3-3: Standardize EvaluationSubstitutionType import
The import source update ensures the table widget uses the centralized enum definition.app/client/src/widgets/JSONFormWidget/widget/propertyConfig.ts (1)
6-6: Correct import for EvaluationSubstitutionType
Import path updated to the sharedEvaluationConstants, matching other property configs.app/client/src/widgets/MultiSelectWidget/widget/index.tsx (1)
20-20: LGTM - Clean import refactoringThe centralization of
EvaluationSubstitutionTypetoconstants/EvaluationConstantsimproves code organization and consistency.app/client/src/components/formControls/DynamicTextFieldControl.tsx (1)
23-23: LGTM - Consistent with refactoring patternGood adherence to the centralized constants approach.
app/client/src/entities/Widget/utils.test.ts (1)
8-8: LGTM - Test consistency maintainedProper extension of the refactoring to test files ensures consistency across the codebase.
app/client/src/workers/common/DataTreeEvaluator/mockData/ArrayAccessorTree.ts (1)
10-11: LGTM - Selective import migrationWell-executed selective refactoring that only moves the target enum while preserving other imports in their appropriate locations.
app/client/src/sagas/JSPaneSagas.ts (1)
653-653: LGTM - Valuable analytics enhancementAdding
runBehaviourtracking provides useful insights into JS function execution patterns and aligns well with the reactive query improvements.app/client/src/widgets/MetaHOC.tsx (1)
94-94: Good performance optimization with early exit.This guard clause efficiently prevents unnecessary processing when there are no batch actions to execute, improving performance in the reactive flow.
app/client/src/ee/entities/DataTree/types/DataTreeSeed.ts (1)
1-1: Standard EE/CE re-export pattern.Clean implementation of the enterprise edition passthrough export structure.
app/client/src/entities/Action/actionProperties.ts (2)
3-3: Import consolidation to evaluation constants.Good refactoring to centralize evaluation constants in a dedicated file.
51-51: Added reactive path tracking for run behavior.This addition supports the enhanced reactive flow by tracking the "run" property for template evaluation, aligning with the PR's objective to execute reactive queries without manual refresh.
app/client/src/ce/utils/actionExecutionUtils.ts (1)
75-75: Enhanced analytics with run behavior tracking.Good addition to capture run behavior in analytics data, supporting better insights into reactive vs manual action execution patterns.
app/client/src/workers/Evaluation/types.ts (1)
74-74: Essential type definition for reactive action execution.This property enables the evaluation system to communicate which reactive actions need execution, directly supporting the PR's core objective of automatic reactive query execution.
app/client/src/ce/workers/Evaluation/evaluationUtils.test.ts (1)
47-47: Import path refactor looks good.The centralization of
EvaluationSubstitutionTypeto the constants module improves maintainability and follows good architectural practices.app/client/src/workers/common/DataTreeEvaluator/mockData/NestedArrayAccessorTree.ts (1)
11-11: Consistent import refactor applied correctly.The import path change aligns with the codebase-wide centralization effort and maintains the integrity of the mock data.
app/client/src/components/editorComponents/CodeEditor/index.tsx (1)
164-164: LGTM! Import refactoring aligns with centralization best practices.The consolidation of
EvaluationSubstitutionTypeinto a shared constants module improves maintainability and follows good architectural patterns.app/client/src/ce/sagas/index.tsx (2)
55-55: Good integration of new reactive query functionality.The import follows the established pattern for saga organization.
117-117: ```shell
#!/bin/bashVerify import path and saga registration in the root saga file
rg "import.*PostEvaluationSagas" -n app/client/src/ce/sagas/index.tsx
Display lines around the registration of PostEvaluationSagas
sed -n '100,130p' app/client/src/ce/sagas/index.tsx
</details> <details> <summary>app/client/src/ee/sagas/moduleInstanceSagaUtils.ts (1)</summary> `1-1`: Let's re-verify the CE file location with `find`, since the previous `fd` flags were invalid: ```shell #!/bin/bash # Search for the CE moduleInstanceSagaUtils file find . -type f -path "*/ce/sagas/moduleInstanceSagaUtils.ts"app/client/src/utils/FilterInternalProperties/__tests__/index.test.ts (1)
11-11: Consistent with codebase-wide import refactoring.The test maintains functionality while using the centralized type definition.
app/client/src/entities/DataTree/dataTreeWidget.test.ts (1)
11-11: Final piece of the systematic import refactoring.Maintains test integrity while using the centralized type definition, completing the consistent pattern across the codebase.
app/client/src/utils/autocomplete/__tests__/dataTreeTypeDefCreator.test.ts (1)
15-15: LGTM - Clean import refactorThe centralization of
EvaluationSubstitutionTypeto a dedicated constants module improves code organization.app/client/src/ce/constants/ReduxActionConstants.tsx (1)
621-622: LGTM - New action constants for reactive flowThese action constants properly support the reactive query execution feature described in the PR objectives. The naming follows established conventions.
app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts (3)
153-153: LGTM - Test updated for runBehaviour metadataThe addition of
runBehaviour: "MANUAL"aligns with the new reactive query execution system.Also applies to: 158-158
189-196: LGTM - Test updated for dynamic trigger pathsThe
dynamicTriggerPathListaddition properly reflects the new reactive trigger tracking functionality.
200-200: LGTM - Test updated for reactive data pathsThe addition of reactive paths for function data (
"myFun1.data","myFun2.data") supports the new reactive evaluation system.Also applies to: 202-202
app/client/src/workers/Evaluation/handlers/evalTrigger.ts (2)
24-24: LGTM - Evaluation tree cloning for reactive comparisonThe deep clone using
klonaprovides a snapshot of the previous evaluation state, enabling proper change detection in the reactive system.
40-40: LGTM - Passing cloned tree for evaluationThe old evaluation tree is correctly passed to enable comparison-based evaluation in the reactive flow.
app/client/src/entities/Action/actionProperties.test.ts (2)
4-4: LGTM: Import standardizationGood practice consolidating the import source for
EvaluationSubstitutionType.
43-43: LGTM: Consistent test updates for new reactive pathsThe addition of
runto expected reactive paths is consistently applied across all test cases, which aligns with the PR objective of supporting newly bound reactive queries.Also applies to: 97-97, 167-167, 227-227, 289-289, 305-305
app/client/src/workers/Evaluation/JSObject/utils.ts (2)
10-10: LGTM: Import standardization and new dependencyGood practice consolidating imports and adding the necessary
ActionRunBehaviourimport for the new functionality.Also applies to: 28-28
98-101: LGTM: Proper runBehaviour property handlingThe logic correctly preserves existing
runBehaviourvalues while providing a sensible default ofMANUALfor new actions.app/client/src/workers/Evaluation/evalTreeWithChanges.test.ts (2)
133-133: LGTM: Test parameter updates for new function signaturesCorrectly adapting test calls to match updated function signatures that now accept an additional
oldEvalTreeparameter.Also applies to: 229-229, 264-264, 304-304, 345-345, 379-379, 423-423, 469-469, 500-500, 532-532, 571-571
240-240: LGTM: Response structure updateAdding
executeReactiveActions: []to the expected response structure aligns with the new reactive query execution functionality.app/client/src/ce/workers/common/DependencyMap/utils/getEntityDependenciesByType.ts (1)
135-143: LGTM: JS function data dependency mappingExcellent addition of explicit dependencies from function
.dataproperties to their parent functions. The existence check ensures defensive programming and supports the reactive evaluation system.app/client/src/selectors/editorSelectors.tsx (1)
133-144: LGTM: Well-structured execution status selectorProperly implemented memoized selector that flattens on-load actions and combines them with execution status. The data structure with
idandisExecutedproperties is clean and useful for tracking reactive query execution.app/client/src/workers/common/DependencyMap/index.ts (1)
392-392: Parameter addition looks correct.Adding
configTreeto thesortDependenciescall aligns with the enhanced dependency sorting functionality mentioned in the summary. This supports the reactive dependency detection improvements.app/client/src/widgets/TableWidgetV2/widget/index.tsx (1)
990-994: LGTM! Deep equality check aligns with reactive flow improvements.The change from reference inequality to deep equality using
fast-deep-equalis appropriate for detecting actual content changes intableData. This ensures reactive updates trigger correctly when table data content changes, even if the reference remains the same.Note: Deep equality checks have higher computational cost than reference checks, but this is likely necessary for the enhanced reactive behavior described in the PR objectives.
app/client/src/ce/sagas/ActionExecution/ActionExecutionSagas.ts (1)
158-164: Improved error handling for invalid dynamic strings.The change from throwing an error to logging a warning and returning early is a more graceful approach to handling invalid
dynamicStringvalues. This prevents execution flow disruption while still providing visibility into the issue through logging.The validation logic correctly handles both falsy values and type mismatches, which enhances the robustness of the action execution system.
app/client/src/workers/Evaluation/JSObject/JSObject.test.ts (4)
12-12: Import addition looks goodAdding the
ActionRunBehaviourimport to support the new test data structure.
99-106: Good test data enhancementAdding
dynamicTriggerPathListto the test configuration aligns with the new reactive action tracking features.
182-188: Appropriate metadata additionsThe
runBehaviourproperties ensure tests reflect the enhanced JS action metadata structure.
228-235: Test data consistency maintainedThe
dynamicTriggerPathListaddition to unEvalDataTree keeps test data consistent with the evaluation framework changes.app/client/src/sagas/ActionExecution/PluginActionSaga.ts (4)
873-873: Good defensive programmingAdding optional chaining prevents potential runtime errors when
pluginis undefined.
967-967: Analytics enhancement approvedIncluding
runBehaviourin analytics provides valuable insights into action execution patterns.
1129-1129: Consistent analytics improvementAdding
runBehaviourto page load action analytics maintains consistency with the overall analytics strategy.
1167-1170: Redux integration looks solidDispatching
SET_ONLOAD_ACTION_EXECUTEDenables downstream reactive query execution tracking. The payload structure is appropriate.app/client/src/workers/Evaluation/handlers/evalTree.ts (5)
36-36: Appropriate cloning utility importUsing
klona/jsonfor deep cloning is a good choice for performance and reliability.
71-71: Clean initializationInitializing
executeReactiveActionsas an empty array provides a clear starting point for tracking.
140-140: Consistent reactive action trackingThe assignment pattern across different evaluation paths ensures
executeReactiveActionsis captured from all evaluation scenarios.Also applies to: 193-193, 256-256
197-197: Smart state preservation approachDeep cloning the current tree before updates enables proper comparison for detecting reactive actions. Passing
oldDataTreetoevalAndValidateSubTreeis the correct approach.Also applies to: 246-246
366-366: Complete data flow integrationIncluding
executeReactiveActionsin the response enables downstream saga processing for reactive query execution.app/client/src/layoutSystems/common/DSLConversions/fixedToAutoLayout.ts (1)
812-813: Good type safety improvement.The change from generic type to specific
DSLWidgettype is appropriate since this function is only used withDSLWidgetobjects. This improves type safety and removes unnecessary generic complexity.app/client/src/sagas/EvaluationsSaga.ts (2)
150-150: LGTM on parameter extraction.The addition of
executeReactiveActionsto the destructuring follows the existing pattern and is properly typed.
163-167: Feature flag pattern looks good.The feature flag selection and usage follows established patterns in the codebase.
app/client/src/workers/Evaluation/__tests__/evaluation.test.ts (3)
19-20: Good import reorganization.Moving
EvaluationSubstitutionTypetoconstants/EvaluationConstantsand importingActionRunBehaviourfrom the appropriate types file improves code organization.
301-302: Appropriate test data updates.The addition of
dynamicTriggerPathListandrunBehaviourproperties with sensible defaults aligns with the new reactive action metadata requirements.
636-637: Correct test updates for new method signature.The addition of empty array and
evaluator.evalTreeparameters toevalAndValidateSubTreecalls properly reflects the updated method signature for reactive action support.Also applies to: 676-677, 724-725, 789-790, 834-835, 900-901, 973-974, 1009-1010, 1052-1053, 1096-1097, 1137-1138
app/client/src/ce/sagas/moduleInstanceSagaUtils.ts (1)
6-12: Well-defined interface.The
ParamsForJSModuleInstanceinterface provides clear typing for the module instance execution parameters.app/client/src/entities/DependencyMap/__tests__/index.test.ts (2)
2746-2746: Test expectation updated to reflect new dependency behavior.The test now expects
apiDatato haveundefineddependencies instead of being linked toapiData.data. This aligns with the PR's changes to exclude.datapaths from automatic parent-child dependency creation.
2816-2817: Test expectations correctly updated for new dependency logic.The test now expects:
addDependencyto be called 0 times (previously expected to be called)- Direct dependencies of "apiData" to be empty
This reflects the refined dependency model where certain
.datapath dependencies are no longer automatically created.app/client/src/ce/entities/DataTree/dataTreeJSAction.ts (3)
3-3: LGTM: Import path centralizationGood refactoring to centralize
EvaluationSubstitutionTypeinconstants/EvaluationConstants.
22-22: LGTM: Reactive paths implementationProper implementation of reactive paths for JS actions. The merge with binding paths maintains backward compatibility while enabling reactive execution.
Also applies to: 74-76, 94-94
54-54: LGTM: Dynamic trigger paths and run behavior trackingCorrect implementation of dynamic trigger path tracking and run behavior preservation for JS actions. This properly supports the reactive query execution flow.
Also applies to: 63-63, 73-73, 99-99
app/client/src/ce/entities/DataTree/dataTreeAction.ts (2)
12-12: LGTM: Consistent import path centralizationSame improvement as in the previous file for centralizing evaluation constants.
77-81: LGTM: Dependency and trigger path setupCorrect implementation of dependency mapping and dynamic trigger paths for actions. The "run" dependency including all dynamic binding paths properly supports reactive execution.
Also applies to: 112-113
app/client/src/workers/common/DataTreeEvaluator/dataTreeEvaluator.test.ts (3)
27-27: LGTM: Test updates for new evaluation signatureConsistent import path change and correct parameter updates for the enhanced evaluation method signature.
Also applies to: 307-309
484-489: LGTM: Updated dependency assertions for refined logicCorrect updates to dependency expectations that align with the new reactive dependency handling logic.
Also applies to: 509-515, 582-588
799-805: LGTM: JS action metadata test updatesProper test data updates that include the new
runBehaviouranddynamicTriggerPathListproperties with appropriate values.Also applies to: 846-853, 861-867, 908-915
app/client/src/ce/entities/DataTree/types.ts (1)
13-14: LGTM: Consistent interface updates for reactive functionalityProper addition of
dynamicTriggerPathListandrunBehaviourproperties across all relevant interfaces. The type imports are appropriate and consistent.Also applies to: 63-65, 72-72, 90-90
app/client/src/ce/reducers/uiReducers/editorReducer.tsx (3)
42-42: LGTM!The initialization of
onLoadActionExecutionas an empty object is appropriate.
333-344: LGTM!The handler correctly updates the execution status for individual actions.
134-136: ```bash
#!/bin/bash
rg "SavePageResponse" -n .</details> <details> <summary>app/client/src/ce/workers/Evaluation/evaluationUtils.ts (1)</summary> `1148-1157`: **Move visited set initialization to avoid shared state.** The default `visited` parameter could be shared between calls if the function is called multiple times. Consider initializing it within the function or in a wrapper. ```diff export function getExternalChangedDependencies( property: string, dependencies: Record<string, string[]>, valuechanged: Record<string, boolean>, entityName: string, - visited = new Set<string>(), + visited?: Set<string>, ): boolean { + if (!visited) { + visited = new Set<string>(); + } if (visited.has(property)) return false;Likely an incorrect or invalid review comment.
app/client/src/workers/common/DataTreeEvaluator/index.ts (4)
158-158: Import source change looks goodThe import of
EvaluationSubstitutionTypehas been moved to a more appropriate location in the constants directory.
345-349: Good addition of configTree parameter to sortDependenciesThe optional
configTreeparameter enables better dependency sorting with additional context. The implementation correctly passes it toDependencyMapUtils.sortDependencies.Also applies to: 1575-1580
1459-1487: Good optimization for JS variable evaluationThe logic correctly avoids re-evaluating JS variables when their uneval value hasn't changed. This is a good performance optimization.
785-796: Potential issue with overly broad.datapath filteringThe current logic filters out any path ending with
.data, which could inadvertently exclude legitimate properties that happen to end with this suffix.Consider using the
isDataPathutility more precisely:- pathsToSkipFromEval = pathsToSkipFromEval.filter((path) => { - if (!path.endsWith(".data")) return true; - - const { entityName } = getEntityNameAndPropertyPath(path); - const entity = unEvalTree[entityName]; - - if (entity && isDataPath(entity, path)) { - return false; // filter out - } - - return true; // keep for other entity types - }); + pathsToSkipFromEval = pathsToSkipFromEval.filter((path) => { + const { entityName } = getEntityNameAndPropertyPath(path); + const entity = unEvalTree[entityName]; + + // Only filter out if it's specifically a data path for the entity + return !(entity && isDataPath(entity, path)); + });Likely an incorrect or invalid review comment.
2 participants
Description
Updating
updateDependenciesmap to execute newly binded queries in reactive flow without needing to refresh page to make it work as expected.Fixes #41017
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/15785768276
Commit: b7c967b
Cypress dashboard.
Tags:
@tag.AllSpec:
Fri, 20 Jun 2025 20:15:44 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Developer Experience