chore: debounce handle action updates #38396
Conversation
WalkthroughThis pull request introduces modifications to the action data handling mechanism in the application. The changes primarily involve updating type definitions, modifying Redux action types, and refactoring the evaluation saga to handle action data updates more efficiently. The modifications aim to streamline the process of updating and evaluating action data across different components of the application. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Redux Store
participant EvaluationSaga
participant EvalWorker
Client->>Redux Store: Dispatch UPDATE_ACTION_DATA
Redux Store->>EvaluationSaga: Trigger Action
EvaluationSaga->>EvalWorker: Update Action Data
EvalWorker-->>EvaluationSaga: Evaluation Complete
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
🧹 Nitpick comments (1)
app/client/src/sagas/EvaluationsSaga.ts (1)
600-616: Optional chaining suggestion.
Consider usingif (actionDataPayload?.length)to simplify the null check.- if (actionDataPayload && actionDataPayload.length) { + if (actionDataPayload?.length) {🧰 Tools
🪛 Biome (1.9.4)
[error] 604-604: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/client/src/actions/pluginActionActions.ts(1 hunks)app/client/src/ce/actions/evaluationActionsList.ts(1 hunks)app/client/src/sagas/ActionExecution/PluginActionSaga.ts(0 hunks)app/client/src/sagas/EvaluationsSaga.test.ts(6 hunks)app/client/src/sagas/EvaluationsSaga.ts(5 hunks)
💤 Files with no reviewable changes (1)
- app/client/src/sagas/ActionExecution/PluginActionSaga.ts
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/src/sagas/EvaluationsSaga.ts
[error] 604-604: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (18)
app/client/src/sagas/EvaluationsSaga.ts (8)
97-101: Properly imported new types.
These imports look correctly declared.
543-547: New BUFFERED_ACTION interface is coherent.
No issues observed with the structure or property definitions.
550-553: Initialization logic is clear.
These variables and their initial states are appropriate for buffering.
568-575: State reset approach is valid.
Resetting the consolidated values ensures correct handling of subsequent actions.
578-583: Consolidated return structure is well-defined.
No issues with the final object structure containing the necessary props.
617-619: Minimal overhead approach.
Marking hasBufferedAction and canTake is straightforward with no apparent pitfalls.
804-826: Separate handling for UPDATE_ACTION_DATA.
The new condition ensures the worker receives the updated data properly.
827-853: Buffered action logic is well-structured.
Delegating UPDATE_ACTION_DATA and falling back to existing flow is consistent.
app/client/src/ce/actions/evaluationActionsList.ts (1)
111-111: Adding UPDATE_ACTION_DATA is consistent.
Its inclusion in EVALUATE_REDUX_ACTIONS aligns with the saga’s handling logic.
app/client/src/actions/pluginActionActions.ts (1)
391-391: Exported type improves reusability.
Exposing actionDataPayload fosters clarity and consistency across modules.
app/client/src/sagas/EvaluationsSaga.test.ts (8)
29-29: New import used in tests.
Importing updateActionData sets the stage for saga buffer coverage.
194-196: Correct initialization of actionDataPayloadConsolidated.
Defaults properly to an empty array for subsequent checks.
214-216: Accumulated action data remains consistent.
The final state includes all changes to JS objects.
238-240: Flagging isAllAffected if any action triggers broad changes.
Merging logic for JS objects is properly handled.
256-258: Ensures a clean slate for subsequent buffers.
Resetting collected objects prevents accidental carryover.
271-273: Default fallback is correct.
If no affected objects exist, the default empties are used.
279-330: UPDATE_ACTION_DATA debouncing.
The consolidation of multiple payloads into a single array is correct and improves efficiency.
331-386: Combined buffering.
Handling both UPDATE_ACTION_DATA and other actions together is logically sound and well-tested.
Description
Debounced handleActionUpdate actions together with bufferedActions, this has reduced the webworker scripting and LCP by about 25-30% on a windows machine.
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/12542044958
Commit: 834a437
Cypress dashboard.
Tags:
@tag.AllSpec:
Mon, 30 Dec 2024 06:24:18 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Refactor
Tests