-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: function undefined error after resetWidget is called due to sync… #33600
Conversation
WalkthroughWalkthroughThe recent updates focus on enhancing the evaluation process in the Changes
Assessment against linked issues
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9169204952. |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9170559529. |
Deploy-Preview-URL: https://ce-33600.dp.appsmith.com |
@rishabhrathod01 in the issue created you have written "resetWidget function causes the next async method to be undefined". so we need to switch it back to async for next method. why can't we do this before calling next async rather than depending on previous one to be in the right context? |
For example:-
Here once the resetWidget is executed, we won't be able to switch back to async eval as
|
Understood! then reset Widget should be in async always isn't it? rather than setting it back. AFAIK we were treating all Appsmith platform functions as async and executing them in async context. @rishabhrathod01 |
@ApekshaBhosale We added an sync evaluation inside the resetWidget to fix bug explained in this comment https://github.com/appsmithorg/appsmith/pull/29151/files#r1607899227 |
Description
Fixed the error of async function like
Api.run
beingundefined
after runningresetWidget
.Context:-
we have 2 types of evaluation i.e., sync and async evaluation.
These are dependent on the field we are evaluating, for the data field we use sync evaluation and for the trigger field, we use async evaluation. In Async evaluation, we also define entity functions like
Api.run
.undefined
error.Fixes #33601
Steps to reproduce
Automation
/ok-to-test tags="@tag.JS"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9170093222
Commit: f9e8893
Cypress dashboard url: Click here!
Communication
Should the DevRel and Marketing teams inform users about this change?