chore: optimise first evaluation by adding worker scope cache (CE) #38068
Conversation
WalkthroughThe changes in this pull request enhance the handling of JavaScript action entities by introducing a new property, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
app/client/src/workers/Evaluation/__tests__/Actions.test.ts (1)
551-551: Simplify redundant type assertionThe type assertion
dataTree as unknown as DataTreecan be simplified todataTree as DataTree.Apply this diff to simplify the type assertion:
- const EVAL_CONTEXT = getDataTreeContext({ - dataTree: dataTree as unknown as DataTree, + const EVAL_CONTEXT = getDataTreeContext({ + dataTree: dataTree as DataTree, configTree, isTriggerBased: true, });app/client/src/sagas/ActionExecution/PluginActionSaga.ts (1)
Line range hint
338-346: Define a specific type forfilePickerInfoConsider defining an interface for
filePickerInfoinstead of usingRecord<string, any>to enhance type safety.Define an interface for
filePickerInfo:interface FilePickerInstrumentation { numberOfFiles: number; totalSize: number; fileTypes: string[]; fileSizes: number[]; }Update the function signature:
-function triggerFileUploadInstrumentation( - filePickerInfo: Record<string, any>, +function triggerFileUploadInstrumentation( + filePickerInfo: FilePickerInstrumentation, status: string, statusCode: string, pluginName: string, pluginType: string, timeTaken: string, ) {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (22)
app/client/src/ce/entities/DataTree/dataTreeJSAction.ts(1 hunks)app/client/src/ce/entities/DataTree/types.ts(1 hunks)app/client/src/ce/entities/FeatureFlag.ts(2 hunks)app/client/src/ce/workers/Evaluation/Actions.ts(3 hunks)app/client/src/ce/workers/Evaluation/evaluationUtils.ts(1 hunks)app/client/src/entities/Engine/AppViewerEngine.ts(0 hunks)app/client/src/index.tsx(1 hunks)app/client/src/sagas/ActionExecution/PluginActionSaga.ts(1 hunks)app/client/src/sagas/ActionExecution/geolocationSaga.ts(1 hunks)app/client/src/sagas/EvalWorkerActionSagas.ts(1 hunks)app/client/src/sagas/EvaluationsSaga.test.ts(1 hunks)app/client/src/sagas/EvaluationsSaga.ts(3 hunks)app/client/src/sagas/JSLibrarySaga.ts(1 hunks)app/client/src/utils/workerInstances.ts(1 hunks)app/client/src/workers/Evaluation/JSObject/test.ts(1 hunks)app/client/src/workers/Evaluation/__tests__/Actions.test.ts(2 hunks)app/client/src/workers/Evaluation/__tests__/evaluate.test.ts(1 hunks)app/client/src/workers/Evaluation/evaluate.ts(5 hunks)app/client/src/workers/Evaluation/handlers/jsLibrary.ts(1 hunks)app/client/src/workers/Evaluation/handlers/workerEnv.ts(2 hunks)app/client/src/workers/common/DataTreeEvaluator/index.ts(16 hunks)app/client/src/workers/common/DataTreeEvaluator/test.ts(2 hunks)
💤 Files with no reviewable changes (1)
- app/client/src/entities/Engine/AppViewerEngine.ts
👮 Files not reviewed due to content moderation or server errors (5)
- app/client/src/workers/Evaluation/JSObject/test.ts
- app/client/src/sagas/EvaluationsSaga.test.ts
- app/client/src/workers/Evaluation/handlers/jsLibrary.ts
- app/client/src/workers/Evaluation/tests/evaluate.test.ts
- app/client/src/ce/workers/Evaluation/evaluationUtils.ts
🔇 Additional comments (35)
app/client/src/workers/Evaluation/__tests__/Actions.test.ts (2)
9-9: Import statement updated appropriately
The import of getDataTreeContext is correctly added.
Line range hint 399-426: Refactored evaluation context setup is correct
The changes in evaluateSync correctly update the evaluation context and handle scopeCache appropriately.
app/client/src/workers/Evaluation/evaluate.ts (5)
26-26: Import getDataTreeContext
The import statement for getDataTreeContext is correctly added.
106-106: Export resetWorkerGlobalScope function
Exporting resetWorkerGlobalScope allows external access, which is appropriate for the updated usage.
276-284: Proper use of getDataTreeContext
The evaluation context is correctly initialized and merged using getDataTreeContext.
377-378: Add optional scopeCache parameter
The addition of scopeCache as an optional parameter enhances the flexibility of the evaluateSync function.
399-426: Refactored context setup in evaluateSync
The refactoring improves clarity by directly constructing EVAL_CONTEXT and conditionally assigning scopeCache.
app/client/src/sagas/EvaluationsSaga.ts (3)
28-28: Update import for evalWorker
The import of evalWorker from utils/workerInstances is correctly updated.
120-120: Import waitForFetchEnvironments
The addition of waitForFetchEnvironments import is appropriate for ensuring environment data availability.
297-298: Ensure environment data is fetched before evaluation
Calling waitForFetchEnvironments before evaluation ensures that environment variables are properly loaded.
app/client/src/sagas/ActionExecution/PluginActionSaga.ts (1)
105-106: Import evalWorker from utils/workerInstances
The import statement for evalWorker is correctly updated.
app/client/src/workers/common/DataTreeEvaluator/index.ts (9)
41-41: Imported isPropertyAnEntityAction
The import of isPropertyAnEntityAction from "ee/workers/Evaluation/evaluationUtils" is appropriate.
90-94: Added Necessary Imports
The new imports for resetWorkerGlobalScope, EvalResult, and EvaluateContext are correctly added.
1070-1071: Reset Worker Global Scope
Calling resetWorkerGlobalScope() ensures a clean evaluation environment before processing.
1097-1108: Initialized Data Tree Contexts
Creation of triggerBasedDataTreeContext and nonTriggerBasedDataTreeContext enhances context management during evaluation.
1118-1121: Skip Evaluation for Entity Actions
The condition to skip evaluation of properties that are entity actions is correctly implemented.
1164-1175: Conditional Scope Cache Based on Feature Flag
Properly checks the release_evaluation_scope_cache feature flag and sets scopeCache accordingly.
1185-1185: Passed scopeCache to getDynamicValue
Adding scopeCache as a parameter enhances evaluation flexibility.
1499-1499: Updated getDynamicValue Signature
Including scopeCache in the function parameters improves context handling.
Line range hint 1550-1560: Passed scopeCache to evaluateSync
Ensures consistent evaluation by passing scopeCache.
app/client/src/utils/workerInstances.ts (1)
1-13: Introduced evalWorker Instance
Creation of evalWorker using GracefulWorkerService enhances worker management.
app/client/src/workers/Evaluation/handlers/workerEnv.ts (3)
1-1: Imported Feature Flag Types
Importing FeatureFlags and FeatureFlag ensures type safety.
4-4: Typed flags Property
Defining flags as FeatureFlags improves code clarity and type checking.
23-25: Added isFFEnabled Method
Implemented isFFEnabled to check feature flag states efficiently.
app/client/src/ce/entities/DataTree/dataTreeJSAction.ts (1)
92-92: Included actionNames in configEntity
Adding actionNames array enhances access to action identifiers.
app/client/src/ce/entities/FeatureFlag.ts (1)
46-46: LGTM! Verify feature flag integration
The feature flag follows the naming convention and is properly integrated.
Also applies to: 85-85
✅ Verification successful
Feature flag is properly integrated and used
The feature flag is correctly implemented in FeatureFlag.ts and is being used appropriately in DataTreeEvaluator for conditional scope caching optimization.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify feature flag usage across the codebase
rg -l "release_evaluation_scope_cache" | grep -v "FeatureFlag.ts"
Length of output: 120
Script:
#!/bin/bash
# Check how the feature flag is used in DataTreeEvaluator
rg -C 3 "release_evaluation_scope_cache" app/client/src/workers/common/DataTreeEvaluator/index.ts
Length of output: 341
app/client/src/index.tsx (1)
3-4: LGTM! Worker initialization properly placed
Early initialization of the eval worker ensures its availability throughout the application lifecycle.
app/client/src/sagas/ActionExecution/geolocationSaga.ts (1)
8-8: LGTM! Verify worker instance usage
Import source updated to use centralized worker instance.
✅ Verification successful
Import change verified and consistent across codebase
All instances of evalWorker are now correctly imported from utils/workerInstances, including the changed file. No remaining references to the old import path from sagas/EvaluationsSaga were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify consistent usage of evalWorker import across the codebase
rg -l "import.*evalWorker.*from.*['\"]sagas/EvaluationsSaga['\"]"
Length of output: 71
Script:
#!/bin/bash
# Search for evalWorker imports across the codebase
rg "import.*evalWorker" -A 2
# Search for evalWorker usage to ensure consistent patterns
ast-grep --pattern 'import { evalWorker } from "$_"'
Length of output: 8740
app/client/src/sagas/EvalWorkerActionSagas.ts (2)
18-18: LGTM: Worker instance import added
The addition of evalWorker import aligns with the optimization goal.
Line range hint 99-101: Verify message handling flow
The new evalWorker.respond implementation needs verification to ensure proper message handling.
✅ Verification successful
Message handling implementation is properly structured
The evalWorker.respond implementation in GracefulWorkerService is robust and handles message passing correctly:
- Ensures worker readiness before sending messages
- Properly handles message IDs and response routing
- Includes telemetry and error handling
- Maintains proper cleanup of channels
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other evalWorker.respond usages
rg -A 3 "evalWorker\.respond"
Length of output: 332
Script:
#!/bin/bash
# Check the implementation of evalWorker.respond
ast-grep --pattern 'evalWorker = {
$$$
respond($$$) {
$$$
}
$$$
}'
# Check for message handling patterns
rg -A 5 "messageId.*result"
Length of output: 633
Script:
#!/bin/bash
# Let's check the worker implementation and message handling
rg -B 5 -A 10 "handleEvalWorkerMessage"
# Check for any worker setup or initialization
rg -B 3 "evalWorker.*=.*"
Length of output: 3185
Script:
#!/bin/bash
# Let's check the GracefulWorkerService implementation
ast-grep --pattern 'class GracefulWorkerService {
$$$
}'
# Check for any message handling types
rg "type TMessage"
Length of output: 21358
app/client/src/ce/workers/Evaluation/Actions.ts (2)
Line range hint 43-55: Function renamed for clarity and parameters simplified
The renaming from addDataTreeToContext to getDataTreeContext better reflects its purpose. The parameter structure is now more focused.
🧰 Tools
🪛 Biome (1.9.4)
[error] 58-58: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
104-106: Performance optimization: Early return for non-trigger evaluations
Good optimization. Early return prevents unnecessary processing for non-trigger based evaluations.
app/client/src/ce/entities/DataTree/types.ts (1)
99-99: Verify the usage of new actionNames property
The addition of actionNames to JSActionEntityConfig is good. However, we should verify its usage across the codebase.
✅ Verification successful
Let me gather more information about how this property is being used in the actual implementation.
The actionNames property is properly implemented and used
The property is correctly used across the codebase:
- Populated in
dataTreeJSAction.tsby mapping action names - Used in evaluation utils for property path validation
- Properly referenced in test files and entity selectors
- Type is consistently used across multiple modules
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check usage of actionNames in JSActionEntityConfig
ast-grep --pattern 'interface JSActionEntityConfig {
$$$
actionNames: $_
$$$
}'
# Check references to actionNames
rg "actionNames.*:" -A 3
Length of output: 1637
Script:
#!/bin/bash
# Check for actionNames usage in implementation
rg "actionNames" -A 5 --type ts
# Check for JSActionEntityConfig usage
ast-grep --pattern 'JSActionEntityConfig'
Length of output: 5093
app/client/src/workers/common/DataTreeEvaluator/test.ts (1)
771-771: LGTM: Action names array matches the declared functions
The added actionNames arrays correctly list all the functions declared in their respective JSObjects' meta configurations.
Also applies to: 825-825
app/client/src/sagas/JSLibrarySaga.ts (1)
24-24: LGTM! Verify worker instance consistency
The import change aligns with the centralized worker instance management.
✅ Verification successful
The previous search didn't yield results because we were looking for a specific import pattern. Let's broaden the search to understand the worker instance usage patterns across the codebase and verify the consistency of imports.
Import change is consistent with codebase patterns
The import change in JSLibrarySaga.ts aligns with the established pattern across the codebase. All worker instance imports consistently come from "utils/workerInstances", with some files using the default import and others using named imports like in your change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify consistent worker instance imports across the codebase
rg -l "import.*evalWorker.*from.*EvaluationsSaga"
Length of output: 51
Script:
#!/bin/bash
# Search for evalWorker imports across the codebase
rg "import.*evalWorker" -A 2
# Also check for any direct references to EvaluationsSaga and worker
rg "EvaluationsSaga.*worker" -A 2
# Look for worker instance imports
rg "from.*workerInstances"
Length of output: 8733
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
app/client/src/ce/workers/Evaluation/Actions.ts (2)
Line range hint
55-97: Consider simplifying the entity functions logicThe
skipEntityFunctionscondition is counter-intuitive:const skipEntityFunctions = !removeEntityFunctions && !isTriggerBased;It reads as "skip if we're not removing functions and not trigger-based", which is complex to reason about.
Consider inverting the condition for clarity:
-const skipEntityFunctions = !removeEntityFunctions && !isTriggerBased; -if (skipEntityFunctions) continue; +const shouldProcessEntityFunctions = removeEntityFunctions || isTriggerBased; +if (!shouldProcessEntityFunctions) continue;🧰 Tools
🪛 Biome (1.9.4)
[error] 58-58: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
98-114: Consider consolidating the return logicThe multiple conditional returns make the control flow harder to follow. Consider using a single return with a conditional expression.
- if (removeEntityFunctions) { - removeEntityFunctionsFromEvalContext( - entityFunctionCollection, - EVAL_CONTEXT, - ); - return EVAL_CONTEXT; - } - - if (!isTriggerBased) { - return EVAL_CONTEXT; - } - - addEntityFunctionsToEvalContext(EVAL_CONTEXT, entityFunctionCollection); - return EVAL_CONTEXT; + if (removeEntityFunctions) { + removeEntityFunctionsFromEvalContext(entityFunctionCollection, EVAL_CONTEXT); + } else if (isTriggerBased) { + addEntityFunctionsToEvalContext(EVAL_CONTEXT, entityFunctionCollection); + } + return EVAL_CONTEXT;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/src/ce/workers/Evaluation/Actions.ts(3 hunks)
🔇 Additional comments (2)
app/client/src/ce/workers/Evaluation/Actions.ts (2)
Line range hint 43-55: Well-structured function signature refactor!
The renaming and parameter restructuring improves clarity and encapsulation. The function name now better reflects its purpose.
🧰 Tools
🪛 Biome (1.9.4)
[error] 58-58: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
Line range hint 43-114: Verify all callers are updated with the new function signature
The function signature change from addDataTreeToContext to getDataTreeContext requires updates in all calling code.
✅ Verification successful
All callers have been updated with the new function signature
The search results show that:
- All references to
addDataTreeToContextonly exist in test file names/descriptions - All actual usage points are using the new
getDataTreeContextwith correct parameters including:dataTreeconfigTreeisTriggerBased- Optional
removeEntityFunctions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to the old function name
rg "addDataTreeToContext" --type ts
# Search for calls to the new function to verify parameter structure
rg "getDataTreeContext" -A 3 --type ts
Length of output: 3537
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
app/client/src/workers/Evaluation/fns/resetWidget.ts (2)
Line range hint
41-44: Consider using a more explicit function signature.The tuple type for arguments could be made more readable by using a regular parameter list.
-async function resetWidget( - ...args: [widgetName: string, resetChildren: boolean] -) { - const widgetName = args[0]; - const resetChildren = args[1]; +async function resetWidget( + widgetName: string, + resetChildren: boolean +) {
Line range hint
190-270: Consider breaking down the complex descendant resolution logic.The
getWidgetDescendantToResetfunction handles multiple responsibilities and could be split into smaller, more focused functions for better maintainability and testing.Consider splitting into:
- A function to handle meta widgets
- A function to handle canvas widgets
- A utility function for common descendant logic
+function getMetaWidgetDescendants( + sortedWidgetsMeta: ReturnType<typeof sortWidgetsMetaByParent>, + evaluatedDataTree: DataTree +): DescendantWidgetMap[] { + const descendants: DescendantWidgetMap[] = []; + for (const childMetaWidgetId of Object.keys( + sortedWidgetsMeta.childrenWidgetsMeta, + )) { + // Meta widget handling logic here + } + return descendants; +} +function getCanvasWidgetDescendants( + widget: FlattenedWidgetProps, + canvasWidgets: CanvasWidgetsReduxState, + evaluatedDataTree: DataTree +): DescendantWidgetMap[] { + const descendants: DescendantWidgetMap[] = []; + // Canvas widget handling logic here + return descendants; +}app/client/src/workers/Evaluation/evaluate.ts (3)
Line range hint
106-127: Add JSDoc comments to document the critical nature of this functionThis function is crucial for preventing memory leaks and scope pollution between evaluations. Consider adding JSDoc comments to explain:
- Purpose of global scope reset
- When it should be called
- What it preserves/removes
369-378: Consider adding performance monitoring for scopeCache optimizationThe addition of scopeCache parameter is a good optimization. Consider:
- Adding telemetry to measure the performance impact
- Documenting the expected performance gains
Line range hint
1163-1173: LGTM! Consider adding monitoring for the feature flagGood use of feature flag for the scope cache optimization. Consider:
- Adding metrics to track cache hit rates
- Monitoring memory usage with the cache
app/client/src/workers/common/DataTreeEvaluator/index.ts (1)
1069-1070: LGTM! Consider adding performance monitoringGood separation of trigger and non-trigger contexts. Consider:
- Adding performance metrics for context creation
- Monitoring memory usage with separate contexts
Also applies to: 1096-1107
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
app/client/src/workers/Evaluation/JSObject/index.ts(1 hunks)app/client/src/workers/Evaluation/__tests__/evaluate.test.ts(6 hunks)app/client/src/workers/Evaluation/evaluate.ts(5 hunks)app/client/src/workers/Evaluation/fns/resetWidget.ts(1 hunks)app/client/src/workers/common/DataTreeEvaluator/index.ts(16 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/client/src/workers/Evaluation/JSObject/index.ts
🔇 Additional comments (6)
app/client/src/workers/Evaluation/fns/resetWidget.ts (2)
18-18: LGTM! Good improvement in import style.
The switch to named imports enhances tree-shaking capabilities and follows modern JavaScript best practices.
Line range hint 127-134: Verify context switching safety.
The switch between sync and async evaluation contexts could lead to race conditions if multiple resets occur simultaneously.
app/client/src/workers/Evaluation/evaluate.ts (1)
26-26: LGTM! Verify the impact of switching to getDataTreeContext
The change from addDataTreeToContext to getDataTreeContext improves immutability.
✅ Verification successful
The results will help verify if the new function is properly implemented, tested, and all usages are consistent with the change.
Change to getDataTreeContext is properly integrated across the codebase
The function is consistently used in evaluation contexts across the worker files, with proper test coverage in place. The old addDataTreeToContext only appears in a test description and has been fully replaced.
- Used in DataTreeEvaluator for both trigger and non-trigger based contexts
- Properly implemented in CE worker actions
- Test coverage exists in Actions.test.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining usages of addDataTreeToContext
rg "addDataTreeToContext"
Length of output: 138
Script:
#!/bin/bash
# Check the implementation of getDataTreeContext and its test coverage
ast-grep --pattern 'function getDataTreeContext' -A 10
Length of output: 56
Script:
#!/bin/bash
# Check for any test files related to getDataTreeContext
rg "getDataTreeContext" -g "*.test.*"
Length of output: 225
Script:
#!/bin/bash
# Check for all usages of getDataTreeContext
rg "getDataTreeContext"
Length of output: 970
app/client/src/workers/Evaluation/__tests__/evaluate.test.ts (1)
1-4: LGTM! Test coverage looks comprehensive
Good test coverage for the new evaluateSync functionality and scope cache changes.
app/client/src/workers/common/DataTreeEvaluator/index.ts (2)
41-41: LGTM! Clean import organization
The new imports are well organized and support the evaluation optimization changes.
Also applies to: 90-94
1117-1119: LGTM! Good optimization for entity actions
Skipping evaluation for entity actions is a good optimization that prevents unnecessary work.
dvj1988
changed the title
dvj1988
changed the title
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12250989683. |
|
Deploy-Preview-URL: https://ce-38068.dp.appsmith.com |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
app/client/src/workers/common/DataTreeEvaluator/index.ts (2)
1096-1107: Consider caching the context objectsThe context objects are created for each evaluation cycle. Consider caching them at the class level to avoid repeated object creation.
- const triggerBasedDataTreeContext = getDataTreeContext({ - dataTree: contextTree, - configTree: oldConfigTree, - isTriggerBased: true, - }); - - const nonTriggerBasedDataTreeContext = getDataTreeContext({ - dataTree: contextTree, - configTree: oldConfigTree, - isTriggerBased: false, - }); + this.triggerBasedDataTreeContext = this.triggerBasedDataTreeContext || getDataTreeContext({ + dataTree: contextTree, + configTree: oldConfigTree, + isTriggerBased: true, + }); + + this.nonTriggerBasedDataTreeContext = this.nonTriggerBasedDataTreeContext || getDataTreeContext({ + dataTree: contextTree, + configTree: oldConfigTree, + isTriggerBased: false, + });
1163-1173: Consider extracting scope cache selection logicThe scope cache selection logic could be extracted into a separate method for better readability and reusability.
+ private getScopeCache(entity: DataTreeEntity | undefined): EvaluateContext | undefined { + if (!WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache)) { + return undefined; + } + if (!!entity && isAnyJSAction(entity)) { + return this.triggerBasedDataTreeContext; + } + return this.nonTriggerBasedDataTreeContext; + } - let scopeCache; - if (WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache)) { - if (!!entity && isAnyJSAction(entity)) { - scopeCache = triggerBasedDataTreeContext; - } else { - scopeCache = nonTriggerBasedDataTreeContext; - } - } + const scopeCache = this.getScopeCache(entity);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/src/workers/common/DataTreeEvaluator/index.ts(16 hunks)
🔇 Additional comments (2)
app/client/src/workers/common/DataTreeEvaluator/index.ts (2)
1069-1070: LGTM: Global scope reset before evaluation
The addition of resetWorkerGlobalScope() ensures a clean evaluation context.
1117-1119: LGTM: Skip evaluation for entity actions
Good optimization to skip evaluation for entity actions.
| if ( | ||
| WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache) | ||
| ) { | ||
| set( | ||
| triggerBasedDataTreeContext, | ||
| fullPropertyPath, | ||
| klona(parsedValue), | ||
| ); | ||
| set( | ||
| nonTriggerBasedDataTreeContext, | ||
| fullPropertyPath, | ||
| klona(parsedValue), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract duplicate scope cache update logic
The scope cache update logic is duplicated across multiple switch cases. Consider extracting it into a helper method.
+ private updateScopeCaches(fullPropertyPath: string, value: unknown) {
+ if (!WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache)) {
+ return;
+ }
+ const clonedValue = klona(value);
+ set(this.triggerBasedDataTreeContext, fullPropertyPath, clonedValue);
+ set(this.nonTriggerBasedDataTreeContext, fullPropertyPath, clonedValue);
+ }
- if (WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache)) {
- set(triggerBasedDataTreeContext, fullPropertyPath, klona(parsedValue));
- set(nonTriggerBasedDataTreeContext, fullPropertyPath, klona(parsedValue));
- }
+ this.updateScopeCaches(fullPropertyPath, parsedValue);Also applies to: 1311-1324, 1367-1382, 1397-1410
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts (1)
194-194: Consider adding test for empty actions arrayThe test suite verifies the happy path with two actions but lacks coverage for edge cases.
Consider adding a test case for an empty actions array:
it("handles empty actions array", () => { const jsCollection = { // ... existing test data config: { // ... existing config actions: [], } }; const result = generateDataTreeJSAction(jsCollection); expect(result.configEntity.actionNames).toEqual([]); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts(2 hunks)app/client/src/workers/Evaluation/__tests__/evaluation.test.ts(0 hunks)app/client/src/workers/Evaluation/handlers/workerEnv.ts(1 hunks)app/client/src/workers/common/DataTreeEvaluator/index.ts(16 hunks)
💤 Files with no reviewable changes (1)
- app/client/src/workers/Evaluation/tests/evaluation.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/workers/Evaluation/handlers/workerEnv.ts
🔇 Additional comments (3)
app/client/src/workers/common/DataTreeEvaluator/index.ts (3)
1068-1069: LGTM: Clean worker scope initialization
Good practice to reset the worker's global scope before starting evaluation.
1116-1118: LGTM: Efficient action property handling
Good optimization to skip evaluation of entity action properties.
1095-1105: LGTM: Well-structured context separation
The separation of trigger-based and non-trigger-based contexts is a good optimization. Let's verify the context usage patterns.
✅ Verification successful
Let me check the usage of these contexts to ensure they're being used appropriately.
LGTM: Context separation is properly implemented and utilized
The contexts are correctly used throughout the codebase:
- Appropriate context switching based on trigger condition
- Consistent usage in property evaluations with proper cloning
- Clear separation maintained in all evaluation paths
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check usage patterns of getDataTreeContext
ast-grep --pattern 'getDataTreeContext({
$$$
isTriggerBased: $_,
$$$
})'
Length of output: 1968
Script:
#!/bin/bash
# Search for usage of triggerBasedDataTreeContext and nonTriggerBasedDataTreeContext
rg "triggerBasedDataTreeContext|nonTriggerBasedDataTreeContext" -A 2
Length of output: 3486
| if (WorkerEnv.flags.release_evaluation_scope_cache) { | ||
| set( | ||
| triggerBasedDataTreeContext, | ||
| fullPropertyPath, | ||
| klona(parsedValue), | ||
| ); | ||
| set( | ||
| nonTriggerBasedDataTreeContext, | ||
| fullPropertyPath, | ||
| klona(parsedValue), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting duplicated scope cache update logic
The scope cache update logic is duplicated across multiple switch cases. This could be extracted into a helper method to improve maintainability.
+ private updateScopeCaches(fullPropertyPath: string, value: unknown) {
+ if (!WorkerEnv.flags.release_evaluation_scope_cache) {
+ return;
+ }
+ const clonedValue = klona(value);
+ set(this.triggerBasedDataTreeContext, fullPropertyPath, clonedValue);
+ set(this.nonTriggerBasedDataTreeContext, fullPropertyPath, clonedValue);
+ }
- if (WorkerEnv.flags.release_evaluation_scope_cache) {
- set(triggerBasedDataTreeContext, fullPropertyPath, klona(parsedValue));
- set(nonTriggerBasedDataTreeContext, fullPropertyPath, klona(parsedValue));
- }
+ this.updateScopeCaches(fullPropertyPath, parsedValue);Also applies to: 1306-1317, 1360-1371, 1386-1397
| @@ -191,6 +191,7 @@ describe("generateDataTreeJSAction", () => { | |||
| myVar1: "SMART_SUBSTITUTE", | |||
| myVar2: "SMART_SUBSTITUTE", | |||
| }, | |||
| actionNames: ["myFun2", "myFun1"], | |||
There was a problem hiding this comment.
💡 Codebase verification
The order of action names in tests doesn't match the implementation
The implementation in dataTreeJSAction.ts shows that action names are derived directly from the actions array using actions.map((action) => action.name), but the test expectations have a reversed order ["myFun2", "myFun1"] compared to how they appear in the test's actions array. This inconsistency should be fixed.
- In
dataTreeJSAction.test.ts: Update the expectedactionNamesarray to match the actual order of actions in the input data.
🔗 Analysis chain
Verify action names order consistency
The actionNames array maintains the same order ["myFun2", "myFun1"] in both test cases. Ensure this order matches the intended evaluation order in the actual implementation.
Also applies to: 393-393
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the order of action names in the implementation
# Expected: The order of actions in generateDataTreeJSAction should match the test expectations
# Search for the implementation of generateDataTreeJSAction
ast-grep --pattern 'function generateDataTreeJSAction($_) {
$$$
}'
Length of output: 70
Script:
#!/bin/bash
# Let's try a different approach to find the implementation
# First, let's find the file containing the implementation
rg -l "generateDataTreeJSAction" --type ts
# Then let's look at the implementation with context
rg "generateDataTreeJSAction" --type ts -B 5 -A 20
# Also search for any related types or interfaces
rg "interface.*JSActionConfig" --type ts -A 5
Length of output: 13478
Script:
#!/bin/bash
# Let's look at the actions array in the test file to verify the order
rg -A 10 "actions: \[" app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts
# And check if there's any logic in the implementation that determines the action order
rg -B 5 -A 10 "actionNames" app/client/src/ce/entities/DataTree/dataTreeJSAction.ts
Length of output: 1244
| @@ -305,6 +294,8 @@ export function* evaluateTreeSaga( | |||
| evalTreeRequestData, | |||
| ); | |||
|
|
|||
| yield call(waitForFetchEnvironments); | |||
There was a problem hiding this comment.
Move this to the postEvalActionDispatcher definition and add comments.
There was a problem hiding this comment.
waiting on this to be moved
| @@ -1084,6 +1092,18 @@ export default class DataTreeEvaluator { | |||
| const { isFirstTree, metaWidgets, unevalUpdates } = options; | |||
| let staleMetaIds: string[] = []; | |||
|
|
|||
| const triggerBasedDataTreeContext = getDataTreeContext({ | |||
There was a problem hiding this comment.
Trigger based eval context is probably not required. Create a new pr and test with only nonTriggerBasedDataTreeContext
| @@ -305,6 +294,8 @@ export function* evaluateTreeSaga( | |||
| evalTreeRequestData, | |||
| ); | |||
|
|
|||
| yield call(waitForFetchEnvironments); | |||
There was a problem hiding this comment.
waiting on this to be moved
| userScript: string, | ||
| dataTree: DataTree, | ||
| isJSCollection: boolean, | ||
| context?: EvaluateContext, | ||
| // TODO: Fix this the next time the file is edited | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| evalArguments?: Array<any>, | ||
| configTree?: ConfigTree, | ||
| configTree: ConfigTree = {}, | ||
| scopeCache?: EvalContext, |
There was a problem hiding this comment.
Can we name the variable to be able to explain it's usecase.
| scopeCache?: EvalContext, | |
| evalContextCache?: EvalContext, |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/src/workers/common/DataTreeEvaluator/index.ts (1)
1095-1104: Initialize evalContextCache with undefinedThe variable should be explicitly initialized to undefined when the feature flag is disabled.
-let evalContextCache; +let evalContextCache: EvaluateContext | undefined; if (WorkerEnv.flags.release_evaluation_scope_cache) { evalContextCache = getDataTreeContext({ dataTree: contextTree, configTree: oldConfigTree, isTriggerBased: false, }); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
app/client/src/ce/entities/FeatureFlag.ts(2 hunks)app/client/src/sagas/ActionExecution/PluginActionSaga.ts(1 hunks)app/client/src/sagas/EvaluationsSaga.ts(1 hunks)app/client/src/sagas/PostEvaluationSagas.ts(2 hunks)app/client/src/workers/common/DataTreeEvaluator/index.ts(15 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/src/sagas/PostEvaluationSagas.ts
- app/client/src/ce/entities/FeatureFlag.ts
👮 Files not reviewed due to content moderation or server errors (2)
- app/client/src/sagas/EvaluationsSaga.ts
- app/client/src/sagas/ActionExecution/PluginActionSaga.ts
🔇 Additional comments (4)
app/client/src/workers/common/DataTreeEvaluator/index.ts (4)
1068-1069: LGTM: Clean worker state initialization
Good practice to reset worker global scope before evaluation to prevent state leaks.
1114-1118: LGTM: Good performance optimization
Skipping evaluations for actions in JSObjects is a good optimization.
1236-1242: Extract duplicate scope cache update logic
The scope cache update logic is duplicated across multiple switch cases. Consider extracting it into a helper method.
Also applies to: 1289-1295, 1337-1347, 1361-1366
1478-1478: LGTM: Consistent cache parameter propagation
The evalContextCache parameter is consistently added and properly typed across all relevant functions.
Also applies to: 1529-1529, 1623-1623, 1638-1639
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/src/workers/common/DataTreeEvaluator/index.ts (1)
1095-1104: Consider adding error handling for cache initializationWhile the cache initialization is feature-flagged, consider adding error handling for cache initialization failures.
if (WorkerEnv.flags.release_evaluation_scope_cache) { + try { evalContextCache = getDataTreeContext({ dataTree: contextTree, configTree: oldConfigTree, isTriggerBased: false, }); + } catch (error) { + this.errors.push({ + type: EvalErrorTypes.EVAL_CONTEXT_ERROR, + message: (error as Error).message, + }); + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/workers/Evaluation/__tests__/evaluation.test.ts(0 hunks)app/client/src/workers/common/DataTreeEvaluator/index.ts(15 hunks)
💤 Files with no reviewable changes (1)
- app/client/src/workers/Evaluation/tests/evaluation.test.ts
🔇 Additional comments (4)
app/client/src/workers/common/DataTreeEvaluator/index.ts (4)
41-41: LGTM - Imports are well organized
The new imports support the worker scope cache functionality and evaluation context.
Also applies to: 90-94, 155-156
1114-1118: LGTM - Optimization for action evaluations
Skipping evaluations for actions in JSObjects is a good optimization that prevents redundant processing.
1236-1242: 🛠️ Refactor suggestion
Extract duplicate cache update logic
The cache update logic is duplicated across multiple switch cases. Consider extracting it into a helper method.
+ private updateScopeCaches(fullPropertyPath: string, value: unknown) {
+ if (!WorkerEnv.flags.release_evaluation_scope_cache || !this.evalContextCache) {
+ return;
+ }
+ set(this.evalContextCache, fullPropertyPath, klona(value));
+ }
- if (WorkerEnv.flags.release_evaluation_scope_cache && evalContextCache) {
- set(evalContextCache, fullPropertyPath, klona(parsedValue));
- }
+ this.updateScopeCaches(fullPropertyPath, parsedValue);Also applies to: 1289-1299, 1341-1351, 1365-1374
1068-1069: Verify worker scope reset behavior
The worker global scope reset is crucial for preventing state leaks between evaluations.
✅ Verification successful
The first script will help us find the implementation of the resetWorkerGlobalScope function to understand what it does. The second script will help us find any references to worker scope or global scope to understand the context and importance of resetting it.
Let me analyze the results and provide a proper verification of the worker scope reset behavior.
Worker scope reset implementation is robust and necessary
The resetWorkerGlobalScope() function properly cleans up the worker's global scope by:
- Preserving essential worker APIs and DOM APIs
- Keeping library-reserved identifiers and accessors
- Safely removing or nullifying other global variables
The implementation is called at strategic points before evaluations to prevent state leaks between executions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any state persistence issues
ast-grep --pattern 'resetWorkerGlobalScope()'
Length of output: 66326
Script:
#!/bin/bash
# Search for resetWorkerGlobalScope implementation
ast-grep --pattern 'function resetWorkerGlobalScope() {
$$$
}'
# Search for any references to worker scope or global scope
rg -A 5 "resetWorkerGlobalScope|workerGlobalScope|globalScope" --type ts
Length of output: 3796
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12291730436. |
|
Deploy-Preview-URL: https://ce-38068.dp.appsmith.com |
dvj1988
requested review from
vsvamsi1 and
rajatagrawal
and removed request for
ApekshaBhosale,
ayushpahwa and
rishabhrathod01
| @@ -1,4 +1,3 @@ | |||
| import { createMessage, customJSLibraryMessages } from "ee/constants/messages"; | |||
There was a problem hiding this comment.
Note to the reviewer: This change was done to reduce the evalWorker chunk size. This change reduces the size by 20KB.
| set(evalContextCache, fullPropertyPath, klona(parsedValue)); | ||
| } |
There was a problem hiding this comment.
Can you use klonaJSON here
There was a problem hiding this comment.
It was a miss. Changed it to klonaJSON.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
app/client/src/workers/common/DataTreeEvaluator/index.ts (3)
1068-1069: Consider extracting feature flag check into a helper methodThe feature flag check for
release_evaluation_scope_cacheis repeated multiple times in the code. Consider extracting it into a helper method.+ private isEvalScopeCacheEnabled(): boolean { + return WorkerEnv.flags.release_evaluation_scope_cache; + } - if (WorkerEnv.flags.release_evaluation_scope_cache) { + if (this.isEvalScopeCacheEnabled()) {Also applies to: 1095-1104
1114-1118: Add a comment explaining the optimization rationaleConsider adding a comment explaining why we skip evaluations for actions in JSObjects.
+ // Skip evaluations for actions in JSObjects since they are handled separately + // and don't require immediate evaluation if (isPropertyAnEntityAction(entity, propertyPath, entityConfig)) { continue; }
1486-1486: Add JSDoc comments for evalContextCache parameterConsider adding JSDoc comments to document the purpose and usage of the evalContextCache parameter.
+ /** + * @param evalContextCache Optional cache context to optimize evaluation + * @description When provided, stores evaluated values to prevent redundant calculations + */Also applies to: 1537-1537, 1631-1631, 1646-1647
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/workers/common/DataTreeEvaluator/index.ts(15 hunks)
🔇 Additional comments (2)
app/client/src/workers/common/DataTreeEvaluator/index.ts (2)
41-41: LGTM: Import statements are well organized
The new imports are properly structured and directly support the worker scope caching functionality.
Also applies to: 90-94, 155-156
1236-1242: Extract duplicate scope cache update logic
The scope cache update logic is duplicated across multiple switch cases. Consider extracting it into a helper method.
Also applies to: 1289-1299, 1341-1351, 1365-1374
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
app/client/src/workers/common/DataTreeEvaluator/test.ts (1)
771-771: LGTM! Efficient use of Set for action names.The addition of
actionNamesas a Set provides quick lookups for available actions in both JSObject configurations.Consider adding a test case to verify that duplicate action names are handled correctly by the Set data structure.
Also applies to: 825-825
app/client/src/workers/Evaluation/JSObject/test.ts (3)
Line range hint
69-70: Fix async function mismatch in test assertionsThe test data shows
myFun2as an async function in the actual result but the expected result doesn't match this:- myFun2: new String("() => {\n yeso;\n}"), + myFun2: new String("async () => {\n yeso;\n}"),Also applies to: 108-109
Line range hint
143-159: Simplify test data by removing duplicatesThe test data contains duplicate declarations which make it harder to maintain. Consider simplifying:
-body: "export default {\n\tmyVar1: [],\n\tmyVar1: [],\n\tmyVar2: {},\n\tmyFun1: () => {\n\t\t//write code here\n\t\t\n\t},\n\tmyFun2: () => {\n\t\t//use async-await or promises\n\t\tyeso\n\t}\n,\n\tmyFun2: () => {\n\t\t//use async-await or promises\n\t\tyeso\n\t}}", +body: "export default {\n\tmyVar1: [],\n\tmyVar2: {},\n\tmyFun1: () => {\n\t\t//write code here\n\t\t\n\t},\n\tmyFun2: () => {\n\t\t//use async-await or promises\n\t\tyeso\n\t}}",
Line range hint
73-82: Consider using template literals for better readabilityThe test data uses concatenated strings which are hard to read. Consider using template literals:
-body: "export default {\n\tmyVar1: [],\n\tmyVar2: {},\n\tmyFun1: () => {\n\t\t//write code here\n\t\t\n\t},\n\tmyFun2: () => {\n\t\t//use async-await or promises\n\t\tyeso\n\t}\n}", +body: `export default { + myVar1: [], + myVar2: {}, + myFun1: () => { + //write code here + }, + myFun2: () => { + //use async-await or promises + yeso + } +}`,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts(2 hunks)app/client/src/ce/entities/DataTree/dataTreeJSAction.ts(2 hunks)app/client/src/ce/entities/DataTree/types.ts(1 hunks)app/client/src/ce/workers/Evaluation/evaluationUtils.ts(1 hunks)app/client/src/workers/Evaluation/JSObject/test.ts(1 hunks)app/client/src/workers/common/DataTreeEvaluator/test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- app/client/src/ce/entities/DataTree/types.ts
- app/client/src/ce/workers/Evaluation/evaluationUtils.ts
- app/client/src/ce/entities/DataTree/dataTreeJSAction.ts
- app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts
| @@ -216,6 +213,7 @@ describe("saveResolvedFunctionsAndJSUpdates", function () { | |||
| pluginType: "JS", | |||
| name: "JSObject1", | |||
| actionId: "64013546b956c26882acc587", | |||
| actionNames: new Set(["myFun1", "myFun2"]), | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add assertion for actionNames property
The test adds the actionNames property but doesn't verify it in the assertions. Consider adding:
expect(result).toStrictEqual(expectedJSUpdates);
+expect(unEvalDataTree.JSObject1.actionNames).toEqual(new Set(["myFun1", "myFun2"]));Committable suggestion skipped: line range outside the PR's diff.
…ppsmithorg#38068) ## Description - Add evalContextCache to reduce the number of times the evalContext is created from the contextTree - remove resetWorkerGlobalScope execution before every evalSync - Instantiate the evalWorker in src/index.tsx Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## 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/12295048843> > Commit: 04b1e85 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12295048843&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Thu, 12 Dec 2024 12:21:04 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 - **New Features** - Introduced a new property `actionNames` to enhance action tracking within JavaScript action entities. - Added a new feature flag `release_evaluation_scope_cache` for improved feature management. - Implemented a new function `isPropertyAnEntityAction` to identify action properties in JavaScript entities. - Enhanced the `loadAppEntities` method to improve JavaScript library loading processes. - Updated the evaluation context initialization process to utilize `getDataTreeContext`. - Expanded the `WIDGET_CONFIG_MAP` to include detailed configurations for various widget types. - **Bug Fixes** - Enhanced error handling for unsafe function calls in evaluation logic. - Improved error handling and logging for library installation and uninstallation processes. - **Tests** - Expanded test coverage for action bindings and dependencies in the `DataTreeEvaluator`. - Updated tests to validate the new `actionNames` property and its integration. - Modified tests to ensure correct functionality of the `evaluateSync` function. - Added new test cases to assess the behavior of the evaluator with widget updates. - **Chores** - Streamlined imports and initialization of worker instances across various files. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
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/12295048843
Commit: 04b1e85
Cypress dashboard.
Tags:
@tag.AllSpec:
Thu, 12 Dec 2024 12:21:04 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
actionNamesto enhance action tracking within JavaScript action entities.release_evaluation_scope_cachefor improved feature management.isPropertyAnEntityActionto identify action properties in JavaScript entities.loadAppEntitiesmethod to improve JavaScript library loading processes.getDataTreeContext.WIDGET_CONFIG_MAPto include detailed configurations for various widget types.Bug Fixes
Tests
DataTreeEvaluator.actionNamesproperty and its integration.evaluateSyncfunction.Chores