Buffered both trigger and Buffered #38215
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request modifies the Changes
Sequence DiagramsequenceDiagram
participant Saga as EvaluationsSaga
participant Channel as evtActionChannel
participant Timeout as Timeout Mechanism
loop Action Processing
Saga->>Channel: Listen for actions
Saga->>Timeout: Set 1000ms timeout
alt Trigger Action Detected
Saga-->>Saga: Buffer Action
Saga-->>Saga: Continue Listening
else Timeout Occurs
Saga-->>Saga: Process Buffered Action
end
end
Possibly Related PRs
Poem
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/client/src/sagas/EvaluationsSaga.ts (1)
776-776: Avoid reassigning 'action' variable to prevent confusionReassigning the
actionvariable shadows the outer scope variable and can lead to confusion. Use a different variable name.Apply this diff to rename the variable:
- const action = bufferedAction as any; + const bufferedEvalAction = bufferedAction as any; const affectedJSObjects = getAffectedJSObjectIdsFromAction( - action, + bufferedEvalAction, ); yield call(evalAndLintingHandler, true, - action, + bufferedEvalAction, { shouldReplay: get(action, "payload.shouldReplay"), forceEvaluation: shouldForceEval(action),
| const { action, timeout } = yield race({ | ||
| action: take(evtActionChannel), | ||
| timeout: delay(1000), | ||
| }); | ||
|
|
||
| // We are dequing actions from the buffer and inferring the JS actions affected by each | ||
| // action. Through this we know ahead the nodes we need to specifically diff, thereby improving performance. | ||
| const affectedJSObjects = getAffectedJSObjectIdsFromAction(action); | ||
| if ( | ||
| action?.type === ReduxActionTypes.TRIGGER_EVAL || | ||
| isEqual(action, bufferedAction) | ||
| ) { | ||
| hasTriggerAction = true; | ||
| continue; | ||
| } | ||
|
|
||
| yield call(evalAndLintingHandler, true, action, { | ||
| shouldReplay: get(action, "payload.shouldReplay"), | ||
| forceEvaluation: shouldForceEval(action), | ||
| requiresLogging: shouldLog(action), | ||
| affectedJSObjects, | ||
| }); | ||
| if (timeout) { | ||
| if (hasTriggerAction) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const action = bufferedAction as any; | ||
| const affectedJSObjects = getAffectedJSObjectIdsFromAction(action); | ||
|
|
||
| yield call(evalAndLintingHandler, true, action, { | ||
| shouldReplay: get(action, "payload.shouldReplay"), | ||
| forceEvaluation: shouldForceEval(action), | ||
| requiresLogging: shouldLog(action), | ||
| affectedJSObjects, | ||
| }); | ||
| } | ||
| } else { | ||
| // We are dequing actions from the buffer and inferring the JS actions affected by each | ||
| // action. Through this we know ahead the nodes we need to specifically diff, thereby improving performance. | ||
| const affectedJSObjects = getAffectedJSObjectIdsFromAction(action); | ||
|
|
||
| yield call(evalAndLintingHandler, true, action, { | ||
| shouldReplay: get(action, "payload.shouldReplay"), | ||
| forceEvaluation: shouldForceEval(action), | ||
| requiresLogging: shouldLog(action), | ||
| affectedJSObjects, | ||
| }); | ||
| } | ||
|
|
||
| hasTriggerAction = false; |
There was a problem hiding this comment.
Fix usage of 'bufferedAction' before its declaration
The variable bufferedAction is used before its declaration at line 803, which will cause a ReferenceError. Move the declaration of bufferedAction before its usage.
Apply this diff to fix the issue:
+ const bufferedAction = {
+ postEvalActions: [],
+ affectedJSObjects: {
+ isAllAffected: false,
+ ids: [],
+ },
+ type: "BUFFERED_ACTION",
+ };
while (true) {
const { action, timeout } = yield race({
action: take(evtActionChannel),
timeout: delay(1000),
});Committable suggestion skipped: line range outside the PR's diff.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12379400180. |
|
Deploy-Preview-URL: https://ce-38215.dp.appsmith.com |
|
/build-deploy-preview skip-tests=true |
vsvamsi1
added
ok-to-test
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12380384006. |
vsvamsi1
added
ok-to-test
|
Deploy-Preview-URL: https://ce-38215.dp.appsmith.com |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
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
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12379248898
Commit: 0bcb2a6
Cypress dashboard.
Tags: @tag.All
Spec:
The following are new failures, please fix them before merging the PR:
- cypress/e2e/Regression/ClientSide/ActionExecution/NavigateTo_spec.ts
- cypress/e2e/Regression/ClientSide/Debugger/Widget_property_navigation_spec.ts
- cypress/e2e/Regression/ClientSide/OneClickBinding/TableWidget/OneClickBindingMysql_spec.ts
- cypress/e2e/Regression/ClientSide/OneClickBinding/TableWidget/Table_MongoDB_spec.ts
- cypress/e2e/Regression/ClientSide/OneClickBinding/TableWidget/Table_postgres_spec.ts
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/Childwigets/List_FilePicker_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicChildWidgetInteraction_spec.js
- cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_WithButtonGroupIconWidget_spec.ts
List of identified flaky tests.Wed, 18 Dec 2024 06:16:43 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes